fix: Make JpaFilterConverter work without an EntityManager (#3047)

Fixes #2569
vaadin · Dec 19, 2024 · c1d3131 · c1d3131
1 parent 05237a3
commit c1d3131
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 91 deletions.
diff --git a/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/CrudConfiguration.java b/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/CrudConfiguration.java
diff --git a/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/JpaFilterConverter.java b/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/JpaFilterConverter.java
@@ -1,36 +1,27 @@
 package com.vaadin.hilla.crud;
 
-import jakarta.persistence.EntityManager;
+import org.springframework.data.jpa.domain.Specification;
 
 import com.vaadin.hilla.crud.filter.AndFilter;
 import com.vaadin.hilla.crud.filter.Filter;
 import com.vaadin.hilla.crud.filter.OrFilter;
 import com.vaadin.hilla.crud.filter.PropertyStringFilter;
-import jakarta.persistence.criteria.Path;
-import jakarta.persistence.criteria.Root;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.data.jpa.domain.Specification;
-import org.springframework.stereotype.Component;
 
 /**
  * Utility class for converting Hilla {@link Filter} specifications into JPA
  * filter specifications. This class can be used to implement filtering for
  * custom {@link ListService} or {@link CrudService} implementations that use
  * JPA as the data source.
- * <p>
- * This class requires an EntityManager to be available as a Spring bean and
- * thus should be injected into the bean that wants to use the converter.
- * Manually creating new instances of this class will not work.
  */
-@Component
-public class JpaFilterConverter {
+public final class JpaFilterConverter {
 
-    @Autowired
-    private EntityManager em;
+    private JpaFilterConverter() {
+        // Utils only
+    }
 
     /**
-     * Converts the given Hilla filter specification into a JPA filter
-     * specification for the specified entity class.
+     * Converts the given filter specification into a JPA filter specification
+     * for the specified entity class.
      * <p>
      * If the filter contains {@link PropertyStringFilter} instances, their
      * properties, or nested property paths, need to match the structure of the
@@ -45,7 +36,8 @@ public class JpaFilterConverter {
      *            the entity class
      * @return a JPA filter specification for the given filter
      */
-    public <T> Specification<T> toSpec(Filter rawFilter, Class<T> entity) {
+    public static <T> Specification<T> toSpec(Filter rawFilter,
+            Class<T> entity) {
         if (rawFilter == null) {
             return Specification.anyOf();
         }
@@ -56,35 +48,10 @@ public <T> Specification<T> toSpec(Filter rawFilter, Class<T> entity) {
             return Specification.anyOf(filter.getChildren().stream()
                     .map(f -> toSpec(f, entity)).toList());
         } else if (rawFilter instanceof PropertyStringFilter filter) {
-            Class<?> javaType = extractPropertyJavaType(entity,
-                    filter.getPropertyId());
-            return new PropertyStringFilterSpecification<>(filter, javaType);
+            return new PropertyStringFilterSpecification<>(filter);
         } else {
-            if (rawFilter != null) {
-                throw new IllegalArgumentException("Unknown filter type "
-                        + rawFilter.getClass().getName());
-            }
-            return Specification.anyOf();
+            throw new IllegalArgumentException(
+                    "Unknown filter type " + rawFilter.getClass().getName());
         }
     }
-
-    private Class<?> extractPropertyJavaType(Class<?> entity,
-            String propertyId) {
-        if (propertyId.contains(".")) {
-            String[] parts = propertyId.split("\\.");
-            Root<?> root = em.getCriteriaBuilder().createQuery(entity)
-                    .from(entity);
-            Path<?> path = root.get(parts[0]);
-            int i = 1;
-            while (i < parts.length) {
-                path = path.get(parts[i]);
-                i++;
-            }
-            return path.getJavaType();
-        } else {
-            return em.getMetamodel().entity(entity).getAttribute(propertyId)
-                    .getJavaType();
-        }
-    }
-
 }
diff --git a/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/ListRepositoryService.java b/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/ListRepositoryService.java
@@ -25,9 +25,6 @@
 public class ListRepositoryService<T, ID, R extends CrudRepository<T, ID> & JpaSpecificationExecutor<T>>
         implements ListService<T>, GetService<T, ID>, CountService {
 
-    @Autowired
-    private JpaFilterConverter jpaFilterConverter;
-
     @Autowired
     private ApplicationContext applicationContext;
 
@@ -105,7 +102,7 @@ public long count(@Nullable Filter filter) {
      * @return a JPA specification
      */
     protected Specification<T> toSpec(@Nullable Filter filter) {
-        return jpaFilterConverter.toSpec(filter, entityClass);
+        return JpaFilterConverter.toSpec(filter, entityClass);
     }
 
     @SuppressWarnings("unchecked")

diff --git a/.../java/endpoint/src/main/java/com/vaadin/hilla/crud/PropertyStringFilterSpecification.java b/.../java/endpoint/src/main/java/com/vaadin/hilla/crud/PropertyStringFilterSpecification.java
@@ -17,18 +17,16 @@
 public class PropertyStringFilterSpecification<T> implements Specification<T> {
 
     private final PropertyStringFilter filter;
-    private final Class<?> javaType;
 
-    public PropertyStringFilterSpecification(PropertyStringFilter filter,
-            Class<?> javaType) {
+    public PropertyStringFilterSpecification(PropertyStringFilter filter) {
         this.filter = filter;
-        this.javaType = javaType;
     }
 
     @Override
     public Predicate toPredicate(Root<T> root, CriteriaQuery<?> query,
             CriteriaBuilder criteriaBuilder) {
         String value = filter.getFilterValue();
+        Class<?> javaType = getJavaType(filter.getPropertyId(), root);
         Path<String> propertyPath = getPath(filter.getPropertyId(), root);
         if (javaType == String.class) {
             Expression<String> expr = criteriaBuilder.lower(propertyPath);
@@ -171,6 +169,21 @@ private Path<String> getPath(String propertyId, Root<T> root) {
         return path;
     }
 
+    private Class<?> getJavaType(String propertyId, Root<T> root) {
+        if (propertyId.contains(".")) {
+            String[] parts = propertyId.split("\\.");
+            Path<?> path = root.get(parts[0]);
+            int i = 1;
+            while (i < parts.length) {
+                path = path.get(parts[i]);
+                i++;
+            }
+            return path.getJavaType();
+        } else {
+            return root.get(propertyId).getJavaType();
+        }
+    }
+
     private boolean isBoolean(Class<?> javaType) {
         return javaType == boolean.class || javaType == Boolean.class;
     }

diff --git a/...esources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports b/...esources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports
@@ -1,7 +1,6 @@
 com.vaadin.hilla.EndpointController
 com.vaadin.hilla.push.PushConfigurer
 com.vaadin.hilla.ApplicationContextProvider
-com.vaadin.hilla.crud.CrudConfiguration
 com.vaadin.hilla.startup.EndpointRegistryInitializer
 com.vaadin.hilla.startup.RouteUnifyingServiceInitListener
 com.vaadin.hilla.route.RouteUtil

diff --git a/packages/java/endpoint/src/test/java/com/vaadin/hilla/crud/CrudRepositoryServiceTest.java b/packages/java/endpoint/src/test/java/com/vaadin/hilla/crud/CrudRepositoryServiceTest.java
@@ -1,8 +1,9 @@
 package com.vaadin.hilla.crud;
 
-import com.vaadin.hilla.BrowserCallable;
-import com.vaadin.hilla.EndpointController;
-import com.vaadin.hilla.push.PushConfigurer;
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Function;
+
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -18,12 +19,9 @@
 import org.springframework.data.repository.CrudRepository;
 import org.springframework.data.repository.query.FluentQuery;
 import org.springframework.stereotype.Repository;
-import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringRunner;
 
-import java.util.List;
-import java.util.Optional;
-import java.util.function.Function;
+import com.vaadin.hilla.BrowserCallable;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -35,7 +33,6 @@
         CrudRepositoryServiceTest.CustomCrudRepositoryService.class,
         CrudRepositoryServiceTest.CustomJpaRepository.class,
         CrudRepositoryServiceTest.CustomJpaRepositoryService.class })
-@ContextConfiguration(classes = { CrudConfiguration.class })
 @EnableAutoConfiguration
 public class CrudRepositoryServiceTest {
 

diff --git a/packages/java/endpoint/src/test/java/com/vaadin/hilla/crud/FilterTest.java b/packages/java/endpoint/src/test/java/com/vaadin/hilla/crud/FilterTest.java
@@ -31,9 +31,6 @@ public class FilterTest {
     @Autowired
     private TestRepository repository;
 
-    @Autowired
-    private JpaFilterConverter jpaFilterConverter;
-
     @Test
     public void filterStringPropertyUsingContains() {
         setupNames("Jack", "John", "Johnny", "Polly", "Josh");
@@ -304,7 +301,7 @@ public void filterUnknownEnumValue() {
         executeFilter(filter);
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test(expected = InvalidDataAccessApiUsageException.class)
     public void filterNonExistingProperty() {
         setupNames("Jack", "John", "Johnny", "Polly", "Josh");
         PropertyStringFilter filter = createFilter("foo", Matcher.EQUALS,
@@ -425,7 +422,7 @@ private void assertFilterResult(Filter filter, List<TestObject> result) {
     }
 
     private List<TestObject> executeFilter(Filter filter) {
-        Specification<TestObject> spec = jpaFilterConverter.toSpec(filter,
+        Specification<TestObject> spec = JpaFilterConverter.toSpec(filter,
                 TestObject.class);
         return repository.findAll(spec);
     }

diff --git a/packages/java/endpoint/src/test/java/com/vaadin/hilla/crud/TestApplication.java b/packages/java/endpoint/src/test/java/com/vaadin/hilla/crud/TestApplication.java
@@ -1,10 +1,7 @@
 package com.vaadin.hilla.crud;
 
 import org.springframework.boot.autoconfigure.SpringBootApplication;
-import org.springframework.context.annotation.Import;
-import org.springframework.test.context.ContextConfiguration;
 
 @SpringBootApplication
-@Import(CrudConfiguration.class)
 public class TestApplication {
 }
diff --git a/packages/java/endpoint/src/test/java/com/vaadin/hilla/crud/filter/FilterTransformerTest.java b/packages/java/endpoint/src/test/java/com/vaadin/hilla/crud/filter/FilterTransformerTest.java
@@ -26,9 +26,6 @@ public class FilterTransformerTest {
     @Autowired
     private TestRepository repository;
 
-    @Autowired
-    private JpaFilterConverter jpaFilterConverter;
-
     @Test
     public void testRemap() {
         var testObject = new TestObject();
@@ -88,7 +85,7 @@ public void testRemap() {
                 });
 
         var filter = transformer.apply(andFilter);
-        var spec = jpaFilterConverter.toSpec(filter, TestObject.class);
+        var spec = JpaFilterConverter.toSpec(filter, TestObject.class);
         var result = repository.findAll(spec);
         assertEquals(2, result.size());