Separate OperationPredicates between filtering a collection and acces…

…sing a single item Some filters should only be applied when filtering a collection, not when accessing a single item
xenit-eu · Aug 29, 2023 · 03160d5 · 03160d5
1 parent cc69327
commit 03160d5
Show file tree

Hide file tree

Showing 9 changed files with 53 additions and 27 deletions.
diff --git a/...tionFilteringOnlyOperationPredicates.java → ...llectionFilteringOperationPredicates.java b/...tionFilteringOnlyOperationPredicates.java → ...llectionFilteringOperationPredicates.java
@@ -12,26 +12,31 @@
  * box {@link RepositoryInvokerAdapterFactory}
  */
 @RequiredArgsConstructor
-public class CollectionFilteringOnlyOperationPredicates implements OperationPredicates {
+public class CollectionFilteringOperationPredicates implements OperationPredicates {
 
     private final Predicate predicate;
 
     @Override
     public OperationPredicates and(OperationPredicates predicate) {
-        if (predicate instanceof CollectionFilteringOnlyOperationPredicates) {
-            return new CollectionFilteringOnlyOperationPredicates(ExpressionUtils.and(
-                    this.readPredicate(),
-                    predicate.readPredicate()
+        if (predicate instanceof CollectionFilteringOperationPredicates) {
+            return new CollectionFilteringOperationPredicates(ExpressionUtils.and(
+                    this.collectionFilterPredicate(),
+                    predicate.collectionFilterPredicate()
             ));
         }
         return OperationPredicates.super.and(predicate);
     }
 
     @Override
-    public Predicate readPredicate() {
+    public Predicate collectionFilterPredicate() {
         return predicate;
     }
 
+    @Override
+    public Predicate readPredicate() {
+        return null;
+    }
+
     @Override
     public Predicate afterCreatePredicate() {
         return null;

diff --git a/.../thunx/spring/data/querydsl/predicate/injector/resolver/CompositeOperationPredicates.java b/.../thunx/spring/data/querydsl/predicate/injector/resolver/CompositeOperationPredicates.java
@@ -27,6 +27,11 @@ private Predicate combine(Function<OperationPredicates, Predicate> extractor) {
                 .orElse(null);
     }
 
+    @Override
+    public Predicate collectionFilterPredicate() {
+        return combine(OperationPredicates::collectionFilterPredicate);
+    }
+
     @Override
     public Predicate readPredicate() {
         return combine(OperationPredicates::readPredicate);

diff --git a/...ntentgrid/thunx/spring/data/querydsl/predicate/injector/resolver/OperationPredicates.java b/...ntentgrid/thunx/spring/data/querydsl/predicate/injector/resolver/OperationPredicates.java
@@ -20,7 +20,13 @@ default OperationPredicates and(OperationPredicates predicate) {
     }
 
     /**
-     * @return Predicate used for reading an entity
+     * @return Predicate used for filtering a collection of entities
+     */
+    @Nullable
+    Predicate collectionFilterPredicate();
+
+    /**
+     * @return Predicate used for reading a single entity
      */
     @Nullable
     Predicate readPredicate();

diff --git a/...pring/data/querydsl/predicate/injector/rest/webmvc/QuerydslBindingsPredicateResolver.java b/...pring/data/querydsl/predicate/injector/rest/webmvc/QuerydslBindingsPredicateResolver.java
@@ -2,7 +2,7 @@
 
 import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.resolver.QuerydslPredicateResolver;
 import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.resolver.OperationPredicates;
-import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.resolver.CollectionFilteringOnlyOperationPredicates;
+import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.resolver.CollectionFilteringOperationPredicates;
 import java.util.Arrays;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -47,7 +47,7 @@ public Optional<OperationPredicates> resolve(MethodParameter methodParameter, Cl
         var bindings = querydslBindingsFactory.createBindingsFor(domainTypeInfo);
 
         return Optional.of(predicateBuilder.getPredicate(domainTypeInfo, toMultiValueMap(parameters), bindings))
-                .map(CollectionFilteringOnlyOperationPredicates::new);
+                .map(CollectionFilteringOperationPredicates::new);
     }
 
     /**

diff --git a/...data/querydsl/predicate/injector/rest/webmvc/QuerydslRepositoryInvokerAdapterFactory.java b/...data/querydsl/predicate/injector/rest/webmvc/QuerydslRepositoryInvokerAdapterFactory.java
@@ -2,7 +2,7 @@
 
 import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.repository.RepositoryInvokerAdapterFactory;
 import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.resolver.OperationPredicates;
-import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.resolver.CollectionFilteringOnlyOperationPredicates;
+import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.resolver.CollectionFilteringOperationPredicates;
 import com.querydsl.core.types.Predicate;
 import lombok.RequiredArgsConstructor;
 import org.springframework.data.querydsl.QuerydslPredicateExecutor;
@@ -11,9 +11,8 @@
 import org.springframework.data.repository.support.RepositoryInvoker;
 
 /**
- * Default {@link RepositoryInvokerAdapterFactory} that only processes
- * {@link CollectionFilteringOnlyOperationPredicates} with the Spring Data REST
- * {@link QuerydslRepositoryInvokerAdapter}.
+ * Default {@link RepositoryInvokerAdapterFactory} that only processes {@link CollectionFilteringOperationPredicates}
+ * with the Spring Data REST {@link QuerydslRepositoryInvokerAdapter}.
  */
 @RequiredArgsConstructor
 public class QuerydslRepositoryInvokerAdapterFactory implements RepositoryInvokerAdapterFactory {
@@ -32,13 +31,13 @@ public RepositoryInvoker adaptRepositoryInvoker(RepositoryInvoker invoker, Class
     }
 
     private Predicate unwrapQuerydslPredicates(OperationPredicates operationPredicates) {
-        if (operationPredicates instanceof CollectionFilteringOnlyOperationPredicates) {
-            return operationPredicates.readPredicate();
+        if (operationPredicates instanceof CollectionFilteringOperationPredicates) {
+            return operationPredicates.collectionFilterPredicate();
         }
 
         throw new IllegalArgumentException(
                 "QuerydslRepositoryInvokerAdapter only supports %s, but received %s"
-                        .formatted(CollectionFilteringOnlyOperationPredicates.class, operationPredicates.getClass())
+                        .formatted(CollectionFilteringOperationPredicates.class, operationPredicates.getClass())
         );
     }
 }
diff --git a/...dicate/injector/rest/webmvc/SpringDataQuerydslPredicateInjectorAutoConfigurationTest.java b/...dicate/injector/rest/webmvc/SpringDataQuerydslPredicateInjectorAutoConfigurationTest.java
@@ -1,7 +1,7 @@
 package com.contentgrid.thunx.spring.data.querydsl.predicate.injector.rest.webmvc;
 
 import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.resolver.QuerydslPredicateResolver;
-import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.resolver.CollectionFilteringOnlyOperationPredicates;
+import com.contentgrid.thunx.spring.data.querydsl.predicate.injector.resolver.CollectionFilteringOperationPredicates;
 import com.querydsl.core.types.EntityPath;
 import jakarta.persistence.Basic;
 import jakarta.persistence.Entity;
@@ -26,7 +26,6 @@
 import org.springframework.boot.testcontainers.service.connection.ServiceConnection;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.Import;
 import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
 import org.springframework.data.querydsl.QuerydslPredicateExecutor;
 import org.springframework.data.querydsl.binding.QuerydslBinderCustomizer;
@@ -59,7 +58,7 @@ static class TestConfiguration {
         QuerydslPredicateResolver additionalPredicate() {
             return (methodParameter, domainType, parameters) -> {
                 if(parameters.containsKey("test") && domainType == TestEntity.class) {
-                    return Optional.of(new CollectionFilteringOnlyOperationPredicates(
+                    return Optional.of(new CollectionFilteringOperationPredicates(
                             QSpringDataQuerydslPredicateInjectorAutoConfigurationTest_TestEntity.testEntity
                                     .value
                                     .eq("entity2")

diff --git a/...ring/src/main/java/com/contentgrid/thunx/spring/data/querydsl/AllOperationPredicates.java b/...ring/src/main/java/com/contentgrid/thunx/spring/data/querydsl/AllOperationPredicates.java
@@ -6,8 +6,14 @@
 
 @RequiredArgsConstructor
 class AllOperationPredicates implements OperationPredicates {
+
     private final Predicate sharedPredicate;
 
+    @Override
+    public Predicate collectionFilterPredicate() {
+        return sharedPredicate;
+    }
+
     @Override
     public Predicate readPredicate() {
         return sharedPredicate;

diff --git a/...ng/src/main/java/com/contentgrid/thunx/spring/data/rest/AbacRepositoryInvokerAdapter.java b/...ng/src/main/java/com/contentgrid/thunx/spring/data/rest/AbacRepositoryInvokerAdapter.java
@@ -74,7 +74,7 @@ public AbacRepositoryInvokerAdapter(
             PathBuilder<?> pathBuilder,
             ConversionService conversionService
     ) {
-        super(delegate, executor, predicate.readPredicate());
+        super(delegate, executor, predicate.collectionFilterPredicate());
         this.executor = executor;
         this.predicate = predicate;
         this.transactionManager = transactionManager;

diff --git a/...rc/test/java/com/contentgrid/thunx/spring/data/rest/AbacRepositoryInvokerAdapterTest.java b/...rc/test/java/com/contentgrid/thunx/spring/data/rest/AbacRepositoryInvokerAdapterTest.java
@@ -64,6 +64,11 @@ private static class TestOperationPredicates implements OperationPredicates {
 
         private final Predicate predicate;
 
+        @Override
+        public Predicate collectionFilterPredicate() {
+            return predicate;
+        }
+
         @Override
         public Predicate readPredicate() {
             return predicate;
@@ -115,6 +120,7 @@ void invokeFindById_shouldRedirectToExecutor() {
             return true;
         }));
 
+        verify(predicate, atMostOnce()).collectionFilterPredicate();
         verify(predicate, atLeastOnce()).readPredicate();
         verifyNoMoreInteractions(predicate);
     }
@@ -144,7 +150,7 @@ void invokeSave_isNew_withoutId_predicateMatches() {
         verify(transactionManager).commit(any(TransactionStatus.class));
         verify(transactionManager, never()).rollback(any(TransactionStatus.class));
 
-        verify(predicate, atMostOnce()).readPredicate();
+        verify(predicate, atMostOnce()).collectionFilterPredicate();
         verify(predicate).afterCreatePredicate();
         verifyNoMoreInteractions(predicate);
     }
@@ -174,7 +180,7 @@ void invokeSave_isNew_withProvidedId_predicateMatches() {
         inOrderVerifier.verify(transactionManager).commit(any(TransactionStatus.class));
         inOrderVerifier.verifyNoMoreInteractions();
 
-        verify(predicate, atMostOnce()).readPredicate();
+        verify(predicate, atMostOnce()).collectionFilterPredicate();
         verify(predicate).afterCreatePredicate();
         verifyNoMoreInteractions(predicate);
     }
@@ -214,7 +220,7 @@ void invokeSave_isUpdate_predicateMatches() {
         inOrderVerifier.verify(transactionManager).commit(any(TransactionStatus.class));
         inOrderVerifier.verifyNoMoreInteractions();
 
-        verify(predicate, atMostOnce()).readPredicate();
+        verify(predicate, atMostOnce()).collectionFilterPredicate();
         verify(predicate).beforeUpdatePredicate();
         verify(predicate).afterUpdatePredicate();
         verifyNoMoreInteractions(predicate);
@@ -243,7 +249,7 @@ void invokeSave_isUpdate_preSavePredicateMismatch_shouldThrow() {
         inOrderVerifier.verify(transactionManager).rollback(any(TransactionStatus.class));
         inOrderVerifier.verifyNoMoreInteractions();
 
-        verify(predicate, atMostOnce()).readPredicate();
+        verify(predicate, atMostOnce()).collectionFilterPredicate();
         verify(predicate).beforeUpdatePredicate();
         verify(predicate, atMostOnce()).afterUpdatePredicate();
         verifyNoMoreInteractions(predicate);
@@ -277,7 +283,7 @@ void invokeSave_isUpdate_postSavePredicateMismatch_shouldThrow() {
         inOrderVerifier.verify(transactionManager).rollback(any(TransactionStatus.class));
         inOrderVerifier.verifyNoMoreInteractions();
 
-        verify(predicate, atMostOnce()).readPredicate();
+        verify(predicate, atMostOnce()).collectionFilterPredicate();
         verify(predicate).beforeUpdatePredicate();
         verify(predicate).afterUpdatePredicate();
         verifyNoMoreInteractions(predicate);
@@ -295,7 +301,7 @@ void invokeDeleteById_predicateMatches() {
 
         verify(delegate).invokeDeleteById(id);
 
-        verify(predicate, atMostOnce()).readPredicate();
+        verify(predicate, atMostOnce()).collectionFilterPredicate();
         verify(predicate).beforeDeletePredicate();
         verifyNoMoreInteractions(predicate);
     }
@@ -311,7 +317,7 @@ void invokeDeleteById_predicateMismatch() {
 
         verify(delegate, never()).invokeDeleteById(id);
 
-        verify(predicate, atMostOnce()).readPredicate();
+        verify(predicate, atMostOnce()).collectionFilterPredicate();
         verify(predicate).beforeDeletePredicate();
         verifyNoMoreInteractions(predicate);
     }