From 0273476ba6696ba3566c2efaa47721626233a738 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 28 Aug 2017 09:37:34 +0200 Subject: [PATCH] DATAJPA-1170 - Polishing. Moved implementation code back to Specifications so that it can be made package protected in Lovelace. Replaced simple Specification implementations for composability and negation with static factory methods using lambdas. Added @Deprecated to methods and Specifications type. Polished JavaDoc @deprecated tags. Removed obsolete generics declarations on instantiations. Added JavaDoc to newly added methods in Specification. Original pull request: #211. --- .../data/jpa/domain/Specification.java | 135 ++++-------------- .../data/jpa/domain/Specifications.java | 68 +++++++-- .../jpa/domain/SpecificationUnitTests.java | 23 +-- 3 files changed, 96 insertions(+), 130 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/domain/Specification.java b/src/main/java/org/springframework/data/jpa/domain/Specification.java index 62569fb83..d30728fc7 100644 --- a/src/main/java/org/springframework/data/jpa/domain/Specification.java +++ b/src/main/java/org/springframework/data/jpa/domain/Specification.java @@ -15,17 +15,15 @@ */ package org.springframework.data.jpa.domain; -import org.springframework.util.Assert; +import static org.springframework.data.jpa.domain.Specifications.CompositionType.*; + +import java.io.Serializable; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; -import java.io.Serializable; - -import static org.springframework.data.jpa.domain.Specification.CompositionType.*; - /** * Specification in the sense of Domain Driven Design. * @@ -34,20 +32,33 @@ import static org.springframework.data.jpa.domain.Specification.CompositionType. * @author Krzysztof Rzymkowski * @author Sebastian Staudt */ +@SuppressWarnings("deprecation") public interface Specification extends Serializable { long serialVersionUID = 1L; + /** + * Negates the given {@link Specification}. + * + * @param + * @param spec can be {@literal null}. + * @return + * @since 2.0 + */ static Specification not(Specification spec) { - return new NegatedSpecification<>(spec); + return Specifications.negated(spec); } + /** + * Simple static factory method to add some syntactic sugar around a {@link Specification}. + * + * @param + * @param spec can be {@literal null}. + * @return + * @since 2.0 + */ static Specification where(Specification spec) { - if (spec == null) { - return new Specifications<>(null); - } - - return spec; + return Specifications.where(spec); } /** @@ -55,9 +66,10 @@ public interface Specification extends Serializable { * * @param other can be {@literal null}. * @return The conjunction of the specifications + * @since 2.0 */ default Specification and(Specification other) { - return new ComposedSpecification<>(this, other, AND); + return Specifications.composed(this, other, AND); } /** @@ -65,9 +77,10 @@ public interface Specification extends Serializable { * * @param other can be {@literal null}. * @return The disjunction of the specifications + * @since 2.0 */ default Specification or(Specification other) { - return new ComposedSpecification<>(this, other, OR); + return Specifications.composed(this, other, OR); } /** @@ -79,100 +92,4 @@ public interface Specification extends Serializable { * @return a {@link Predicate}, may be {@literal null}. */ Predicate toPredicate(Root root, CriteriaQuery query, CriteriaBuilder cb); - - /** - * Enum for the composition types for {@link Predicate}s. - * - * @author Thomas Darimont - */ - enum CompositionType { - - AND { - @Override - public Predicate combine(CriteriaBuilder builder, Predicate lhs, Predicate rhs) { - return builder.and(lhs, rhs); - } - }, - - OR { - @Override - public Predicate combine(CriteriaBuilder builder, Predicate lhs, Predicate rhs) { - return builder.or(lhs, rhs); - } - }; - - abstract Predicate combine(CriteriaBuilder builder, Predicate lhs, Predicate rhs); - } - - /** - * A {@link Specification} that negates a given {@code Specification}. - * - * @author Thomas Darimont - * @since 1.6 - */ - class NegatedSpecification implements Specification, Serializable { - - private static final long serialVersionUID = 1L; - - private final Specification spec; - - /** - * Creates a new {@link NegatedSpecification} from the given {@link Specification} - * - * @param spec may be {@literal null} - */ - NegatedSpecification(Specification spec) { - this.spec = spec; - } - - public Predicate toPredicate(Root root, CriteriaQuery query, CriteriaBuilder builder) { - return spec == null ? null : builder.not(spec.toPredicate(root, query, builder)); - } - } - - /** - * A {@link Specification} that combines two given {@code Specification}s via a given {@link CompositionType}. - * - * @author Thomas Darimont - * @since 1.6 - */ - class ComposedSpecification implements Specification, Serializable { - - private static final long serialVersionUID = 1L; - - private final Specification lhs; - private final Specification rhs; - private final CompositionType compositionType; - - /** - * Creates a new {@link ComposedSpecification} from the given {@link Specification} for the left-hand-side and the - * right-hand-side with the given {@link CompositionType}. - * - * @param lhs may be {@literal null} - * @param rhs may be {@literal null} - * @param compositionType must not be {@literal null} - */ - ComposedSpecification(Specification lhs, Specification rhs, CompositionType compositionType) { - - Assert.notNull(compositionType, "CompositionType must not be null!"); - - this.lhs = lhs; - this.rhs = rhs; - this.compositionType = compositionType; - } - - /** - * Returns {@link Predicate} for the given {@link Root} and {@link CriteriaQuery} that is constructed via the given - * {@link CriteriaBuilder}. - */ - public Predicate toPredicate(Root root, CriteriaQuery query, CriteriaBuilder builder) { - - Predicate otherPredicate = rhs == null ? null : rhs.toPredicate(root, query, builder); - Predicate thisPredicate = lhs == null ? null : lhs.toPredicate(root, query, builder); - - return thisPredicate == null ? otherPredicate : otherPredicate == null ? thisPredicate : this.compositionType - .combine(builder, thisPredicate, otherPredicate); - } - } - } diff --git a/src/main/java/org/springframework/data/jpa/domain/Specifications.java b/src/main/java/org/springframework/data/jpa/domain/Specifications.java index d57e33579..20dd8b21c 100644 --- a/src/main/java/org/springframework/data/jpa/domain/Specifications.java +++ b/src/main/java/org/springframework/data/jpa/domain/Specifications.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2014 the original author or authors. + * Copyright 2008-2017 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,6 +15,8 @@ */ package org.springframework.data.jpa.domain; +import static org.springframework.data.jpa.domain.Specifications.CompositionType.*; + import java.io.Serializable; import javax.persistence.criteria.CriteriaBuilder; @@ -22,15 +24,15 @@ import javax.persistence.criteria.CriteriaQuery; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; -import static org.springframework.data.jpa.domain.Specification.CompositionType.*; - /** * Helper class to easily combine {@link Specification} instances. * * @author Oliver Gierke * @author Thomas Darimont * @author Sebastian Staudt + * @deprecated since 2.0, use factory methods on {@link Specification} instead. */ +@Deprecated public class Specifications implements Specification, Serializable { private static final long serialVersionUID = 1L; @@ -49,49 +51,53 @@ public class Specifications implements Specification, Serializable { /** * Simple static factory method to add some syntactic sugar around a {@link Specification}. * - * @deprecated Use {@link Specification#where} instead + * @deprecated since 2.0, use {@link Specification#where} instead * @param * @param spec can be {@literal null}. * @return */ + @Deprecated public static Specifications where(Specification spec) { - return new Specifications(spec); + return new Specifications<>(spec); } /** * ANDs the given {@link Specification} to the current one. * - * @deprecated Use {@link Specification#and} instead + * @deprecated since 2.0, use {@link Specification#and} instead * @param * @param other can be {@literal null}. * @return */ + @Deprecated public Specifications and(Specification other) { - return new Specifications(new ComposedSpecification(spec, other, AND)); + return new Specifications<>(composed(spec, other, AND)); } /** * ORs the given specification to the current one. * - * @deprecated Use {@link Specification#or} instead + * @deprecated since 2.0, use {@link Specification#or} instead * @param * @param other can be {@literal null}. * @return */ + @Deprecated public Specifications or(Specification other) { - return new Specifications(new ComposedSpecification(spec, other, OR)); + return new Specifications<>(composed(spec, other, OR)); } /** * Negates the given {@link Specification}. * - * @deprecated Use {@link Specification#not} instead + * @deprecated since 2.0, use {@link Specification#not} instead * @param * @param spec can be {@literal null}. * @return */ + @Deprecated public static Specifications not(Specification spec) { - return new Specifications(new NegatedSpecification(spec)); + return new Specifications<>(negated(spec)); } /* @@ -102,4 +108,44 @@ public class Specifications implements Specification, Serializable { return spec == null ? null : spec.toPredicate(root, query, builder); } + /** + * Enum for the composition types for {@link Predicate}s. Can not be turned into lambdas as we need to be + * serializable. + * + * @author Thomas Darimont + */ + enum CompositionType { + + AND { + @Override + public Predicate combine(CriteriaBuilder builder, Predicate lhs, Predicate rhs) { + return builder.and(lhs, rhs); + } + }, + + OR { + @Override + public Predicate combine(CriteriaBuilder builder, Predicate lhs, Predicate rhs) { + return builder.or(lhs, rhs); + } + }; + + abstract Predicate combine(CriteriaBuilder builder, Predicate lhs, Predicate rhs); + } + + static Specification negated(Specification spec) { + return (root, query, builder) -> spec == null ? null : builder.not(spec.toPredicate(root, query, builder)); + } + + static Specification composed(Specification lhs, Specification rhs, CompositionType compositionType) { + + return (root, query, builder) -> { + + Predicate otherPredicate = rhs == null ? null : rhs.toPredicate(root, query, builder); + Predicate thisPredicate = lhs == null ? null : lhs.toPredicate(root, query, builder); + + return thisPredicate == null ? otherPredicate + : otherPredicate == null ? thisPredicate : compositionType.combine(builder, thisPredicate, otherPredicate); + }; + } } diff --git a/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java b/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java index 46c4aed8d..89eb48856 100644 --- a/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java @@ -17,8 +17,8 @@ package org.springframework.data.jpa.domain; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; +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.util.SerializationUtils.*; import java.io.Serializable; @@ -35,10 +35,13 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; /** + * Unit tests for {@link Specification}. + * * @author Oliver Gierke * @author Thomas Darimont * @author Sebastian Staudt */ +@SuppressWarnings("serial") @RunWith(MockitoJUnitRunner.class) public class SpecificationUnitTests implements Serializable { @@ -50,13 +53,12 @@ public class SpecificationUnitTests implements Serializable { @Mock(extraInterfaces = Serializable.class) Predicate predicate; @Before - @SuppressWarnings("unchecked") public void setUp() { spec = (root, query, cb) -> predicate; } - @Test // DATAJPA-300 + @Test // DATAJPA-300, DATAJPA-1170 public void createsSpecificationsFromNull() { Specification specification = where(null); @@ -64,7 +66,7 @@ public class SpecificationUnitTests implements Serializable { assertThat(specification.toPredicate(root, query, builder), is(nullValue())); } - @Test // DATAJPA-300 + @Test // DATAJPA-300, DATAJPA-1170 public void negatesNullSpecToNull() { Specification specification = not(null); @@ -73,7 +75,7 @@ public class SpecificationUnitTests implements Serializable { assertThat(specification.toPredicate(root, query, builder), is(nullValue())); } - @Test // DATAJPA-300 + @Test // DATAJPA-300, DATAJPA-1170 public void andConcatenatesSpecToNullSpec() { Specification specification = where(null); @@ -83,7 +85,7 @@ public class SpecificationUnitTests implements Serializable { assertThat(specification.toPredicate(root, query, builder), is(predicate)); } - @Test // DATAJPA-300 + @Test // DATAJPA-300, DATAJPA-1170 public void andConcatenatesNullSpecToSpec() { Specification specification = spec.and(null); @@ -92,7 +94,7 @@ public class SpecificationUnitTests implements Serializable { assertThat(specification.toPredicate(root, query, builder), is(predicate)); } - @Test // DATAJPA-300 + @Test // DATAJPA-300, DATAJPA-1170 public void orConcatenatesSpecToNullSpec() { Specification specification = where(null); @@ -102,7 +104,7 @@ public class SpecificationUnitTests implements Serializable { assertThat(specification.toPredicate(root, query, builder), is(predicate)); } - @Test // DATAJPA-300 + @Test // DATAJPA-300, DATAJPA-1170 public void orConcatenatesNullSpecToSpec() { Specification specification = spec.or(null); @@ -129,7 +131,8 @@ public class SpecificationUnitTests implements Serializable { public void complexSpecificationsShouldBeSerializable() { SerializableSpecification serializableSpec = new SerializableSpecification(); - Specification specification = Specification.not(serializableSpec.and(serializableSpec).or(serializableSpec)); + Specification specification = Specification + .not(serializableSpec.and(serializableSpec).or(serializableSpec)); assertThat(specification, is(notNullValue())); @@ -140,10 +143,10 @@ public class SpecificationUnitTests implements Serializable { } public class SerializableSpecification implements Serializable, Specification { + @Override public Predicate toPredicate(Root root, CriteriaQuery query, CriteriaBuilder cb) { return null; } } - }