From 0f9ee7d14cd19ed4235078263610d1ad962bfc0c Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 10 Jan 2017 14:21:51 +0100 Subject: [PATCH 1/2] DATAREST-976 - Embedded properties are now considered in sort property paths. Prepare issue branch. --- pom.xml | 2 +- spring-data-rest-core/pom.xml | 2 +- spring-data-rest-distribution/pom.xml | 2 +- spring-data-rest-hal-browser/pom.xml | 2 +- spring-data-rest-tests/pom.xml | 2 +- spring-data-rest-tests/spring-data-rest-tests-core/pom.xml | 4 ++-- spring-data-rest-tests/spring-data-rest-tests-gemfire/pom.xml | 4 ++-- spring-data-rest-tests/spring-data-rest-tests-jpa/pom.xml | 4 ++-- spring-data-rest-tests/spring-data-rest-tests-mongodb/pom.xml | 4 ++-- .../spring-data-rest-tests-security/pom.xml | 4 ++-- spring-data-rest-tests/spring-data-rest-tests-shop/pom.xml | 2 +- spring-data-rest-tests/spring-data-rest-tests-solr/pom.xml | 4 ++-- spring-data-rest-webmvc/pom.xml | 2 +- 13 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pom.xml b/pom.xml index 19eb761335..23f0e07f17 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-rest-parent - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT pom Spring Data REST diff --git a/spring-data-rest-core/pom.xml b/spring-data-rest-core/pom.xml index e88b8b5833..c83369fc7d 100644 --- a/spring-data-rest-core/pom.xml +++ b/spring-data-rest-core/pom.xml @@ -11,7 +11,7 @@ org.springframework.data spring-data-rest-parent - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT ../pom.xml diff --git a/spring-data-rest-distribution/pom.xml b/spring-data-rest-distribution/pom.xml index 848042713e..d1fc05810d 100644 --- a/spring-data-rest-distribution/pom.xml +++ b/spring-data-rest-distribution/pom.xml @@ -13,7 +13,7 @@ org.springframework.data spring-data-rest-parent - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT ../pom.xml diff --git a/spring-data-rest-hal-browser/pom.xml b/spring-data-rest-hal-browser/pom.xml index b7cc221933..70fc7f98b6 100644 --- a/spring-data-rest-hal-browser/pom.xml +++ b/spring-data-rest-hal-browser/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-rest-parent - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT spring-data-rest-hal-browser diff --git a/spring-data-rest-tests/pom.xml b/spring-data-rest-tests/pom.xml index ef74613619..cfd30d8b39 100644 --- a/spring-data-rest-tests/pom.xml +++ b/spring-data-rest-tests/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-rest-parent - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT ../pom.xml diff --git a/spring-data-rest-tests/spring-data-rest-tests-core/pom.xml b/spring-data-rest-tests/spring-data-rest-tests-core/pom.xml index 3d2b89bac0..396912bc5f 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-core/pom.xml +++ b/spring-data-rest-tests/spring-data-rest-tests-core/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-rest-tests - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT ../pom.xml @@ -17,7 +17,7 @@ org.springframework.data spring-data-rest-webmvc - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT diff --git a/spring-data-rest-tests/spring-data-rest-tests-gemfire/pom.xml b/spring-data-rest-tests/spring-data-rest-tests-gemfire/pom.xml index 5b3afdfb53..cf6c49cfa4 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-gemfire/pom.xml +++ b/spring-data-rest-tests/spring-data-rest-tests-gemfire/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-rest-tests - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT ../pom.xml @@ -17,7 +17,7 @@ org.springframework.data spring-data-rest-tests-core - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT test-jar diff --git a/spring-data-rest-tests/spring-data-rest-tests-jpa/pom.xml b/spring-data-rest-tests/spring-data-rest-tests-jpa/pom.xml index 9f3ca1b295..f11300b55f 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-jpa/pom.xml +++ b/spring-data-rest-tests/spring-data-rest-tests-jpa/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-rest-tests - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT ../pom.xml @@ -21,7 +21,7 @@ org.springframework.data spring-data-rest-tests-core - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT test-jar diff --git a/spring-data-rest-tests/spring-data-rest-tests-mongodb/pom.xml b/spring-data-rest-tests/spring-data-rest-tests-mongodb/pom.xml index 5c7c38f49b..2cedbbcbea 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-mongodb/pom.xml +++ b/spring-data-rest-tests/spring-data-rest-tests-mongodb/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-rest-tests - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT ../pom.xml @@ -17,7 +17,7 @@ org.springframework.data spring-data-rest-tests-core - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT test-jar diff --git a/spring-data-rest-tests/spring-data-rest-tests-security/pom.xml b/spring-data-rest-tests/spring-data-rest-tests-security/pom.xml index a56b53786d..95a0f61a0a 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-security/pom.xml +++ b/spring-data-rest-tests/spring-data-rest-tests-security/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-rest-tests - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT ../pom.xml @@ -21,7 +21,7 @@ org.springframework.data spring-data-rest-tests-core - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT test-jar diff --git a/spring-data-rest-tests/spring-data-rest-tests-shop/pom.xml b/spring-data-rest-tests/spring-data-rest-tests-shop/pom.xml index 703dece1c1..4000bcefc9 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-shop/pom.xml +++ b/spring-data-rest-tests/spring-data-rest-tests-shop/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-rest-tests - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT Spring Data REST Tests - Shop diff --git a/spring-data-rest-tests/spring-data-rest-tests-solr/pom.xml b/spring-data-rest-tests/spring-data-rest-tests-solr/pom.xml index 48779968d8..dea6b5f504 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-solr/pom.xml +++ b/spring-data-rest-tests/spring-data-rest-tests-solr/pom.xml @@ -4,7 +4,7 @@ org.springframework.data spring-data-rest-tests - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT Spring Data REST Tests - Solr @@ -15,7 +15,7 @@ org.springframework.data spring-data-rest-tests-core - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT test-jar diff --git a/spring-data-rest-webmvc/pom.xml b/spring-data-rest-webmvc/pom.xml index 452876350c..c2b2378ef3 100644 --- a/spring-data-rest-webmvc/pom.xml +++ b/spring-data-rest-webmvc/pom.xml @@ -12,7 +12,7 @@ org.springframework.data spring-data-rest-parent - 2.6.0.BUILD-SNAPSHOT + 2.6.0.DATAREST-976-SNAPSHOT ../pom.xml From c1fb7bf070daca9e2464585594e6d3eef32cf914 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 10 Jan 2017 14:19:02 +0100 Subject: [PATCH 2/2] DATAREST-976 - Embedded properties are now considered in sort property paths. We now allow sorting by properties of embedded objects. An embedded object is not linkable to a root object but embedded in the resource itself. If any part of the sort property path points to a linkable association, the whole sort property path is discarded silently and not used for sorting any further. --- .../data/rest/webmvc/jpa/Book.java | 23 +++++++++++++-- .../data/rest/webmvc/jpa/JpaWebTests.java | 4 +-- .../rest/webmvc/jpa/TestDataPopulator.java | 7 +++-- .../RepositoryRestMvcConfiguration.java | 3 +- .../JacksonMappingAwareSortTranslator.java | 12 +++++--- .../webmvc/json/SortTranslatorUnitTests.java | 28 +++++++++++++++++-- src/main/asciidoc/paging-and-sorting.adoc | 2 +- 7 files changed, 63 insertions(+), 16 deletions(-) diff --git a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/main/java/org/springframework/data/rest/webmvc/jpa/Book.java b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/main/java/org/springframework/data/rest/webmvc/jpa/Book.java index 6a6b51a447..5f6d6d8f06 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/main/java/org/springframework/data/rest/webmvc/jpa/Book.java +++ b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/main/java/org/springframework/data/rest/webmvc/jpa/Book.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2016 the original author or authors. + * Copyright 2013-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,10 +15,15 @@ */ package org.springframework.data.rest.webmvc.jpa; +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.NoArgsConstructor; + import java.util.HashSet; import java.util.Set; import javax.persistence.CascadeType; +import javax.persistence.Embeddable; import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.Id; @@ -45,9 +50,11 @@ public class Book { @RestResource(path = "creators") // public Set authors; + public Offer offer; + protected Book() {} - public Book(String isbn, String title, long soldUnits, Iterable authors) { + public Book(String isbn, String title, long soldUnits, Iterable authors, Offer offer) { this.isbn = isbn; this.title = title; @@ -59,5 +66,17 @@ public Book(String isbn, String title, long soldUnits, Iterable authors) author.books.add(this); this.authors.add(author); } + + this.offer = offer; + } + + @Getter + @Embeddable + @AllArgsConstructor + @NoArgsConstructor + static class Offer { + + double price; + String currency; } } diff --git a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java index 5838748297..791bbb064c 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java +++ b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java @@ -494,12 +494,12 @@ public void exectuesSearchThatTakesASort() throws Exception { assertThat(findBySortedLink.getVariableNames(), hasItems("sort", "projection")); // Assert results returned as specified - client.follow(findBySortedLink.expand("title,desc")).// + client.follow(findBySortedLink.expand("offer.price,desc")).// andExpect(jsonPath("$._embedded.books[0].title").value("Spring Data (Second Edition)")).// andExpect(jsonPath("$._embedded.books[1].title").value("Spring Data")).// andExpect(client.hasLinkWithRel("self")); - client.follow(findBySortedLink.expand("title,asc")).// + client.follow(findBySortedLink.expand("offer.price,asc")).// andExpect(jsonPath("$._embedded.books[0].title").value("Spring Data")).// andExpect(jsonPath("$._embedded.books[1].title").value("Spring Data (Second Edition)")).// andExpect(client.hasLinkWithRel("self")); diff --git a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/TestDataPopulator.java b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/TestDataPopulator.java index b40292de2b..1e16f73453 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/TestDataPopulator.java +++ b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/TestDataPopulator.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2016 the original author or authors. + * Copyright 2013-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. @@ -18,6 +18,7 @@ import java.util.Arrays; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.rest.webmvc.jpa.Book.Offer; /** * @author Jon Brisbin @@ -54,8 +55,8 @@ private void populateAuthorsAndBooks() { Iterable authors = this.authors.save(Arrays.asList(ollie, mark, michael, david, john, thomas)); - books.save(new Book("1449323952", "Spring Data", 1000, authors)); - books.save(new Book("1449323953", "Spring Data (Second Edition)", 2000, authors)); + books.save(new Book("1449323952", "Spring Data", 1000, authors, new Offer(21.21, "EUR"))); + books.save(new Book("1449323953", "Spring Data (Second Edition)", 2000, authors, new Offer(30.99, "EUR"))); } private void populateOrders() { diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java index 563c97db44..db67e9ca40 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java @@ -804,7 +804,8 @@ protected List defaultMethodArgumentResolvers() { PageableHandlerMethodArgumentResolver pageableResolver = pageableResolver(); JacksonMappingAwareSortTranslator sortTranslator = new JacksonMappingAwareSortTranslator(objectMapper(), - repositories(), DomainClassResolver.of(repositories(), resourceMappings(), baseUri()), persistentEntities()); + repositories(), DomainClassResolver.of(repositories(), resourceMappings(), baseUri()), persistentEntities(), + associationLinks()); HandlerMethodArgumentResolver sortResolver = new MappingAwareSortArgumentResolver(sortTranslator, sortResolver()); HandlerMethodArgumentResolver jacksonPageableResolver = new MappingAwarePageableArgumentResolver(sortTranslator, diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/JacksonMappingAwareSortTranslator.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/JacksonMappingAwareSortTranslator.java index 01ec64094b..cb8e62a374 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/JacksonMappingAwareSortTranslator.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/JacksonMappingAwareSortTranslator.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 the original author or authors. + * Copyright 2016-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. @@ -31,6 +31,7 @@ import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.context.PersistentEntities; import org.springframework.data.repository.support.Repositories; +import org.springframework.data.rest.webmvc.mapping.Associations; import org.springframework.data.rest.webmvc.support.DomainClassResolver; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -62,16 +63,18 @@ public class JacksonMappingAwareSortTranslator { * @param repositories must not be {@literal null}. * @param domainClassResolver must not be {@literal null}. * @param persistentEntities must not be {@literal null}. + * @param associations must not be {@literal null}. */ public JacksonMappingAwareSortTranslator(ObjectMapper objectMapper, Repositories repositories, - DomainClassResolver domainClassResolver, PersistentEntities persistentEntities) { + DomainClassResolver domainClassResolver, PersistentEntities persistentEntities, Associations associations) { Assert.notNull(repositories, "Repositories must not be null!"); Assert.notNull(domainClassResolver, "DomainClassResolver must not be null!"); + Assert.notNull(associations, "Associations must not be null!"); this.repositories = repositories; this.domainClassResolver = domainClassResolver; - this.sortTranslator = new SortTranslator(persistentEntities, objectMapper); + this.sortTranslator = new SortTranslator(persistentEntities, objectMapper, associations); } /** @@ -117,6 +120,7 @@ public static class SortTranslator { private final @NonNull PersistentEntities persistentEntities; private final @NonNull ObjectMapper objectMapper; + private final @NonNull Associations associations; /** * Translates {@link Sort} orders from Jackson-mapped field names to {@link PersistentProperty} names. Properties @@ -181,7 +185,7 @@ private List mapPropertyPath(PersistentEntity rootEntity, List persistentProperty : persistentProperties) { - if (persistentProperty.isAssociation()) { + if (associations.isLinkableAssociation(persistentProperty)) { return Collections.emptyList(); } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/SortTranslatorUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/SortTranslatorUnitTests.java index 244342355d..8a09132a0b 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/SortTranslatorUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/SortTranslatorUnitTests.java @@ -17,6 +17,7 @@ import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import static org.mockito.Mockito.*; import java.util.Collections; import java.util.List; @@ -27,7 +28,11 @@ import org.springframework.data.domain.Sort; import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext; import org.springframework.data.mapping.context.PersistentEntities; +import org.springframework.data.rest.core.annotation.RestResource; +import org.springframework.data.rest.core.config.RepositoryRestConfiguration; +import org.springframework.data.rest.core.mapping.PersistentEntitiesResourceMappings; import org.springframework.data.rest.webmvc.json.JacksonMappingAwareSortTranslator.SortTranslator; +import org.springframework.data.rest.webmvc.mapping.Associations; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonUnwrapped; @@ -57,7 +62,9 @@ public void setUp() { mappingContext.getPersistentEntity(MultiUnwrapped.class); persistentEntities = new PersistentEntities(Collections.singleton(mappingContext)); - sortTranslator = new SortTranslator(persistentEntities, objectMapper); + + sortTranslator = new SortTranslator(persistentEntities, objectMapper, new Associations( + new PersistentEntitiesResourceMappings(persistentEntities), mock(RepositoryRestConfiguration.class))); } @Test // DATAREST-883 @@ -119,15 +126,24 @@ public void shouldSkipWrongNestedProperties() { assertThat(translatedSort, is(nullValue())); } - @Test // DATAREST-910 + @Test // DATAREST-910, DATAREST-976 public void shouldSkipKnownAssociationProperties() { - Sort translatedSort = sortTranslator.translateSort(new Sort("refEmbedded.name"), + Sort translatedSort = sortTranslator.translateSort(new Sort("association.name"), mappingContext.getPersistentEntity(Plain.class)); assertThat(translatedSort, is(nullValue())); } + @Test // DATAREST-976 + public void shouldMapEmbeddableAssociationProperties() { + + Sort translatedSort = sortTranslator.translateSort(new Sort("refEmbedded.name"), + mappingContext.getPersistentEntity(Plain.class)); + + assertThat(translatedSort.getOrderFor("refEmbedded.name"), is(notNullValue())); + } + @Test // DATAREST-910 public void shouldJacksonFieldNameForNestedFieldMapping() { @@ -161,6 +177,7 @@ static class Plain { public String name; public Embedded embedded; @Reference public Embedded refEmbedded; + @Reference public AnotherRootEntity association; } static class UnwrapEmbedded { @@ -192,4 +209,9 @@ static class EmbeddedWithJsonProperty { } static interface SomeInterface {} + + @RestResource + static class AnotherRootEntity { + public String name; + } } diff --git a/src/main/asciidoc/paging-and-sorting.adoc b/src/main/asciidoc/paging-and-sorting.adoc index e745912a59..26a76c28bf 100644 --- a/src/main/asciidoc/paging-and-sorting.adoc +++ b/src/main/asciidoc/paging-and-sorting.adoc @@ -127,4 +127,4 @@ To have your results sorted on a particular property, add a `sort` URL parameter curl -v "http://localhost:8080/people/search/nameStartsWith?name=K&sort=name,desc" ---- -To sort the results by more than one property, keep adding as many `sort=PROPERTY` parameters as you need. They will be added to the `Pageable` in the order they appear in the query string. \ No newline at end of file +To sort the results by more than one property, keep adding as many `sort=PROPERTY` parameters as you need. They will be added to the `Pageable` in the order they appear in the query string. Results can be sorted by top-level and nested properties. Use property path notation to express a nested sort property. Sorting by linkable associations (i.e. resources to top-level resources) is not supported. \ No newline at end of file