Skip to content

Commit 454defc

Browse files
odrotbohmodrotbohm
committed
DATACMNS-319 - Made Repositories less greedy in looking up beans.
We now don't try to eagerly lookup the beans Repositories shall capture in the constructor anymore but leniently look them up on the first request. Update tests for classes using Repositories alongside.
1 parent 4303fdf commit 454defc

File tree

6 files changed

+220
-185
lines changed

6 files changed

+220
-185
lines changed

‎src/main/java/org/springframework/data/repository/support/Repositories.java‎

Lines changed: 64 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012 the original author or authors.
2+
* Copyright 2012-2013 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,13 +16,16 @@
1616
package org.springframework.data.repository.support;
1717

1818
import java.io.Serializable;
19-
import java.util.Collection;
19+
import java.util.Arrays;
2020
import java.util.Collections;
2121
import java.util.HashMap;
22+
import java.util.HashSet;
2223
import java.util.Iterator;
2324
import java.util.List;
2425
import java.util.Map;
26+
import java.util.Set;
2527

28+
import org.springframework.beans.factory.BeanFactory;
2629
import org.springframework.beans.factory.BeanFactoryUtils;
2730
import org.springframework.beans.factory.ListableBeanFactory;
2831
import org.springframework.data.mapping.PersistentEntity;
@@ -44,13 +47,16 @@ public class Repositories implements Iterable<Class<?>> {
4447
static final Repositories NONE = new Repositories();
4548

4649
private final Map<Class<?>, RepositoryFactoryInformation<Object, Serializable>> domainClassToBeanName = new HashMap<Class<?>, RepositoryFactoryInformation<Object, Serializable>>();
47-
private final Map<RepositoryFactoryInformation<Object, Serializable>, CrudRepository<Object, Serializable>> repositories = new HashMap<RepositoryFactoryInformation<Object, Serializable>, CrudRepository<Object, Serializable>>();
50+
private final Map<RepositoryFactoryInformation<Object, Serializable>, String> repositories = new HashMap<RepositoryFactoryInformation<Object, Serializable>, String>();
51+
52+
private final BeanFactory beanFactory;
53+
private final Set<String> repositoryFactoryBeanNames = new HashSet<String>();
4854

4955
/**
5056
* Constructor to create the {@link #NONE} instance.
5157
*/
5258
private Repositories() {
53-
59+
this.beanFactory = null;
5460
}
5561

5662
/**
@@ -59,28 +65,14 @@ private Repositories() {
5965
*
6066
* @param factory must not be {@literal null}.
6167
*/
62-
@SuppressWarnings({ "rawtypes", "unchecked" })
6368
public Repositories(ListableBeanFactory factory) {
6469

6570
Assert.notNull(factory);
71+
this.beanFactory = factory;
6672

67-
Collection<RepositoryFactoryInformation> providers = BeanFactoryUtils.beansOfTypeIncludingAncestors(factory,
68-
RepositoryFactoryInformation.class).values();
69-
70-
for (RepositoryFactoryInformation<Object, Serializable> info : providers) {
71-
72-
RepositoryInformation information = info.getRepositoryInformation();
73-
Class repositoryInterface = information.getRepositoryInterface();
74-
75-
if (CrudRepository.class.isAssignableFrom(repositoryInterface)) {
76-
Class<CrudRepository<Object, Serializable>> objectType = repositoryInterface;
77-
CrudRepository<Object, Serializable> repository = BeanFactoryUtils.beanOfTypeIncludingAncestors(factory,
78-
objectType);
79-
80-
this.domainClassToBeanName.put(information.getDomainType(), info);
81-
this.repositories.put(info, repository);
82-
}
83-
}
73+
String[] beanNamesForType = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(factory,
74+
RepositoryFactoryInformation.class, false, false);
75+
this.repositoryFactoryBeanNames.addAll(Arrays.asList(beanNamesForType));
8476
}
8577

8678
/**
@@ -90,6 +82,7 @@ public Repositories(ListableBeanFactory factory) {
9082
* @return
9183
*/
9284
public boolean hasRepositoryFor(Class<?> domainClass) {
85+
lookupRepositoryFactoryInformationFor(domainClass);
9386
return domainClassToBeanName.containsKey(domainClass);
9487
}
9588

@@ -101,7 +94,14 @@ public boolean hasRepositoryFor(Class<?> domainClass) {
10194
*/
10295
@SuppressWarnings("unchecked")
10396
public <T, S extends Serializable> CrudRepository<T, S> getRepositoryFor(Class<?> domainClass) {
104-
return (CrudRepository<T, S>) repositories.get(domainClassToBeanName.get(domainClass));
97+
98+
RepositoryFactoryInformation<Object, Serializable> information = getRepoInfoFor(domainClass);
99+
100+
if (information == null) {
101+
return null;
102+
}
103+
104+
return (CrudRepository<T, S>) beanFactory.getBean(repositories.get(information));
105105
}
106106

107107
/**
@@ -166,14 +166,54 @@ private RepositoryFactoryInformation<Object, Serializable> getRepoInfoFor(Class<
166166
}
167167
}
168168

169-
return null;
169+
return lookupRepositoryFactoryInformationFor(domainClass);
170170
}
171171

172172
/*
173173
* (non-Javadoc)
174174
* @see java.lang.Iterable#iterator()
175175
*/
176176
public Iterator<Class<?>> iterator() {
177+
lookupRepositoryFactoryInformationFor(null);
177178
return domainClassToBeanName.keySet().iterator();
178179
}
180+
181+
/**
182+
* Looks up the {@link RepositoryFactoryInformation} for a given domain type. Will inspect the {@link BeanFactory} for
183+
* beans implementing {@link RepositoryFactoryInformation} and cache the domain class to repository bean name mappings
184+
* for further lookups. If a {@link RepositoryFactoryInformation} for the given domain type is found we interrupt the
185+
* lookup proces to prevent beans from being looked up early.
186+
*
187+
* @param domainType
188+
* @return
189+
*/
190+
@SuppressWarnings("unchecked")
191+
private RepositoryFactoryInformation<Object, Serializable> lookupRepositoryFactoryInformationFor(Class<?> domainType) {
192+
193+
if (domainClassToBeanName.containsKey(domainType)) {
194+
return domainClassToBeanName.get(domainType);
195+
}
196+
197+
for (String repositoryFactoryName : repositoryFactoryBeanNames) {
198+
199+
RepositoryFactoryInformation<Object, Serializable> information = beanFactory.getBean(repositoryFactoryName,
200+
RepositoryFactoryInformation.class);
201+
202+
RepositoryInformation info = information.getRepositoryInformation();
203+
Class<?> repositoryInterface = info.getRepositoryInterface();
204+
205+
if (!CrudRepository.class.isAssignableFrom(repositoryInterface)) {
206+
continue;
207+
}
208+
209+
repositories.put(information, BeanFactoryUtils.transformedBeanName(repositoryFactoryName));
210+
domainClassToBeanName.put(info.getDomainType(), information);
211+
212+
if (info.getDomainType().equals(domainType)) {
213+
return information;
214+
}
215+
}
216+
217+
return null;
218+
}
179219
}

‎src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactoryBean.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012 the original author or authors.
2+
* Copyright 2012-2013 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@
1919

2020
import java.io.Serializable;
2121

22+
import org.springframework.data.mapping.context.SampleMappingContext;
2223
import org.springframework.data.repository.Repository;
2324

2425
/**
@@ -27,14 +28,27 @@
2728
public class DummyRepositoryFactoryBean<T extends Repository<S, ID>, S, ID extends Serializable> extends
2829
RepositoryFactoryBeanSupport<T, S, ID> {
2930

31+
private T repository;
32+
33+
public DummyRepositoryFactoryBean() {
34+
setMappingContext(new SampleMappingContext());
35+
}
36+
37+
/* (non-Javadoc)
38+
* @see org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport#setRepositoryInterface(java.lang.Class)
39+
*/
40+
@Override
41+
public void setRepositoryInterface(Class<? extends T> repositoryInterface) {
42+
this.repository = mock(repositoryInterface);
43+
super.setRepositoryInterface(repositoryInterface);
44+
}
45+
3046
/*
3147
* (non-Javadoc)
3248
* @see org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport#createRepositoryFactory()
3349
*/
3450
@Override
3551
protected RepositoryFactorySupport createRepositoryFactory() {
36-
37-
Repository<?, ?> repository = mock(Repository.class);
3852
return new DummyRepositoryFactory(repository);
3953
}
4054
}

