DATAREST-93 - Cleanups in repository invoker area.

Made RepositoryInvokerFactory an interface and renamed the previously existing implementation class to DefaultRepositoryInvokerFactory. Removed instantiation of the class from ResourceMetadataHandlerMethodArgumentResolver to keep the logic which implementation to use solely in the configuration.

spring-data-rest-core/src/main/java/org/springframework/data/rest/core/invoke/DefaultRepositoryInvokerFactory.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright 2013 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.rest.core.invoke;
+
+import java.io.Serializable;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.springframework.core.convert.ConversionService;
+import org.springframework.data.repository.CrudRepository;
+import org.springframework.data.repository.PagingAndSortingRepository;
+import org.springframework.data.repository.core.RepositoryInformation;
+import org.springframework.data.repository.support.Repositories;
+import org.springframework.util.Assert;
+
+/**
+ * Default implementation of {@link RepositoryInvokerFactory} to inspect the requested repository type and create a
+ * matching {@link RepositoryInvoker} that suits the repository best. That means, the more concrete the base interface
+ * of the repository is, the more concrete will the actual invoker become - which means it will favor concrete method
+ * invocations over reflection ones.
+ * 
+ * @author Oliver Gierke
+ */
+public class DefaultRepositoryInvokerFactory implements RepositoryInvokerFactory {
+
+	private final Repositories repositories;
+	private final ConversionService conversionService;
+	private final Map<Class<?>, RepositoryInvoker> invokers;
+
+	/**
+	 * Creates a new {@link DefaultRepositoryInvokerFactory} for the given {@link Repositories} and
+	 * {@link ConversionService}.
+	 * 
+	 * @param repositories must not be {@literal null}.
+	 * @param conversionService must not be {@literal null}.
+	 */
+	public DefaultRepositoryInvokerFactory(Repositories repositories, ConversionService conversionService) {
+
+		Assert.notNull(repositories, "Repositories must not be null!");
+		Assert.notNull(conversionService, "ConversionService must not be null!");
+
+		this.repositories = repositories;
+		this.conversionService = conversionService;
+		this.invokers = new HashMap<Class<?>, RepositoryInvoker>();
+
+	}
+
+	/**
+	 * Creates a {@link RepositoryInvoker} for the repository managing the given domain type.
+	 * 
+	 * @param domainType
+	 * @return
+	 */
+	@SuppressWarnings("unchecked")
+	private RepositoryInvoker prepareInvokers(Class<?> domainType) {
+
+		Object repository = repositories.getRepositoryFor(domainType);
+		RepositoryInformation information = repositories.getRepositoryInformationFor(domainType);
+
+		if (repository instanceof PagingAndSortingRepository) {
+			return new PagingAndSortingRepositoryInvoker((PagingAndSortingRepository<Object, Serializable>) repository,
+					information, conversionService);
+		} else if (repository instanceof CrudRepository) {
+			return new CrudRepositoryInvoker((CrudRepository<Object, Serializable>) repository, information,
+					conversionService);
+		} else {
+			return new ReflectionRepositoryInvoker(repository, information, conversionService);
+		}
+	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.rest.core.invoke.RepositoryInvokerFactory#getInvokerFor(java.lang.Class)
+	 */
+	@Override
+	public RepositoryInvoker getInvokerFor(Class<?> domainType) {
+
+		RepositoryInvoker invoker = invokers.get(domainType);
+
+		if (invoker != null) {
+			return invoker;
+		}
+
+		invoker = prepareInvokers(domainType);
+		invokers.put(domainType, invoker);
+
+		return invoker;
+	}
+}

spring-data-rest-core/src/main/java/org/springframework/data/rest/core/invoke/RepositoryInvoker.java

@@ -33,7 +33,7 @@ public interface RepositoryInvoker extends RepositoryInvocationInformation {
 
 	Iterable<Object> invokeFindAll(Pageable pageable);
 
-	Iterable<Object> invokeFindAll(Sort pageable);
+	Iterable<Object> invokeFindAll(Sort sort);
 
 	void invokeDelete(Serializable serializable);
 

spring-data-rest-core/src/main/java/org/springframework/data/rest/core/invoke/RepositoryInvokerFactory.java

@@ -15,65 +15,19 @@
  */
 package org.springframework.data.rest.core.invoke;
 
-import java.io.Serializable;
-import java.util.HashMap;
-import java.util.Map;
-
-import org.springframework.core.convert.ConversionService;
-import org.springframework.data.repository.CrudRepository;
-import org.springframework.data.repository.PagingAndSortingRepository;
-import org.springframework.data.repository.core.RepositoryInformation;
-import org.springframework.data.repository.support.Repositories;
-
 /**
+ * Interface for a factory to create {@link RepositoryInvoker} instances for repositories managing a particular domain
+ * type.
+ * 
  * @author Oliver Gierke
  */
-public class RepositoryInvokerFactory {
-
-	private final Repositories repositories;
-	private final ConversionService conversionService;
-
-	private final Map<Class<?>, RepositoryInvoker> invokers;
+public interface RepositoryInvokerFactory {
 
 	/**
-	 * @param repositories
+	 * Returns the {@link RepositoryInvoker} for a repository managing the given domain type.
+	 * 
+	 * @param domainType must not be {@literal null}.
+	 * @return
 	 */
-	public RepositoryInvokerFactory(Repositories repositories, ConversionService conversionService) {
-
-		this.repositories = repositories;
-		this.conversionService = conversionService;
-		this.invokers = new HashMap<Class<?>, RepositoryInvoker>();
-
-	}
-
-	@SuppressWarnings("unchecked")
-	private RepositoryInvoker prepareInvokers(Class<?> domainType) {
-
-		Object repository = repositories.getRepositoryFor(domainType);
-		RepositoryInformation information = repositories.getRepositoryInformationFor(domainType);
-
-		if (repository instanceof PagingAndSortingRepository) {
-			return new PagingAndSortingRepositoryInvoker((PagingAndSortingRepository<Object, Serializable>) repository,
-					information, conversionService);
-		} else if (repository instanceof CrudRepository) {
-			return new CrudRepositoryInvoker((CrudRepository<Object, Serializable>) repository, information,
-					conversionService);
-		} else {
-			return new ReflectionRepositoryInvoker(repository, information, conversionService);
-		}
-	}
-
-	public RepositoryInvoker getInvokerFor(Class<?> domainType) {
-
-		RepositoryInvoker invoker = invokers.get(domainType);
-
-		if (invoker != null) {
-			return invoker;
-		}
-
-		invoker = prepareInvokers(domainType);
-		invokers.put(domainType, invoker);
-
-		return invoker;
-	}
+	RepositoryInvoker getInvokerFor(Class<?> domainType);
 }

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestRequestHandlerMethodArgumentResolver.java

@@ -16,7 +16,6 @@
 package org.springframework.data.rest.webmvc;
 
 import org.springframework.core.MethodParameter;
-import org.springframework.core.convert.ConversionService;
 import org.springframework.data.mapping.PersistentEntity;
 import org.springframework.data.repository.support.Repositories;
 import org.springframework.data.rest.core.invoke.RepositoryInvoker;
@@ -39,20 +38,22 @@ public class RepositoryRestRequestHandlerMethodArgumentResolver implements Handl
 	private final ResourceMetadataHandlerMethodArgumentResolver resourceMetadataResolver;
 
 	/**
-	 * Creates a new {@link RepositoryRestRequestHandlerMethodArgumentResolver} using the given {@link Repositories} and
-	 * {@link ConversionService}.
+	 * Creates a new {@link RepositoryRestRequestHandlerMethodArgumentResolver} using the given {@link Repositories},
+	 * {@link RepositoryInvokerFactory} and {@link ResourceMetadataHandlerMethodArgumentResolver}.
 	 * 
 	 * @param repositories must not be {@literal null}.
-	 * @param conversionService must not be {@literal null}.
+	 * @param invokerFactory must not be {@literal null}.
+	 * @param resourceMetadataResolver must not be {@literal null}.
 	 */
 	public RepositoryRestRequestHandlerMethodArgumentResolver(Repositories repositories,
-			ConversionService conversionService, ResourceMetadataHandlerMethodArgumentResolver resourceMetadataResolver) {
+			RepositoryInvokerFactory invokerFactory, ResourceMetadataHandlerMethodArgumentResolver resourceMetadataResolver) {
 
 		Assert.notNull(repositories, "Repositories must not be null!");
-		Assert.notNull(conversionService, "ConversionService must not be null!");
+		Assert.notNull(invokerFactory, "invokerFactory must not be null!");
+		Assert.notNull(resourceMetadataResolver, "ResourceMetadataHandlerMethodArgumentResolver must not be null!");
 
 		this.repositories = repositories;
-		this.invokerFactory = new RepositoryInvokerFactory(repositories, conversionService);
+		this.invokerFactory = invokerFactory;
 		this.resourceMetadataResolver = resourceMetadataResolver;
 	}
 

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java

@@ -38,6 +38,7 @@ import org.springframework.data.rest.core.UriDomainClassConverter;
 import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
 import org.springframework.data.rest.core.event.AnnotatedHandlerBeanPostProcessor;
 import org.springframework.data.rest.core.event.ValidatingRepositoryEventListener;
+import org.springframework.data.rest.core.invoke.DefaultRepositoryInvokerFactory;
 import org.springframework.data.rest.core.invoke.RepositoryInvokerFactory;
 import org.springframework.data.rest.core.mapping.ResourceMappings;
 import org.springframework.data.rest.core.support.DomainObjectMerger;
@@ -204,7 +205,7 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon
 	 */
 	@Bean
 	public RepositoryRestRequestHandlerMethodArgumentResolver repoRequestArgumentResolver() {
-		return new RepositoryRestRequestHandlerMethodArgumentResolver(repositories(), defaultConversionService(),
+		return new RepositoryRestRequestHandlerMethodArgumentResolver(repositories(), repositoryInvokerFactory(),
 				resourceMetadataHandlerMethodArgumentResolver());
 	}
 
@@ -376,7 +377,7 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon
 
 	@Bean
 	public RepositoryInvokerFactory repositoryInvokerFactory() {
-		return new RepositoryInvokerFactory(repositories(), defaultConversionService());
+		return new DefaultRepositoryInvokerFactory(repositories(), defaultConversionService());
 	}
 
 	private List<HttpMessageConverter<?>> defaultMessageConverters() {