From 5dacc87694afb302c741361731c46061d221d6f0 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 19 Feb 2018 13:34:48 +0100 Subject: [PATCH] DATAJPA-1233 - Polishing. Formatting, imports. --- .../query/AbstractStringBasedJpaQuery.java | 11 ++- .../data/jpa/repository/query/NamedQuery.java | 5 +- .../jpa/repository/query/ParameterBinder.java | 8 +- .../query/QueryParameterSetter.java | 17 ++-- .../jpa/repository/UserRepositoryTests.java | 37 +++++--- ...rIndexedQueryParameterSetterUnitTests.java | 88 +++++++++---------- 6 files changed, 93 insertions(+), 73 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java index 145c68cc2..48bf26e6d 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java @@ -15,12 +15,17 @@ */ package org.springframework.data.jpa.repository.query; +import static org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling.*; + import javax.persistence.EntityManager; import javax.persistence.Query; import javax.persistence.Tuple; -import org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling; -import org.springframework.data.repository.query.*; +import org.springframework.data.repository.query.EvaluationContextProvider; +import org.springframework.data.repository.query.ParameterAccessor; +import org.springframework.data.repository.query.ParametersParameterAccessor; +import org.springframework.data.repository.query.ResultProcessor; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.util.Assert; @@ -106,7 +111,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { ? em.createNativeQuery(queryString) // : em.createQuery(queryString, Long.class); - return parameterBinder.get().bind(query, values, ErrorHandling.LENIENT); + return parameterBinder.get().bind(query, values, LENIENT); } /** diff --git a/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java index 739660e63..ecfde50a9 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/NamedQuery.java @@ -15,6 +15,8 @@ */ package org.springframework.data.jpa.repository.query; +import static org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling.*; + import javax.persistence.EntityManager; import javax.persistence.Query; import javax.persistence.TypedQuery; @@ -22,7 +24,6 @@ import javax.persistence.TypedQuery; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.data.jpa.provider.QueryExtractor; -import org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.QueryCreationException; import org.springframework.data.repository.query.RepositoryQuery; @@ -171,6 +172,6 @@ final class NamedQuery extends AbstractJpaQuery { countQuery = em.createQuery(QueryUtils.createCountQueryFor(queryString, countProjection), Long.class); } - return parameterBinder.get().bind(countQuery, values, ErrorHandling.LENIENT); + return parameterBinder.get().bind(countQuery, values, LENIENT); } } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java b/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java index 7d7f4dd88..30db019fd 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java @@ -53,6 +53,10 @@ public class ParameterBinder { this.parameterSetters = parameterSetters; } + public T bind(T jpaQuery, Object[] values) { + return bind(jpaQuery, values, ErrorHandling.STRICT); + } + public T bind(T jpaQuery, Object[] values, ErrorHandling errorHandling) { parameterSetters.forEach(it -> it.setParameter(jpaQuery, values, errorHandling)); @@ -60,10 +64,6 @@ public class ParameterBinder { return jpaQuery; } - public T bind(T jpaQuery, Object[] values) { - return bind(jpaQuery, values, ErrorHandling.STRICT); - } - /** * Binds the parameters to the given query and applies special parameter types (e.g. pagination). * diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java index 0ec4e8d9d..cee43558e 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java @@ -15,7 +15,7 @@ */ package org.springframework.data.jpa.repository.query; -import static org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling.LENIENT; +import static org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling.*; import java.util.Date; import java.util.function.Function; @@ -92,8 +92,11 @@ interface QueryParameterSetter { } else { Integer position = parameter.getPosition(); - if (position != null && (query.getParameters().size() >= parameter.getPosition() - || registerExcessParameters(query) || errorHandling == LENIENT)) { + + if (position != null // + && (query.getParameters().size() >= parameter.getPosition() // + || registerExcessParameters(query) // + || errorHandling == LENIENT)) { errorHandling.execute(() -> query.setParameter(parameter.getPosition(), (Date) value, temporalType)); } @@ -109,8 +112,11 @@ interface QueryParameterSetter { } else { Integer position = parameter.getPosition(); - if (position != null && (query.getParameters().size() >= position || errorHandling == LENIENT - || registerExcessParameters(query))) { + + if (position != null // + && (query.getParameters().size() >= position // + || errorHandling == LENIENT // + || registerExcessParameters(query))) { errorHandling.execute(() -> query.setParameter(position, value)); } @@ -142,6 +148,7 @@ interface QueryParameterSetter { }, LENIENT { + @Override public void execute(Runnable block) { diff --git a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index 6bbfc6047..ac1e14450 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -15,17 +15,21 @@ */ package org.springframework.data.jpa.repository; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.springframework.data.domain.Example.of; -import static org.springframework.data.domain.ExampleMatcher.matching; -import static org.springframework.data.domain.Sort.Direction.ASC; -import static org.springframework.data.domain.Sort.Direction.DESC; +import static org.assertj.core.api.Assertions.*; +import static org.springframework.data.domain.Example.*; +import static org.springframework.data.domain.ExampleMatcher.*; +import static org.springframework.data.domain.Sort.Direction.*; +import static org.springframework.data.jpa.domain.Specification.*; import static org.springframework.data.jpa.domain.Specification.not; -import static org.springframework.data.jpa.domain.Specification.where; import static org.springframework.data.jpa.domain.sample.UserSpecifications.*; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.stream.Stream; import javax.persistence.EntityManager; @@ -44,12 +48,18 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.DataAccessException; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; -import org.springframework.data.domain.*; +import org.springframework.data.domain.Example; +import org.springframework.data.domain.ExampleMatcher; import org.springframework.data.domain.ExampleMatcher.GenericPropertyMatcher; import org.springframework.data.domain.ExampleMatcher.StringMatcher; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Slice; +import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Direction; import org.springframework.data.domain.Sort.Order; -import org.springframework.data.domain.ExampleMatcher.*; import org.springframework.data.jpa.domain.Specification; import org.springframework.data.jpa.domain.sample.Address; import org.springframework.data.jpa.domain.sample.Role; @@ -214,10 +224,10 @@ public class UserRepositoryTests { flushTestUsers(); Order order = new Order(ASC, "firstname").ignoreCase(); - List result = repository.findAll(Sort.by(order)); - assertThat(repository.findAll(Sort.by(order))).hasSize(4).containsExactly(thirdUser, secondUser, fourthUser, - firstUser); + assertThat(repository.findAll(Sort.by(order))) // + .hasSize(4)// + .containsExactly(thirdUser, secondUser, fourthUser, firstUser); } @Test @@ -228,6 +238,7 @@ public class UserRepositoryTests { long before = repository.count(); repository.deleteAll(Arrays.asList(firstUser, secondUser)); + assertThat(repository.existsById(firstUser.getId())).isFalse(); assertThat(repository.existsById(secondUser.getId())).isFalse(); assertThat(repository.count()).isEqualTo(before - 2); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/NamedOrIndexedQueryParameterSetterUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/NamedOrIndexedQueryParameterSetterUnitTests.java index 9c4a6cb30..9d6030e04 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/NamedOrIndexedQueryParameterSetterUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/NamedOrIndexedQueryParameterSetterUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 the original author or authors. + * Copyright 2017-2018 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. @@ -15,14 +15,15 @@ */ package org.springframework.data.jpa.repository.query; -import static java.util.Arrays.asList; -import static javax.persistence.TemporalType.TIME; +import static java.util.Arrays.*; +import static javax.persistence.TemporalType.*; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; -import static org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling.LENIENT; -import static org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling.STRICT; +import static org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling.*; -import lombok.RequiredArgsConstructor; +import lombok.Value; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.List; @@ -38,7 +39,10 @@ import org.junit.Test; import org.springframework.data.jpa.repository.query.QueryParameterSetter.NamedOrIndexedQueryParameterSetter; /** + * Unit tests fir {@link NamedOrIndexedQueryParameterSetter}. + * * @author Jens Schauder + * @author Oliver Gierke */ public class NamedOrIndexedQueryParameterSetterUnitTests { @@ -47,7 +51,7 @@ public class NamedOrIndexedQueryParameterSetterUnitTests { Object[] methodArguments = { new Date() }; List temporalTypes = asList(null, TIME); - List parameters = asList( // + List> parameters = Arrays.> asList( // mock(ParameterExpression.class), // new ParameterImpl("name", null), // new ParameterImpl(null, 1) // @@ -87,7 +91,7 @@ public class NamedOrIndexedQueryParameterSetterUnitTests { Query query = mockExceptionThrowingQueryWithNamedParameters(); - for (Parameter parameter : parameters) { + for (Parameter parameter : parameters) { for (TemporalType temporalType : temporalTypes) { NamedOrIndexedQueryParameterSetter setter = new NamedOrIndexedQueryParameterSetter( // @@ -111,9 +115,8 @@ public class NamedOrIndexedQueryParameterSetterUnitTests { /** * setParameter should be called in the lenient case even if the number of parameters seems to suggest that it fails, - * since the index might not be continuous due to missing parts of count queries compared to the main query. - * - * This happens when a parameter gets used in the ORDER BY clause which gets stripped of for the count query. + * since the index might not be continuous due to missing parts of count queries compared to the main query. This + * happens when a parameter gets used in the ORDER BY clause which gets stripped of for the count query. */ @Test // DATAJPA-1233 public void lenientSetsParameterWhenSuccessIsUnsure() { @@ -130,10 +133,11 @@ public class NamedOrIndexedQueryParameterSetterUnitTests { setter.setParameter(query, methodArguments, LENIENT); - if (temporalType == null) + if (temporalType == null) { verify(query).setParameter(eq(11), any(Date.class)); - else + } else { verify(query).setParameter(eq(11), any(Date.class), eq(temporalType)); + } } softly.assertAll(); @@ -141,9 +145,8 @@ public class NamedOrIndexedQueryParameterSetterUnitTests { } /** - * This scenario happens when the only (name) parameter is part of an ORDER BY clause and gets stripped of for the count query. - * - * Then the count query has no named parameter but the parameter provided has a {@literal null} position. + * This scenario happens when the only (name) parameter is part of an ORDER BY clause and gets stripped of for the + * count query. Then the count query has no named parameter but the parameter provided has a {@literal null} position. */ @Test // DATAJPA-1233 public void parameterNotSetWhenSuccessImpossible() { @@ -160,10 +163,11 @@ public class NamedOrIndexedQueryParameterSetterUnitTests { setter.setParameter(query, methodArguments, LENIENT); - if (temporalType == null) + if (temporalType == null) { verify(query, never()).setParameter(anyInt(), any(Date.class)); - else + } else { verify(query, never()).setParameter(anyInt(), any(Date.class), eq(temporalType)); + } } softly.assertAll(); @@ -171,46 +175,38 @@ public class NamedOrIndexedQueryParameterSetterUnitTests { } @SuppressWarnings("unchecked") - public Query mockExceptionThrowingQueryWithNamedParameters() { + private static Query mockExceptionThrowingQueryWithNamedParameters() { + Query query = mock(Query.class); // make it a query with named parameters - doReturn(Collections.singleton(new ParameterImpl("aName", 3))).when(query).getParameters(); - doThrow(new RuntimeException(EXCEPTION_MESSAGE)).when(query) // - .setParameter(any(Parameter.class), any(Date.class), any(TemporalType.class)); - doThrow(new RuntimeException(EXCEPTION_MESSAGE)).when(query) // - .setParameter(any(Parameter.class), any(Date.class)); - doThrow(new RuntimeException(EXCEPTION_MESSAGE)).when(query) // - .setParameter(anyString(), any(Date.class), any(TemporalType.class)); - doThrow(new RuntimeException(EXCEPTION_MESSAGE)).when(query) // - .setParameter(anyString(), any(Date.class)); - doThrow(new RuntimeException(EXCEPTION_MESSAGE)).when(query) // - .setParameter(anyInt(), any(Date.class), any(TemporalType.class)); - doThrow(new RuntimeException(EXCEPTION_MESSAGE)).when(query) // - .setParameter(anyInt(), any(Date.class)); + doReturn(Collections.singleton(new ParameterImpl("aName", 3))) // + .when(query).getParameters(); + doThrow(new RuntimeException(EXCEPTION_MESSAGE)) // + .when(query).setParameter(any(Parameter.class), any(Date.class), any(TemporalType.class)); + doThrow(new RuntimeException(EXCEPTION_MESSAGE)) // + .when(query).setParameter(any(Parameter.class), any(Date.class)); + doThrow(new RuntimeException(EXCEPTION_MESSAGE)) // + .when(query).setParameter(anyString(), any(Date.class), any(TemporalType.class)); + doThrow(new RuntimeException(EXCEPTION_MESSAGE)) // + .when(query).setParameter(anyString(), any(Date.class)); + doThrow(new RuntimeException(EXCEPTION_MESSAGE)) // + .when(query).setParameter(anyInt(), any(Date.class), any(TemporalType.class)); + doThrow(new RuntimeException(EXCEPTION_MESSAGE)) // + .when(query).setParameter(anyInt(), any(Date.class)); + return query; } - @RequiredArgsConstructor + @Value private static class ParameterImpl implements Parameter { - private final String name; - private final Integer position; - - @Override - public String getName() { - return name; - } - - @Override - public Integer getPosition() { - return position; - } + String name; + Integer position; @Override public Class getParameterType() { return Object.class; } } - }