‎src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.java‎

Lines changed: 25 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2008-2012 the original author or authors.
2+
* Copyright 2008-2013 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,24 +21,24 @@
2121
import static org.mockito.Mockito.*;
2222

2323
import java.io.Serializable;
24-
import java.util.HashMap;
25-
import java.util.Map;
2624

27-
import org.hamcrest.Description;
28-
import org.hamcrest.TypeSafeMatcher;
2925
import org.junit.Before;
3026
import org.junit.Test;
3127
import org.junit.runner.RunWith;
3228
import org.mockito.Mock;
3329
import org.mockito.runners.MockitoJUnitRunner;
30+
import org.springframework.aop.framework.Advised;
31+
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
32+
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
3433
import org.springframework.context.ApplicationContext;
34+
import org.springframework.context.support.GenericApplicationContext;
3535
import org.springframework.core.convert.TypeDescriptor;
3636
import org.springframework.core.convert.support.DefaultConversionService;
3737
import org.springframework.data.repository.CrudRepository;
3838
import org.springframework.data.repository.core.EntityInformation;
3939
import org.springframework.data.repository.core.RepositoryInformation;
4040
import org.springframework.data.repository.core.support.DummyEntityInformation;
41-
import org.springframework.data.repository.core.support.RepositoryFactoryInformation;
41+
import org.springframework.data.repository.core.support.DummyRepositoryFactoryBean;
4242

