From 807118fbde1e988fef0005acfb18a07761e646cf Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 10 Nov 2017 11:34:52 +0100 Subject: [PATCH] DATAJPA-863 - Polishing. Moving from Hamcrest to AssertJ. Fixing JavaDoc. Removing superfluos explicite type arguments. Original pull request: #232. --- .../query/ParameterMetadataProvider.java | 33 +++++++------------ ...meterMetadataProviderIntegrationTests.java | 18 +++++----- .../PartTreeJpaQueryIntegrationTests.java | 25 ++++++++------ 3 files changed, 35 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java b/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java index e8718fc91..45489f471 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java @@ -99,7 +99,7 @@ class ParameterMetadataProvider { this.builder = builder; this.parameters = parameters.getBindableParameters().iterator(); - this.expressions = new ArrayList>(); + this.expressions = new ArrayList<>(); this.bindableParameterValues = bindableParameterValues; this.persistenceProvider = provider; } @@ -115,9 +115,6 @@ class ParameterMetadataProvider { /** * Builds a new {@link ParameterMetadata} for given {@link Part} and the next {@link Parameter}. - * - * @param - * @return */ @SuppressWarnings("unchecked") public ParameterMetadata next(Part part) { @@ -131,9 +128,9 @@ class ParameterMetadataProvider { * Builds a new {@link ParameterMetadata} of the given {@link Part} and type. Forwards the underlying * {@link Parameters} as well. * - * @param + * @param is the type parameter of the returend {@link ParameterMetadata}. * @param type must not be {@literal null}. - * @return + * @return ParameterMetadata for the next parameter. */ @SuppressWarnings("unchecked") public ParameterMetadata next(Part part, Class type) { @@ -146,11 +143,11 @@ class ParameterMetadataProvider { /** * Builds a new {@link ParameterMetadata} for the given type and name. * - * @param + * @param type parameter for the returned {@link ParameterMetadata}. * @param part must not be {@literal null}. * @param type must not be {@literal null}. - * @param parameter - * @return + * @param parameter providing the name for the returned {@link ParameterMetadata}. + * @return a new {@link ParameterMetadata} for the given type and name. */ private ParameterMetadata next(Part part, Class type, Parameter parameter) { @@ -164,9 +161,9 @@ class ParameterMetadataProvider { ParameterExpression expression = parameter.isExplicitlyNamed() ? builder.parameter(reifiedType, parameter.getName().orElseThrow(() -> new IllegalArgumentException("o_O Parameter needs to be named"))) : builder.parameter(reifiedType); - ParameterMetadata value = new ParameterMetadata(expression, part.getType(), - bindableParameterValues == null ? ParameterMetadata.PLACEHOLDER : bindableParameterValues.next(), - this.persistenceProvider); + ParameterMetadata value = new ParameterMetadata<>(expression, part.getType(), + bindableParameterValues == null ? ParameterMetadata.PLACEHOLDER : bindableParameterValues.next(), + this.persistenceProvider); expressions.add(value); return value; @@ -187,11 +184,6 @@ class ParameterMetadataProvider { /** * Creates a new {@link ParameterMetadata}. - * - * @param expression - * @param type - * @param value - * @param provider */ public ParameterMetadata(ParameterExpression expression, Type type, @Nullable Object value, PersistenceProvider provider) { @@ -212,8 +204,6 @@ class ParameterMetadataProvider { /** * Returns whether the parameter shall be considered an {@literal IS NULL} parameter. - * - * @return */ public boolean isIsNullParameter() { return Type.IS_NULL.equals(type); @@ -223,7 +213,6 @@ class ParameterMetadataProvider { * Prepares the object before it's actually bound to the {@link javax.persistence.Query;}. * * @param value must not be {@literal null}. - * @return */ @Nullable public Object prepare(Object value) { @@ -256,8 +245,8 @@ class ParameterMetadataProvider { * {@link Collections}, turn an array into an {@link ArrayList} or simply wrap any other value into a single element * {@link Collections}. * - * @param value - * @return + * @param value the value to be converted to a {@link Collection}. + * @return the object itself as a {@link Collection} or a {@link Collection} constructed from the value. */ @Nullable private static Collection toCollection(@Nullable Object value) { diff --git a/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderIntegrationTests.java index aee1124e4..3e1fb0432 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderIntegrationTests.java @@ -15,8 +15,7 @@ */ package org.springframework.data.jpa.repository.query; -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; +import static org.assertj.core.api.Assertions.assertThat; import java.lang.reflect.Method; import java.util.List; @@ -40,6 +39,7 @@ import org.springframework.test.util.ReflectionTestUtils; * Integration tests for {@link ParameterMetadataProvider}. * * @author Oliver Gierke + * @author Jens Schauder * @soundtrack Elephants Crossing - We are (Irrelephant) */ @RunWith(SpringJUnit4ClassRunner.class) @@ -54,7 +54,7 @@ public class ParameterMetadataProviderIntegrationTests { ParameterMetadataProvider provider = createProvider(Sample.class.getMethod("findByFirstname", String.class)); ParameterMetadata metadata = provider.next(new Part("firstname", User.class)); - assertThat(metadata.getExpression().getName(), is("name")); + assertThat(metadata.getExpression().getName()).isEqualTo("name"); } @Test // DATAJPA-758 @@ -63,7 +63,7 @@ public class ParameterMetadataProviderIntegrationTests { ParameterMetadataProvider provider = createProvider(Sample.class.getMethod("findByLastname", String.class)); ParameterMetadata metadata = provider.next(new Part("lastname", User.class)); - assertThat(metadata.getExpression().getName(), is(nullValue())); + assertThat(metadata.getExpression().getName()).isNull(); } @Test // DATAJPA-772 @@ -72,24 +72,24 @@ public class ParameterMetadataProviderIntegrationTests { ParameterMetadataProvider provider = createProvider(Sample.class.getMethod("findByAgeContaining", Integer.class)); ParameterMetadata metadata = provider.next(new Part("ageContaining", User.class)); - assertThat(metadata.prepare(1), is((Object) 1)); + assertThat(metadata.prepare(1)).isEqualTo(1); } private ParameterMetadataProvider createProvider(Method method) { JpaParameters parameters = new JpaParameters(method); - simulateDiscoveredParametername(parameters, 0, "name"); + simulateDiscoveredParametername(parameters); return new ParameterMetadataProvider(em.getCriteriaBuilder(), parameters, PersistenceProvider.fromEntityManager(em)); } - @SuppressWarnings("unchecked") - private static void simulateDiscoveredParametername(Parameters parameters, int index, String name) { + @SuppressWarnings({ "unchecked", "ConstantConditions" }) + private static void simulateDiscoveredParametername(Parameters parameters) { List list = (List) ReflectionTestUtils.getField(parameters, "parameters"); Object parameter = ReflectionTestUtils.getField(list.get(0), "parameter"); - ReflectionTestUtils.setField(parameter, "parameterName", name); + ReflectionTestUtils.setField(parameter, "parameterName", "name"); } interface Sample { diff --git a/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java index 71ff0486c..d19d64d83 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java @@ -17,8 +17,7 @@ import org.springframework.aop.framework.Advised; */ package org.springframework.data.jpa.repository.query; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.assertThat; +import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.util.ReflectionTestUtils.getField; import java.lang.reflect.Method; @@ -52,6 +51,7 @@ import org.springframework.data.repository.core.support.DefaultRepositoryMetadat import org.springframework.data.repository.query.Param; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.util.Assert; /** * Integration tests for {@link PartTreeJpaQuery}. @@ -59,6 +59,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; * @author Oliver Gierke * @author Mark Paluch * @author Michael Cramer + * @author Jens Schauder */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:infrastructure.xml") @@ -109,11 +110,11 @@ public class PartTreeJpaQueryIntegrationTests { Query query = jpaQuery.createQuery(new Object[] { "Matthews", PageRequest.of(0, 1) }); - assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY)), endsWith("firstname=:param0")); + assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY))).endsWith("firstname=:param0"); query = jpaQuery.createQuery(new Object[] { null, PageRequest.of(0, 1) }); - assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY)), endsWith("firstname is null")); + assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY))).endsWith("firstname is null"); } @Test // DATAJPA-920 @@ -124,7 +125,7 @@ public class PartTreeJpaQueryIntegrationTests { Query query = jpaQuery.createQuery(new Object[] { "Matthews" }); - assertThat(query.getMaxResults(), is(1)); + assertThat(query.getMaxResults()).isEqualTo(1); } @Test // DATAJPA-920 @@ -135,7 +136,7 @@ public class PartTreeJpaQueryIntegrationTests { Query query = jpaQuery.createQuery(new Object[] { "Matthews" }); - assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY)), containsString(".id from User as")); + assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY))).contains(".id from User as"); } @Test // DATAJPA-1074 @@ -146,7 +147,7 @@ public class PartTreeJpaQueryIntegrationTests { Query query = jpaQuery.createQuery(new Object[] {}); - assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY)), endsWith("roles is empty")); + assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY))).endsWith("roles is empty"); } @Test // DATAJPA-1074 @@ -157,7 +158,7 @@ public class PartTreeJpaQueryIntegrationTests { Query query = jpaQuery.createQuery(new Object[] {}); - assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY)), endsWith("roles is not empty")); + assertThat(HibernateUtils.getHibernateQuery(getValue(query, PROPERTY))).endsWith("roles is not empty"); } @Test(expected = IllegalStateException.class) // DATAJPA-1074 @@ -213,15 +214,18 @@ public class PartTreeJpaQueryIntegrationTests { } @SuppressWarnings("unchecked") - private static T getValue(Object source, String path) { + private static T getValue(Object source, String path) { Iterator split = Arrays.asList(path.split("\\.")).iterator(); Object result = source; while (split.hasNext()) { + + Assert.notNull(source, "result must not be null."); result = getField(result, split.next()); } + Assert.notNull(result, "result must not be null."); return (T) result; } @@ -237,7 +241,8 @@ public class PartTreeJpaQueryIntegrationTests { return Version.getVersionString().startsWith("5."); } - interface UserRepository extends Repository { + @SuppressWarnings("unused") + interface UserRepository extends Repository { Page findByFirstname(String firstname, Pageable pageable);