From 93c708379e2c12bb976b3cc8c1f455f57a74d9a9 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 10 Jan 2020 10:55:35 +0100 Subject: [PATCH] DATACMNS-1508 - Address review comments. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added ClassUtils.ifPresent(…) to conditionally call back a Consumer if a class is available from the given ClassLoader. Extract isSuspend(…) method into KotlinReflectionUtils. Deprecate Kotlin-related methods in our ReflectionUtils as parts are available from Spring Framework directly. Rename CoCrudRepository to CoroutineCrudRepository and CoroutineSortingRepository. Add tests for KotlinReflectionUtils to test calls without Kotlin dependencies. Original pull request: #415. --- src/main/asciidoc/kotlin-coroutines.adoc | 4 +- .../context/AbstractMappingContext.java | 5 +- .../data/mapping/model/BeanWrapper.java | 3 +- ...lassGeneratingPropertyAccessorFactory.java | 3 +- .../ImplementationInvocationMetadata.java | 31 +-- .../QueryExecutorMethodInterceptor.java | 221 ++++++++++++++++++ .../support/RepositoryFactorySupport.java | 183 +-------------- .../data/repository/query/Parameter.java | 13 +- .../data/repository/util/ClassUtils.java | 26 +++ .../data/util/KotlinReflectionUtils.java | 48 +++- .../data/util/ReflectionUtils.java | 24 +- ...pository.kt => CoroutineCrudRepository.kt} | 6 +- ...itory.kt => CoroutineSortingRepository.kt} | 4 +- ...eryExecutorMethodInterceptorUnitTests.java | 21 +- .../util/KotlinReflectionUtilsUnitTests.java | 79 +++++++ ...epositoryCustomImplementationUnitTests.kt} | 4 +- ...kt => CoroutineCrudRepositoryUnitTests.kt} | 4 +- 17 files changed, 425 insertions(+), 254 deletions(-) create mode 100644 src/main/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptor.java rename src/main/kotlin/org/springframework/data/repository/kotlin/{CoCrudRepository.kt => CoroutineCrudRepository.kt} (98%) rename src/main/kotlin/org/springframework/data/repository/kotlin/{CoSortingRepository.kt => CoroutineSortingRepository.kt} (86%) create mode 100644 src/test/java/org/springframework/data/util/KotlinReflectionUtilsUnitTests.java rename src/test/kotlin/org/springframework/data/repository/kotlin/{CoCrudRepositoryCustomImplementationUnitTests.kt => CoroutineCrudRepositoryCustomImplementationUnitTests.kt} (94%) rename src/test/kotlin/org/springframework/data/repository/kotlin/{CoCrudRepositoryUnitTests.kt => CoroutineCrudRepositoryUnitTests.kt} (97%) diff --git a/src/main/asciidoc/kotlin-coroutines.adoc b/src/main/asciidoc/kotlin-coroutines.adoc index 5afeac07d..87c5d9c9f 100644 --- a/src/main/asciidoc/kotlin-coroutines.adoc +++ b/src/main/asciidoc/kotlin-coroutines.adoc @@ -65,7 +65,7 @@ Here is an example of a Coroutines repository: ==== [source,kotlin] ---- -interface CoroutinesRepository : CoCrudRepository { +interface CoroutineRepository : CoroutineCrudRepository { suspend fun findOne(id: String): User @@ -78,4 +78,4 @@ Coroutines repositories are built on reactive repositories to expose the non-blo Methods on a Coroutines repository can be backed either by a query method or a custom implementation. Invoking a custom implementation method propagates the Coroutines invocation to the actual implementation method if the custom method is `suspend`able without requiring the implementation method to return a reactive type such as `Mono` or `Flux`. -NOTE: Coroutines repositories are only discovered when the repository extends the `CoCrudRepository` interface. +NOTE: Coroutines repositories are only discovered when the repository extends the `CoroutineCrudRepository` interface. diff --git a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java index 0ca8755bf..54560f4d4 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -41,6 +41,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; +import org.springframework.core.KotlinDetector; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; @@ -57,6 +58,7 @@ import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.spel.EvaluationContextProvider; import org.springframework.data.spel.ExtensionAwareEvaluationContextProvider; import org.springframework.data.util.ClassTypeInformation; +import org.springframework.data.util.KotlinReflectionUtils; import org.springframework.data.util.Optionals; import org.springframework.data.util.Streamable; import org.springframework.data.util.TypeInformation; @@ -477,8 +479,7 @@ public abstract class AbstractMappingContext implements PersistentPropertyAccessor { return; } - if (org.springframework.data.util.ReflectionUtils.isKotlinClass(property.getOwner().getType())) { + if (KotlinDetector.isKotlinType(property.getOwner().getType())) { this.bean = (T) KotlinCopyUtil.setProperty(property, bean, value); return; diff --git a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java index e46f496c2..984441ee4 100644 --- a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java @@ -45,6 +45,7 @@ import org.springframework.asm.MethodVisitor; import org.springframework.asm.Opcodes; import org.springframework.asm.Type; import org.springframework.cglib.core.ReflectUtils; +import org.springframework.core.KotlinDetector; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyAccessor; @@ -1444,7 +1445,7 @@ public class ClassGeneratingPropertyAccessorFactory implements PersistentPropert Class type = property.getOwner().getType(); - if (isAccessible(type) && org.springframework.data.util.ReflectionUtils.isKotlinClass(type)) { + if (isAccessible(type) && KotlinDetector.isKotlinType(type)) { return KotlinCopyMethod.findCopyMethod(type).filter(it -> it.supportsProperty(property)).isPresent(); } diff --git a/src/main/java/org/springframework/data/repository/core/support/ImplementationInvocationMetadata.java b/src/main/java/org/springframework/data/repository/core/support/ImplementationInvocationMetadata.java index 1b43ed9c5..3b28f51f8 100644 --- a/src/main/java/org/springframework/data/repository/core/support/ImplementationInvocationMetadata.java +++ b/src/main/java/org/springframework/data/repository/core/support/ImplementationInvocationMetadata.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 the original author or authors. + * Copyright 2019-2020 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. @@ -31,7 +31,7 @@ import org.springframework.lang.Nullable; /** * Metadata for a implementation {@link Method} invocation. This value object encapsulates whether the called and the - * backing method are regular methods or suspendable Kotlin coroutines methods. It also allows invocation of suspendable + * backing method are regular methods or suspendable Kotlin coroutines methods. It also allows invocation of suspended * methods by backing the invocation using methods returning reactive types. * * @author Mark Paluch @@ -52,10 +52,10 @@ class ImplementationInvocationMetadata { return; } - KFunction declaredFunction = ReflectionUtils.isKotlinClass(declaredMethod.getDeclaringClass()) + KFunction declaredFunction = KotlinDetector.isKotlinType(declaredMethod.getDeclaringClass()) ? KotlinReflectionUtils.findKotlinFunction(declaredMethod) : null; - KFunction baseClassFunction = ReflectionUtils.isKotlinClass(baseClassMethod.getDeclaringClass()) + KFunction baseClassFunction = KotlinDetector.isKotlinType(baseClassMethod.getDeclaringClass()) ? KotlinReflectionUtils.findKotlinFunction(baseClassMethod) : null; @@ -68,11 +68,13 @@ class ImplementationInvocationMetadata { @Nullable public Object invoke(Method methodToCall, Object instance, Object[] args) throws Throwable { - if (suspendedDeclaredMethod && !suspendedBaseClassMethod && reactiveBaseClassMethod) { - return invokeReactiveToSuspend(methodToCall, instance, args); - } + return shouldAdaptReactiveToSuspended() ? invokeReactiveToSuspend(methodToCall, instance, args) + : methodToCall.invoke(instance, args); - return methodToCall.invoke(instance, args); + } + + private boolean shouldAdaptReactiveToSuspended() { + return suspendedDeclaredMethod && !suspendedBaseClassMethod && reactiveBaseClassMethod; } @Nullable @@ -80,16 +82,17 @@ class ImplementationInvocationMetadata { private Object invokeReactiveToSuspend(Method methodToCall, Object instance, Object[] args) throws ReflectiveOperationException { + /* + * Kotlin suspended functions are invoked with a synthetic Continuation parameter that keeps track of the Coroutine context. + * We're invoking a method without Continuation as we expect the method to return any sort of reactive type, + * therefore we need to strip the Continuation parameter. + */ Object[] invocationArguments = new Object[args.length - 1]; System.arraycopy(args, 0, invocationArguments, 0, invocationArguments.length); Object result = methodToCall.invoke(instance, invocationArguments); - Publisher publisher; - if (result instanceof Publisher) { - publisher = (Publisher) result; - } else { - publisher = ReactiveWrapperConverters.toWrapper(result, Publisher.class); - } + Publisher publisher = result instanceof Publisher ? (Publisher) result + : ReactiveWrapperConverters.toWrapper(result, Publisher.class); return AwaitKt.awaitFirstOrNull(publisher, (Continuation) args[args.length - 1]); } diff --git a/src/main/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptor.java b/src/main/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptor.java new file mode 100644 index 000000000..c1940b014 --- /dev/null +++ b/src/main/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptor.java @@ -0,0 +1,221 @@ +/* + * Copyright 2020 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 + * + * https://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.repository.core.support; + +import kotlin.coroutines.Continuation; +import kotlinx.coroutines.reactive.AwaitKt; + +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import org.aopalliance.intercept.MethodInterceptor; +import org.aopalliance.intercept.MethodInvocation; +import org.reactivestreams.Publisher; + +import org.springframework.core.KotlinDetector; +import org.springframework.core.ResolvableType; +import org.springframework.data.projection.ProjectionFactory; +import org.springframework.data.repository.core.NamedQueries; +import org.springframework.data.repository.core.RepositoryInformation; +import org.springframework.data.repository.query.QueryLookupStrategy; +import org.springframework.data.repository.query.QueryMethod; +import org.springframework.data.repository.query.RepositoryQuery; +import org.springframework.data.repository.util.QueryExecutionConverters; +import org.springframework.data.repository.util.ReactiveWrapperConverters; +import org.springframework.data.repository.util.ReactiveWrappers; +import org.springframework.data.util.KotlinReflectionUtils; +import org.springframework.data.util.Pair; +import org.springframework.lang.Nullable; +import org.springframework.util.ConcurrentReferenceHashMap; + +/** + * This {@link MethodInterceptor} intercepts calls to methods of the custom implementation and delegates the to it if + * configured. Furthermore it resolves method calls to finders and triggers execution of them. You can rely on having a + * custom repository implementation instance set if this returns true. + * + * @author Oliver Gierke + * @author Mark Paluch + */ +class QueryExecutorMethodInterceptor implements MethodInterceptor { + + private final Map queries; + private final Map invocationMetadataCache = new ConcurrentReferenceHashMap<>(); + private final QueryExecutionResultHandler resultHandler; + private final NamedQueries namedQueries; + private final List> queryPostProcessors; + + /** + * Creates a new {@link QueryExecutorMethodInterceptor}. Builds a model of {@link QueryMethod}s to be invoked on + * execution of repository interface methods. + */ + public QueryExecutorMethodInterceptor(RepositoryInformation repositoryInformation, + ProjectionFactory projectionFactory, Optional queryLookupStrategy, NamedQueries namedQueries, + List> queryPostProcessors) { + + this.namedQueries = namedQueries; + this.queryPostProcessors = queryPostProcessors; + + this.resultHandler = new QueryExecutionResultHandler(RepositoryFactorySupport.CONVERSION_SERVICE); + + if (!queryLookupStrategy.isPresent() && repositoryInformation.hasQueryMethods()) { + + throw new IllegalStateException("You have defined query method in the repository but " + + "you don't have any query lookup strategy defined. The " + + "infrastructure apparently does not support query methods!"); + } + + this.queries = queryLookupStrategy // + .map(it -> mapMethodsToQuery(repositoryInformation, it, projectionFactory)) // + .orElse(Collections.emptyMap()); + } + + private Map mapMethodsToQuery(RepositoryInformation repositoryInformation, + QueryLookupStrategy lookupStrategy, ProjectionFactory projectionFactory) { + + return repositoryInformation.getQueryMethods().stream() // + .map(method -> lookupQuery(method, repositoryInformation, lookupStrategy, projectionFactory)) // + .peek(pair -> invokeListeners(pair.getSecond())) // + .collect(Pair.toMap()); + } + + private Pair lookupQuery(Method method, RepositoryInformation information, + QueryLookupStrategy strategy, ProjectionFactory projectionFactory) { + return Pair.of(method, strategy.resolveQuery(method, information, projectionFactory, namedQueries)); + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + private void invokeListeners(RepositoryQuery query) { + + for (QueryCreationListener listener : queryPostProcessors) { + + ResolvableType typeArgument = ResolvableType.forClass(QueryCreationListener.class, listener.getClass()) + .getGeneric(0); + + if (typeArgument != null && typeArgument.isAssignableFrom(ResolvableType.forClass(query.getClass()))) { + listener.onCreation(query); + } + } + } + + /* + * (non-Javadoc) + * @see org.aopalliance.intercept.MethodInterceptor#invoke(org.aopalliance.intercept.MethodInvocation) + */ + @Override + @Nullable + public Object invoke(@SuppressWarnings("null") MethodInvocation invocation) throws Throwable { + + Method method = invocation.getMethod(); + + QueryExecutionConverters.ExecutionAdapter executionAdapter = QueryExecutionConverters // + .getExecutionAdapter(method.getReturnType()); + + if (executionAdapter == null) { + return resultHandler.postProcessInvocationResult(doInvoke(invocation), method); + } + + return executionAdapter // + .apply(() -> resultHandler.postProcessInvocationResult(doInvoke(invocation), method)); + } + + @Nullable + private Object doInvoke(MethodInvocation invocation) throws Throwable { + + Method method = invocation.getMethod(); + + if (hasQueryFor(method)) { + + QueryMethodInvoker invocationMetadata = invocationMetadataCache.get(method); + + if (invocationMetadata == null) { + invocationMetadata = new QueryMethodInvoker(method); + invocationMetadataCache.put(method, invocationMetadata); + } + + RepositoryQuery repositoryQuery = queries.get(method); + return invocationMetadata.invoke(repositoryQuery, invocation.getArguments()); + } + + return invocation.proceed(); + } + + /** + * Returns whether we know of a query to execute for the given {@link Method}; + * + * @param method + * @return + */ + private boolean hasQueryFor(Method method) { + return queries.containsKey(method); + } + + /** + * Invoker for Query Methods. Considers + */ + static class QueryMethodInvoker { + + private final boolean suspendedDeclaredMethod; + private final Class returnedType; + private final boolean returnsReactiveType; + + QueryMethodInvoker(Method invokedMethod) { + + if (KotlinDetector.isKotlinReflectPresent()) { + + this.suspendedDeclaredMethod = KotlinReflectionUtils.isSuspend(invokedMethod); + this.returnedType = this.suspendedDeclaredMethod ? KotlinReflectionUtils.getReturnType(invokedMethod) + : invokedMethod.getReturnType(); + } else { + + this.suspendedDeclaredMethod = false; + this.returnedType = invokedMethod.getReturnType(); + } + + this.returnsReactiveType = ReactiveWrappers.supports(returnedType); + } + + @Nullable + public Object invoke(RepositoryQuery query, Object[] args) { + return suspendedDeclaredMethod ? invokeReactiveToSuspend(query, args) : query.execute(args); + } + + @Nullable + @SuppressWarnings({ "unchecked", "ConstantConditions" }) + private Object invokeReactiveToSuspend(RepositoryQuery query, Object[] args) { + + /* + * Kotlin suspended functions are invoked with a synthetic Continuation parameter that keeps track of the Coroutine context. + * We're invoking a method without Continuation as we expect the method to return any sort of reactive type, + * therefore we need to strip the Continuation parameter. + */ + Continuation continuation = (Continuation) args[args.length - 1]; + args[args.length - 1] = null; + Object result = query.execute(args); + + if (returnsReactiveType) { + return ReactiveWrapperConverters.toWrapper(result, returnedType); + } + + Publisher publisher = result instanceof Publisher ? (Publisher) result + : ReactiveWrapperConverters.toWrapper(result, Publisher.class); + + return AwaitKt.awaitFirstOrNull(publisher, continuation); + } + } +} diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java index c60d848b0..c2e122d4a 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java +++ b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java @@ -15,9 +15,6 @@ */ package org.springframework.data.repository.core.support; -import kotlin.coroutines.Continuation; -import kotlin.reflect.KFunction; -import kotlinx.coroutines.reactive.AwaitKt; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NonNull; @@ -29,7 +26,6 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; @@ -38,7 +34,6 @@ import java.util.stream.Collectors; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; -import org.reactivestreams.Publisher; import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.interceptor.ExposeInvocationInterceptor; @@ -47,8 +42,6 @@ import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; -import org.springframework.core.KotlinDetector; -import org.springframework.core.ResolvableType; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor; @@ -69,8 +62,6 @@ import org.springframework.data.repository.util.ClassUtils; import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.data.repository.util.ReactiveWrapperConverters; import org.springframework.data.repository.util.ReactiveWrappers; -import org.springframework.data.util.KotlinReflectionUtils; -import org.springframework.data.util.Pair; import org.springframework.data.util.ReflectionUtils; import org.springframework.lang.Nullable; import org.springframework.transaction.interceptor.TransactionalProxy; @@ -335,7 +326,10 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware, } ProjectionFactory projectionFactory = getProjectionFactory(classLoader, beanFactory); - result.addAdvice(new QueryExecutorMethodInterceptor(information, projectionFactory)); + Optional queryLookupStrategy = getQueryLookupStrategy(queryLookupStrategyKey, + evaluationContextProvider); + result.addAdvice(new QueryExecutorMethodInterceptor(information, projectionFactory, queryLookupStrategy, + namedQueries, queryPostProcessors)); composition = composition.append(RepositoryFragment.implemented(target)); result.addAdvice(new ImplementationMethodExecutionInterceptor(composition)); @@ -530,124 +524,6 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware, baseClass, Arrays.stream(constructorArguments).map(Object::getClass).collect(Collectors.toList())))); } - /** - * This {@code MethodInterceptor} intercepts calls to methods of the custom implementation and delegates the to it if - * configured. Furthermore it resolves method calls to finders and triggers execution of them. You can rely on having - * a custom repository implementation instance set if this returns true. - * - * @author Oliver Gierke - */ - public class QueryExecutorMethodInterceptor implements MethodInterceptor { - - private final Map queries; - private final Map invocationMetadataCache = new ConcurrentReferenceHashMap<>(); - private final QueryExecutionResultHandler resultHandler; - - /** - * Creates a new {@link QueryExecutorMethodInterceptor}. Builds a model of {@link QueryMethod}s to be invoked on - * execution of repository interface methods. - */ - public QueryExecutorMethodInterceptor(RepositoryInformation repositoryInformation, - ProjectionFactory projectionFactory) { - - this.resultHandler = new QueryExecutionResultHandler(CONVERSION_SERVICE); - - Optional lookupStrategy = getQueryLookupStrategy(queryLookupStrategyKey, - RepositoryFactorySupport.this.evaluationContextProvider); - - if (!lookupStrategy.isPresent() && repositoryInformation.hasQueryMethods()) { - - throw new IllegalStateException("You have defined query method in the repository but " - + "you don't have any query lookup strategy defined. The " - + "infrastructure apparently does not support query methods!"); - } - - this.queries = lookupStrategy // - .map(it -> mapMethodsToQuery(repositoryInformation, it, projectionFactory)) // - .orElse(Collections.emptyMap()); - } - - private Map mapMethodsToQuery(RepositoryInformation repositoryInformation, - QueryLookupStrategy lookupStrategy, ProjectionFactory projectionFactory) { - - return repositoryInformation.getQueryMethods().stream() // - .map(method -> lookupQuery(method, repositoryInformation, lookupStrategy, projectionFactory)) // - .peek(pair -> invokeListeners(pair.getSecond())) // - .collect(Pair.toMap()); - } - - private Pair lookupQuery(Method method, RepositoryInformation information, - QueryLookupStrategy strategy, ProjectionFactory projectionFactory) { - return Pair.of(method, strategy.resolveQuery(method, information, projectionFactory, namedQueries)); - } - - @SuppressWarnings({ "rawtypes", "unchecked" }) - private void invokeListeners(RepositoryQuery query) { - - for (QueryCreationListener listener : queryPostProcessors) { - - ResolvableType typeArgument = ResolvableType.forClass(QueryCreationListener.class, listener.getClass()) - .getGeneric(0); - - if (typeArgument != null && typeArgument.isAssignableFrom(ResolvableType.forClass(query.getClass()))) { - listener.onCreation(query); - } - } - } - - /* - * (non-Javadoc) - * @see org.aopalliance.intercept.MethodInterceptor#invoke(org.aopalliance.intercept.MethodInvocation) - */ - @Override - @Nullable - public Object invoke(@SuppressWarnings("null") MethodInvocation invocation) throws Throwable { - - Method method = invocation.getMethod(); - - QueryExecutionConverters.ExecutionAdapter executionAdapter = QueryExecutionConverters // - .getExecutionAdapter(method.getReturnType()); - - if (executionAdapter == null) { - return resultHandler.postProcessInvocationResult(doInvoke(invocation), method); - } - - return executionAdapter // - .apply(() -> resultHandler.postProcessInvocationResult(doInvoke(invocation), method)); - } - - @Nullable - private Object doInvoke(MethodInvocation invocation) throws Throwable { - - Method method = invocation.getMethod(); - - if (hasQueryFor(method)) { - - QueryMethodInvoker invocationMetadata = invocationMetadataCache.get(method); - - if (invocationMetadata == null) { - invocationMetadata = new QueryMethodInvoker(method); - invocationMetadataCache.put(method, invocationMetadata); - } - - RepositoryQuery repositoryQuery = queries.get(method); - return invocationMetadata.invoke(repositoryQuery, invocation.getArguments()); - } - - return invocation.proceed(); - } - - /** - * Returns whether we know of a query to execute for the given {@link Method}; - * - * @param method - * @return - */ - private boolean hasQueryFor(Method method) { - return queries.containsKey(method); - } - } - /** * Method interceptor that calls methods on the {@link RepositoryComposition}. * @@ -727,55 +603,4 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware, this.compositionHash = composition.hashCode(); } } - - static class QueryMethodInvoker { - - private final boolean suspendedDeclaredMethod; - private final Class returnedType; - private final boolean returnsReactiveType; - - QueryMethodInvoker(Method invokedMethod) { - - this.suspendedDeclaredMethod = KotlinDetector.isKotlinReflectPresent() && isSuspendedMethod(invokedMethod); - this.returnedType = this.suspendedDeclaredMethod ? KotlinReflectionUtils.getReturnType(invokedMethod) - : invokedMethod.getReturnType(); - this.returnsReactiveType = ReactiveWrappers.supports(returnedType); - } - - private static boolean isSuspendedMethod(Method invokedMethod) { - - KFunction invokedFunction = ReflectionUtils.isKotlinClass(invokedMethod.getDeclaringClass()) - ? KotlinReflectionUtils.findKotlinFunction(invokedMethod) - : null; - - return invokedFunction != null && invokedFunction.isSuspend(); - } - - @Nullable - public Object invoke(RepositoryQuery query, Object[] args) { - return suspendedDeclaredMethod ? invokeReactiveToSuspend(query, args) : query.execute(args); - } - - @Nullable - @SuppressWarnings({ "unchecked", "ConstantConditions" }) - private Object invokeReactiveToSuspend(RepositoryQuery query, Object[] args) { - - Continuation continuation = (Continuation) args[args.length - 1]; - args[args.length - 1] = null; - Object result = query.execute(args); - - if (returnsReactiveType) { - return ReactiveWrapperConverters.toWrapper(result, returnedType); - } - - Publisher publisher; - if (result instanceof Publisher) { - publisher = (Publisher) result; - } else { - publisher = ReactiveWrapperConverters.toWrapper(result, Publisher.class); - } - - return AwaitKt.awaitFirstOrNull(publisher, continuation); - } - } } diff --git a/src/main/java/org/springframework/data/repository/query/Parameter.java b/src/main/java/org/springframework/data/repository/query/Parameter.java index 7e7e61cbc..04c21ba9f 100644 --- a/src/main/java/org/springframework/data/repository/query/Parameter.java +++ b/src/main/java/org/springframework/data/repository/query/Parameter.java @@ -28,12 +28,12 @@ import org.springframework.core.MethodParameter; import org.springframework.core.ResolvableType; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; +import org.springframework.data.repository.util.ClassUtils; import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.Lazy; import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; /** * Class to abstract a single parameter of a query method. It is held in the context of a {@link Parameters} instance. @@ -58,13 +58,10 @@ public class Parameter { List> types = new ArrayList<>(Arrays.asList(Pageable.class, Sort.class)); - try { - // consider Kotlin Coroutines Continuation a special parameter. That parameter is synthetic and should not get - // bound to any query. - types.add(ClassUtils.forName("kotlin.coroutines.Continuation", Parameter.class.getClassLoader())); - } catch (ClassNotFoundException e) { - // ignore - } + // consider Kotlin Coroutines Continuation a special parameter. That parameter is synthetic and should not get + // bound to any query. + + ClassUtils.ifPresent("kotlin.coroutines.Continuation", Parameter.class.getClassLoader(), types::add); TYPES = Collections.unmodifiableList(types); } diff --git a/src/main/java/org/springframework/data/repository/util/ClassUtils.java b/src/main/java/org/springframework/data/repository/util/ClassUtils.java index 2323e8ebe..f10bd1f0a 100644 --- a/src/main/java/org/springframework/data/repository/util/ClassUtils.java +++ b/src/main/java/org/springframework/data/repository/util/ClassUtils.java @@ -19,6 +19,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Arrays; import java.util.Collection; +import java.util.function.Consumer; import org.springframework.data.repository.Repository; import org.springframework.data.util.ClassTypeInformation; @@ -32,6 +33,7 @@ import org.springframework.util.StringUtils; * Utility class to work with classes. * * @author Oliver Gierke + * @author Mark Paluch */ public abstract class ClassUtils { @@ -56,6 +58,30 @@ public abstract class ClassUtils { return null != ReflectionUtils.findField(type, StringUtils.uncapitalize(property)); } + /** + * Determine whether the {@link Class} identified by the supplied {@code className} is present * and can be loaded and + * call the {@link Consumer action} if the {@link Class} could be loaded. + * + * @param className the name of the class to check. + * @param classLoader the class loader to use. + * @param action the action callback to notify. (may be {@code null} which indicates the default class loader) + * @throws IllegalStateException if the corresponding class is resolvable but there was a readability mismatch in the + * inheritance hierarchy of the class (typically a missing dependency declaration in a Jigsaw module + * definition for a superclass or interface implemented by the class to be checked here) + */ + public static void ifPresent(String className, @Nullable ClassLoader classLoader, Consumer> action) { + + try { + Class theClass = org.springframework.util.ClassUtils.forName(className, classLoader); + action.accept(theClass); + } catch (IllegalAccessError err) { + throw new IllegalStateException( + "Readability mismatch in inheritance hierarchy of class [" + className + "]: " + err.getMessage(), err); + } catch (Throwable ex) { + // Typically ClassNotFoundException or NoClassDefFoundError... + } + } + /** * Returns wthere the given type is the {@link Repository} interface. * diff --git a/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java b/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java index b4704eb34..8ec5ff3f1 100644 --- a/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java +++ b/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 the original author or authors. + * Copyright 2019-2020 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. @@ -26,10 +26,13 @@ import kotlin.reflect.jvm.KTypesJvm; import kotlin.reflect.jvm.ReflectJvmMapping; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.Optional; import java.util.stream.Stream; +import org.springframework.core.KotlinDetector; import org.springframework.core.MethodParameter; +import org.springframework.core.annotation.AnnotationUtils; import org.springframework.lang.Nullable; /** @@ -38,11 +41,32 @@ import org.springframework.lang.Nullable; * * @author Mark Paluch * @since 2.3 + * @see org.springframework.core.KotlinDetector#isKotlinReflectPresent() */ public final class KotlinReflectionUtils { + private static final int KOTLIN_KIND_CLASS = 1; + private KotlinReflectionUtils() {} + /** + * Return {@literal true} if the specified class is a supported Kotlin class. Currently supported are only regular + * Kotlin classes. Other class types (synthetic, SAM, lambdas) are not supported via reflection. + * + * @return {@literal true} if {@code type} is a supported Kotlin class. + */ + public static boolean isSupportedKotlinClass(Class type) { + + if (!KotlinDetector.isKotlinType(type)) { + return false; + } + + return Arrays.stream(type.getDeclaredAnnotations()) // + .filter(annotation -> annotation.annotationType().getName().equals("kotlin.Metadata")) // + .map(annotation -> AnnotationUtils.getValue(annotation, "k")) // + .anyMatch(it -> Integer.valueOf(KOTLIN_KIND_CLASS).equals(it)); + } + /** * Returns a {@link KFunction} instance corresponding to the given Java {@link Method} instance, or {@code null} if * this method cannot be represented by a Kotlin function. @@ -55,14 +79,24 @@ public final class KotlinReflectionUtils { KFunction kotlinFunction = ReflectJvmMapping.getKotlinFunction(method); - if (kotlinFunction == null) { + // Fallback to own lookup because there's no public Kotlin API for that kind of lookup until + // https://youtrack.jetbrains.com/issue/KT-20768 gets resolved. + return kotlinFunction == null ? findKFunction(method).orElse(null) : kotlinFunction; + } - // Fallback to own lookup because there's no public Kotlin API for that kind of lookup until - // https://youtrack.jetbrains.com/issue/KT-20768 gets resolved. - return findKFunction(method).orElse(null); - } + /** + * Returns whether the {@link Method} is declared as suspend (Kotlin Coroutine). + * + * @param method the method to inspect. + * @return {@literal true} if the method is declared as suspend. + * @see KFunction#isSuspend() + */ + public static boolean isSuspend(Method method) { - return kotlinFunction; + KFunction invokedFunction = KotlinDetector.isKotlinType(method.getDeclaringClass()) ? findKotlinFunction(method) + : null; + + return invokedFunction != null && invokedFunction.isSuspend(); } /** diff --git a/src/main/java/org/springframework/data/util/ReflectionUtils.java b/src/main/java/org/springframework/data/util/ReflectionUtils.java index f4a83ad7b..828a264dc 100644 --- a/src/main/java/org/springframework/data/util/ReflectionUtils.java +++ b/src/main/java/org/springframework/data/util/ReflectionUtils.java @@ -31,6 +31,7 @@ import java.util.stream.IntStream; import java.util.stream.Stream; import org.springframework.beans.BeanUtils; +import org.springframework.core.KotlinDetector; import org.springframework.core.MethodParameter; import org.springframework.core.ResolvableType; import org.springframework.core.annotation.AnnotationUtils; @@ -51,10 +52,6 @@ import org.springframework.util.ReflectionUtils.FieldFilter; @UtilityClass public class ReflectionUtils { - private static final boolean KOTLIN_IS_PRESENT = ClassUtils.isPresent("kotlin.Unit", - BeanUtils.class.getClassLoader()); - private static final int KOTLIN_KIND_CLASS = 1; - /** * Creates an instance of the class with the given fully qualified name or returns the given default instance if the * class cannot be loaded or instantiated. @@ -350,12 +347,11 @@ public class ReflectionUtils { * * @return {@literal true} if {@code type} is a Kotlin class. * @since 2.0 + * @deprecated since 2.3, use {@link KotlinDetector#isKotlinType(Class)} instead. */ + @Deprecated public static boolean isKotlinClass(Class type) { - - return KOTLIN_IS_PRESENT && Arrays.stream(type.getDeclaredAnnotations()) // - .map(Annotation::annotationType) // - .anyMatch(annotation -> annotation.getName().equals("kotlin.Metadata")); + return KotlinDetector.isKotlinType(type); } /** @@ -364,17 +360,11 @@ public class ReflectionUtils { * * @return {@literal true} if {@code type} is a supported Kotlin class. * @since 2.0 + * @deprecated since 2.3, use {@link KotlinReflectionUtils#isSupportedKotlinClass(Class)} instead. */ + @Deprecated public static boolean isSupportedKotlinClass(Class type) { - - if (!isKotlinClass(type)) { - return false; - } - - return Arrays.stream(type.getDeclaredAnnotations()) // - .filter(annotation -> annotation.annotationType().getName().equals("kotlin.Metadata")) // - .map(annotation -> AnnotationUtils.getValue(annotation, "k")) // - .anyMatch(it -> Integer.valueOf(KOTLIN_KIND_CLASS).equals(it)); + return KotlinReflectionUtils.isSupportedKotlinClass(type); } /** diff --git a/src/main/kotlin/org/springframework/data/repository/kotlin/CoCrudRepository.kt b/src/main/kotlin/org/springframework/data/repository/kotlin/CoroutineCrudRepository.kt similarity index 98% rename from src/main/kotlin/org/springframework/data/repository/kotlin/CoCrudRepository.kt rename to src/main/kotlin/org/springframework/data/repository/kotlin/CoroutineCrudRepository.kt index 4b3b08d38..69309fe96 100644 --- a/src/main/kotlin/org/springframework/data/repository/kotlin/CoCrudRepository.kt +++ b/src/main/kotlin/org/springframework/data/repository/kotlin/CoroutineCrudRepository.kt @@ -28,7 +28,7 @@ import reactor.core.publisher.Mono * @see Flow */ @NoRepositoryBean -interface CoCrudRepository : Repository { +interface CoroutineCrudRepository : Repository { /** * Saves a given entity. Use the returned instance for further operations as the save operation might have changed the @@ -86,11 +86,7 @@ interface CoCrudRepository : Repository { /** * Returns all instances of the type `T` with the given IDs. - * - * * If some or all ids are not found, no entities are returned for these IDs. - * - * * Note that the order of elements in the result is not guaranteed. * * @param ids must not be null nor contain any null values. diff --git a/src/main/kotlin/org/springframework/data/repository/kotlin/CoSortingRepository.kt b/src/main/kotlin/org/springframework/data/repository/kotlin/CoroutineSortingRepository.kt similarity index 86% rename from src/main/kotlin/org/springframework/data/repository/kotlin/CoSortingRepository.kt rename to src/main/kotlin/org/springframework/data/repository/kotlin/CoroutineSortingRepository.kt index e7476b9bb..813bb7f2f 100644 --- a/src/main/kotlin/org/springframework/data/repository/kotlin/CoSortingRepository.kt +++ b/src/main/kotlin/org/springframework/data/repository/kotlin/CoroutineSortingRepository.kt @@ -20,7 +20,7 @@ import org.springframework.data.domain.Sort import org.springframework.data.repository.NoRepositoryBean /** - * Extension of [CoCrudRepository] to provide additional methods to retrieve entities using the sorting + * Extension of [CoroutineCrudRepository] to provide additional methods to retrieve entities using the sorting * abstraction. * * @author Mark Paluch @@ -29,7 +29,7 @@ import org.springframework.data.repository.NoRepositoryBean * @see Sort */ @NoRepositoryBean -interface CoSortingRepository : CoCrudRepository { +interface CoroutineSortingRepository : CoroutineCrudRepository { /** * Returns all entities sorted by the given options. diff --git a/src/test/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptorUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptorUnitTests.java index 54d3781c7..15cc8dd62 100755 --- a/src/test/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptorUnitTests.java +++ b/src/test/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptorUnitTests.java @@ -18,6 +18,7 @@ package org.springframework.data.repository.core.support; import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; +import java.util.Collections; import java.util.Optional; import org.junit.Test; @@ -28,10 +29,9 @@ import org.mockito.junit.MockitoJUnitRunner; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.query.QueryLookupStrategy; -import org.springframework.data.util.Streamable; /** - * Unit test for {@link QueryExecuterMethodInterceptor}. + * Unit test for {@link QueryExecutorMethodInterceptor}. * * @author Oliver Gierke * @author Mark Paluch @@ -40,27 +40,24 @@ import org.springframework.data.util.Streamable; @RunWith(MockitoJUnitRunner.class) public class QueryExecutorMethodInterceptorUnitTests { - @Mock RepositoryFactorySupport factory; @Mock RepositoryInformation information; @Mock QueryLookupStrategy strategy; - @Test + @Test // DATACMNS-1508 public void rejectsRepositoryInterfaceWithQueryMethodsIfNoQueryLookupStrategyIsDefined() { when(information.hasQueryMethods()).thenReturn(true); - when(factory.getQueryLookupStrategy(any(), any())).thenReturn(Optional.empty()); - assertThatIllegalStateException().isThrownBy( - () -> factory.new QueryExecutorMethodInterceptor(information, new SpelAwareProxyProjectionFactory())); + assertThatIllegalStateException() + .isThrownBy(() -> new QueryExecutorMethodInterceptor(information, new SpelAwareProxyProjectionFactory(), + Optional.empty(), PropertiesBasedNamedQueries.EMPTY, Collections.emptyList())); } - @Test + @Test // DATACMNS-1508 public void skipsQueryLookupsIfQueryLookupStrategyIsNotPresent() { - when(information.getQueryMethods()).thenReturn(Streamable.empty()); - when(factory.getQueryLookupStrategy(any(), any())).thenReturn(Optional.of(strategy)); - - factory.new QueryExecutorMethodInterceptor(information, new SpelAwareProxyProjectionFactory()); + new QueryExecutorMethodInterceptor(information, new SpelAwareProxyProjectionFactory(), Optional.empty(), + PropertiesBasedNamedQueries.EMPTY, Collections.emptyList()); verify(strategy, times(0)).resolveQuery(any(), any(), any(), any()); } diff --git a/src/test/java/org/springframework/data/util/KotlinReflectionUtilsUnitTests.java b/src/test/java/org/springframework/data/util/KotlinReflectionUtilsUnitTests.java new file mode 100644 index 000000000..6ace055bc --- /dev/null +++ b/src/test/java/org/springframework/data/util/KotlinReflectionUtilsUnitTests.java @@ -0,0 +1,79 @@ +/* + * Copyright 2020 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 + * + * https://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.util; + +import static org.assertj.core.api.Assertions.*; + +import java.net.URL; +import java.net.URLClassLoader; + +import org.junit.Test; + +import org.springframework.core.KotlinDetector; +import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; + +/** + * Unit tests for {@link KotlinReflectionUtilsUnitTests}. + * + * @author Mark Paluch + */ +public class KotlinReflectionUtilsUnitTests { + + @Test // DATACMNS-1508 + public void classShouldLoadWithKotlin() { + assertThat(KotlinDetector.isKotlinPresent()).isTrue(); + assertThat(KotlinReflectionUtils.isSupportedKotlinClass(TypeCreatingSyntheticClass.class)).isTrue(); + } + + @Test // DATACMNS-1508 + public void classShouldLoadWithoutKotlin() throws Exception { + runTest("loadClassWithoutKotlin"); + } + + // executed via reflection in the context of a ClassLoader without Kotlin dependencies. + public void loadClassWithoutKotlin() { + + assertThat(KotlinDetector.isKotlinPresent()).isFalse(); + assertThat(KotlinReflectionUtils.isSupportedKotlinClass(TypeCreatingSyntheticClass.class)).isFalse(); + } + + protected void runTest(String testName) + throws ClassNotFoundException, InstantiationException, IllegalAccessException, NoSuchMethodException { + + KotlinExcludingURLClassLoader classLoader = new KotlinExcludingURLClassLoader( + ((URLClassLoader) getClass().getClassLoader()).getURLs()); + Class testClass = ClassUtils.forName(getClass().getName(), classLoader); + + ReflectionUtils.invokeMethod(testClass.getMethod(testName), testClass.newInstance()); + } + + static class KotlinExcludingURLClassLoader extends URLClassLoader { + public KotlinExcludingURLClassLoader(URL[] urls) { + super(urls, null); + } + + @Override + protected Class findClass(String name) throws ClassNotFoundException { + + if (name.startsWith("kotlin")) { + throw new ClassNotFoundException("Denied: " + name); + } + + return super.findClass(name); + } + } +} diff --git a/src/test/kotlin/org/springframework/data/repository/kotlin/CoCrudRepositoryCustomImplementationUnitTests.kt b/src/test/kotlin/org/springframework/data/repository/kotlin/CoroutineCrudRepositoryCustomImplementationUnitTests.kt similarity index 94% rename from src/test/kotlin/org/springframework/data/repository/kotlin/CoCrudRepositoryCustomImplementationUnitTests.kt rename to src/test/kotlin/org/springframework/data/repository/kotlin/CoroutineCrudRepositoryCustomImplementationUnitTests.kt index 1e10928a6..7343df9f6 100644 --- a/src/test/kotlin/org/springframework/data/repository/kotlin/CoCrudRepositoryCustomImplementationUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/repository/kotlin/CoroutineCrudRepositoryCustomImplementationUnitTests.kt @@ -32,7 +32,7 @@ import org.springframework.data.repository.sample.User * * @author Mark Paluch */ -class CoCrudRepositoryCustomImplementationUnitTests { +class CoroutineCrudRepositoryCustomImplementationUnitTests { val backingRepository = mockk>() lateinit var factory: DummyReactiveRepositoryFactory; @@ -61,7 +61,7 @@ class CoCrudRepositoryCustomImplementationUnitTests { } } - interface MyCoRepository : CoCrudRepository, MyCustomCoRepository + interface MyCoRepository : CoroutineCrudRepository, MyCustomCoRepository interface MyCustomCoRepository { diff --git a/src/test/kotlin/org/springframework/data/repository/kotlin/CoCrudRepositoryUnitTests.kt b/src/test/kotlin/org/springframework/data/repository/kotlin/CoroutineCrudRepositoryUnitTests.kt similarity index 97% rename from src/test/kotlin/org/springframework/data/repository/kotlin/CoCrudRepositoryUnitTests.kt rename to src/test/kotlin/org/springframework/data/repository/kotlin/CoroutineCrudRepositoryUnitTests.kt index 905c99fbf..519d07875 100644 --- a/src/test/kotlin/org/springframework/data/repository/kotlin/CoCrudRepositoryUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/repository/kotlin/CoroutineCrudRepositoryUnitTests.kt @@ -37,7 +37,7 @@ import rx.Single * * @author Mark Paluch */ -class CoCrudRepositoryUnitTests { +class CoroutineCrudRepositoryUnitTests { val backingRepository = mockk>() lateinit var factory: DummyReactiveRepositoryFactory; @@ -159,7 +159,7 @@ class CoCrudRepositoryUnitTests { assertThat(emptyResult).isEmpty() } - interface MyCoRepository : CoCrudRepository { + interface MyCoRepository : CoroutineCrudRepository { suspend fun findOne(id: String): User