4343
/**
4444
* Unit test for {@link DomainClassConverter}.
@@ -56,17 +56,8 @@ public class DomainClassConverterUnitTests {
5656
TypeDescriptor sourceDescriptor;
5757
TypeDescriptor targetDescriptor;
5858

59-
@SuppressWarnings("rawtypes")
60-
Map<String, RepositoryFactoryInformation> providers;
61-
62-
@Mock
63-
ApplicationContext context, parent;
64-
@Mock
65-
UserRepository repository;
6659
@Mock
6760
DefaultConversionService service;
68-
@Mock
69-
RepositoryFactoryInformation<User, Serializable> provider;
7061

7162
@Before
7263
@SuppressWarnings({ "unchecked", "rawtypes" })
@@ -76,27 +67,22 @@ public void setUp() {
7667
RepositoryInformation repositoryInformation = new DummyRepositoryInformation(UserRepository.class);
7768

7869
converter = new DomainClassConverter(service);
79-
providers = new HashMap<String, RepositoryFactoryInformation>();
8070

8171
sourceDescriptor = TypeDescriptor.valueOf(String.class);
8272
targetDescriptor = TypeDescriptor.valueOf(User.class);
83-
84-
when(provider.getEntityInformation()).thenReturn(information);
85-
when(provider.getRepositoryInformation()).thenReturn(repositoryInformation);
8673
}
8774

8875
@Test
8976
public void matchFailsIfNoDaoAvailable() throws Exception {
9077

91-
converter.setApplicationContext(context);
78+
converter.setApplicationContext(new GenericApplicationContext());
9279
assertMatches(false);
9380
}
9481

9582
@Test
9683
public void matchesIfConversionInBetweenIsPossible() throws Exception {
9784

98-
letContextContain(context, provider);
99-
converter.setApplicationContext(context);
85+
converter.setApplicationContext(initContextWithRepo());
10086

10187
when(service.canConvert(String.class, Long.class)).thenReturn(true);
10288

@@ -106,8 +92,7 @@ public void matchesIfConversionInBetweenIsPossible() throws Exception {
10692
@Test
10793
public void matchFailsIfNoIntermediateConversionIsPossible() throws Exception {
10894

109-
letContextContain(context, provider);
110-
converter.setApplicationContext(context);
95+
converter.setApplicationContext(initContextWithRepo());
11196

11297
when(service.canConvert(String.class, Long.class)).thenReturn(false);
11398

@@ -136,16 +121,18 @@ private void assertMatches(boolean matchExpected) {
136121
@Test
137122
public void convertsStringToUserCorrectly() throws Exception {
138123

139-
letContextContain(context, provider);
124+
ApplicationContext context = initContextWithRepo();
140125
converter.setApplicationContext(context);
141126

142127
when(service.canConvert(String.class, Long.class)).thenReturn(true);
143128
when(service.convert(anyString(), eq(Long.class))).thenReturn(1L);
144-
when(repository.findOne(1L)).thenReturn(USER);
145129

146-
Object user = converter.convert("1", sourceDescriptor, targetDescriptor);
147-
assertThat(user, is(instanceOf(User.class)));
148-
assertThat(user, is((Object) USER));
130+
converter.convert("1", sourceDescriptor, targetDescriptor);
131+
132+
UserRepository bean = context.getBean(UserRepository.class);
133+
UserRepository repo = (UserRepository) ((Advised) bean).getTargetSource().getTarget();
134+
135+
verify(repo, times(1)).findOne(1L);
149136
}
150137

151138
/**
@@ -154,54 +141,24 @@ public void convertsStringToUserCorrectly() throws Exception {
154141
@Test
155142
public void discoversFactoryAndRepoFromParentApplicationContext() {
156143

157-
letContextContain(parent, provider);
158-
when(context.getParentBeanFactory()).thenReturn(parent);
144+
ApplicationContext parent = initContextWithRepo();
145+
ApplicationContext context = new GenericApplicationContext(parent);
146+
159147
when(service.canConvert(String.class, Long.class)).thenReturn(true);
160148

161149
converter.setApplicationContext(context);
162150
assertThat(converter.matches(sourceDescriptor, targetDescriptor), is(true));
163151
}
164152

165-
private void letContextContain(ApplicationContext context, Object bean) {
166-
167-
configureContextToReturnBeans(context, repository, provider);
168-
169-
Map<String, Object> beanMap = getBeanAsMap(bean);
170-
when(context.getBeansOfType(argThat(is(subtypeOf(bean.getClass()))))).thenReturn(beanMap);
171-
}
172-
173-
private void configureContextToReturnBeans(ApplicationContext context, UserRepository repository,
174-
RepositoryFactoryInformation<User, Serializable> provider) {
175-
176-
Map<String, UserRepository> map = getBeanAsMap(repository);
177-
when(context.getBeansOfType(UserRepository.class)).thenReturn(map);
178-
179-
providers.put("provider", provider);
180-
when(context.getBeansOfType(RepositoryFactoryInformation.class)).thenReturn(providers);
181-
}
182-
183-
private <T> Map<String, T> getBeanAsMap(T bean) {
184-
185-
Map<String, T> beanMap = new HashMap<String, T>();
186-
beanMap.put(bean.getClass().getName(), bean);
187-
return beanMap;
188-
}
189-
190-
private static <T> TypeSafeMatcher<Class<T>> subtypeOf(final Class<? extends T> type) {
191-
192-
return new TypeSafeMatcher<Class<T>>() {
193-
194-
public void describeTo(Description arg0) {
153+
private ApplicationContext initContextWithRepo() {
195154

196-
arg0.appendText("not a subtype of");
197-
}
155+
BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(DummyRepositoryFactoryBean.class);
156+
builder.addPropertyValue("repositoryInterface", UserRepository.class);
198157

199-
@Override
200-
public boolean matchesSafely(Class<T> arg0) {
158+
DefaultListableBeanFactory factory = new DefaultListableBeanFactory();
159+
factory.registerBeanDefinition("provider", builder.getBeanDefinition());
201160

202-
return arg0.isAssignableFrom(type);
203-
}
204-
};
161+
return new GenericApplicationContext(factory);
205162
}
206163

207164
private static class User {

0 commit comments

Comments
 (0)