From c1d3131d097f8f61b18fd679ccc46faef67a6a4c Mon Sep 17 00:00:00 2001 From: Artur Date: Thu, 19 Dec 2024 14:10:08 +0200 Subject: [PATCH] fix: Make JpaFilterConverter work without an EntityManager (#3047) Fixes #2569 --- .../vaadin/hilla/crud/CrudConfiguration.java | 17 ------ .../vaadin/hilla/crud/JpaFilterConverter.java | 57 ++++--------------- .../hilla/crud/ListRepositoryService.java | 5 +- .../PropertyStringFilterSpecification.java | 21 +++++-- ...ot.autoconfigure.AutoConfiguration.imports | 1 - .../hilla/crud/CrudRepositoryServiceTest.java | 13 ++--- .../com/vaadin/hilla/crud/FilterTest.java | 7 +-- .../vaadin/hilla/crud/TestApplication.java | 3 - .../crud/filter/FilterTransformerTest.java | 5 +- 9 files changed, 38 insertions(+), 91 deletions(-) delete mode 100644 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/CrudConfiguration.java b/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/CrudConfiguration.java deleted file mode 100644 index b67a3adff0..0000000000 --- a/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/CrudConfiguration.java +++ /dev/null @@ -1,17 +0,0 @@ -package com.vaadin.hilla.crud; - -import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.data.jpa.domain.Specification; - -@Configuration -public class CrudConfiguration { - - @Bean - @ConditionalOnClass(Specification.class) - JpaFilterConverter jpaFilterConverter() { - return new JpaFilterConverter(); - } - -} 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 index 2a343fc856..d3840b2404 100644 --- 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. - *

- * 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. *

* 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 Specification toSpec(Filter rawFilter, Class entity) { + public static Specification toSpec(Filter rawFilter, + Class entity) { if (rawFilter == null) { return Specification.anyOf(); } @@ -56,35 +48,10 @@ public Specification toSpec(Filter rawFilter, Class 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 index 1e2ae42e34..01539fc2c9 100644 --- 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 & JpaSpecificationExecutor> implements ListService, GetService, 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 toSpec(@Nullable Filter filter) { - return jpaFilterConverter.toSpec(filter, entityClass); + return JpaFilterConverter.toSpec(filter, entityClass); } @SuppressWarnings("unchecked") diff --git a/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/PropertyStringFilterSpecification.java b/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/PropertyStringFilterSpecification.java index 103a3b4fcf..e06c843f5e 100644 --- a/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/PropertyStringFilterSpecification.java +++ b/packages/java/endpoint/src/main/java/com/vaadin/hilla/crud/PropertyStringFilterSpecification.java @@ -17,18 +17,16 @@ public class PropertyStringFilterSpecification implements Specification { 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 root, CriteriaQuery query, CriteriaBuilder criteriaBuilder) { String value = filter.getFilterValue(); + Class javaType = getJavaType(filter.getPropertyId(), root); Path propertyPath = getPath(filter.getPropertyId(), root); if (javaType == String.class) { Expression expr = criteriaBuilder.lower(propertyPath); @@ -171,6 +169,21 @@ private Path getPath(String propertyId, Root root) { return path; } + private Class getJavaType(String propertyId, Root 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/packages/java/endpoint/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports b/packages/java/endpoint/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports index 924b966ac9..a90b4907b7 100644 --- a/packages/java/endpoint/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports +++ b/packages/java/endpoint/src/main/resources/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 index 707768599c..b1000167bc 100644 --- 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 index 034750dbe3..6d47fba0d5 100644 --- 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 result) { } private List executeFilter(Filter filter) { - Specification spec = jpaFilterConverter.toSpec(filter, + Specification 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 index 36bb2bbf6b..7f4c62e742 100644 --- 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 index fbf18d4ba3..182fbc2549 100644 --- 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());