From 7071100dab77f1923743927687908e5f339afeab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A2m=20Tran=20Duy?= <91741080+LamWizy@users.noreply.github.com> Date: Wed, 13 Jul 2022 05:40:42 +0200 Subject: [PATCH] =?UTF-8?q?Fixed=20query=20builder=20so=20that=20it=20does?= =?UTF-8?q?=20not=20treat=20embedded=20properties=20as=20=E2=80=A6=20(#268?= =?UTF-8?q?6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fixed query builder so that it does not treat embedded properties as relationships anymore. (#2684) * Removed unused imports. (#2684) * Replaced isRelationship() method with dictionary.isRelation(). (#2684) * Fix style formatting. (#2684) Co-authored-by: Lâm Tran Duy Co-authored-by: Aaron Klish --- .../RootCollectionFetchQueryBuilder.java | 10 +- .../RootCollectionFetchQueryBuilderTest.java | 102 ++++++++++++++++-- 2 files changed, 99 insertions(+), 13 deletions(-) diff --git a/elide-datastore/elide-datastore-jpql/src/main/java/com/yahoo/elide/datastores/jpql/query/RootCollectionFetchQueryBuilder.java b/elide-datastore/elide-datastore-jpql/src/main/java/com/yahoo/elide/datastores/jpql/query/RootCollectionFetchQueryBuilder.java index f8d65dc925..2dcdba0dbb 100644 --- a/elide-datastore/elide-datastore-jpql/src/main/java/com/yahoo/elide/datastores/jpql/query/RootCollectionFetchQueryBuilder.java +++ b/elide-datastore/elide-datastore-jpql/src/main/java/com/yahoo/elide/datastores/jpql/query/RootCollectionFetchQueryBuilder.java @@ -6,6 +6,7 @@ package com.yahoo.elide.datastores.jpql.query; import static com.yahoo.elide.core.utils.TypeHelper.getTypeAlias; + import com.yahoo.elide.core.dictionary.EntityDictionary; import com.yahoo.elide.core.exceptions.InvalidValueException; import com.yahoo.elide.core.filter.expression.FilterExpression; @@ -51,7 +52,7 @@ public Query build() { String filterClause = WHERE + new FilterTranslator(dictionary).apply(filterExpression, USE_ALIAS); //Build the JOIN clause - String joinClause = getJoinClauseFromFilters(filterExpression) + String joinClause = getJoinClauseFromFilters(filterExpression) + getJoinClauseFromSort(entityProjection.getSorting()) + extractToOneMergeJoins(entityClass, entityAlias); @@ -60,7 +61,12 @@ public Query build() { boolean sortOverRelationship = entityProjection.getSorting() != null && entityProjection.getSorting().getSortingPaths().keySet() - .stream().anyMatch(path -> path.getPathElements().size() > 1); + .stream().anyMatch(path -> + path.getPathElements() + .stream() + .anyMatch(element -> + dictionary.isRelation(element.getType(), element.getFieldName()))); + if (requiresDistinct && sortOverRelationship) { //SQL does not support distinct and order by on columns which are not selected throw new InvalidValueException("Combination of pagination, sorting over relationship and" diff --git a/elide-datastore/elide-datastore-jpql/src/test/java/com/yahoo/elide/datastores/hibernate/hql/RootCollectionFetchQueryBuilderTest.java b/elide-datastore/elide-datastore-jpql/src/test/java/com/yahoo/elide/datastores/hibernate/hql/RootCollectionFetchQueryBuilderTest.java index d7a6d5ed80..f5062d86c2 100644 --- a/elide-datastore/elide-datastore-jpql/src/test/java/com/yahoo/elide/datastores/hibernate/hql/RootCollectionFetchQueryBuilderTest.java +++ b/elide-datastore/elide-datastore-jpql/src/test/java/com/yahoo/elide/datastores/hibernate/hql/RootCollectionFetchQueryBuilderTest.java @@ -6,8 +6,11 @@ package com.yahoo.elide.datastores.hibernate.hql; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + import com.yahoo.elide.core.Path; import com.yahoo.elide.core.dictionary.EntityDictionary; +import com.yahoo.elide.core.exceptions.InvalidValueException; import com.yahoo.elide.core.filter.dialect.CaseSensitivityStrategy; import com.yahoo.elide.core.filter.dialect.ParseException; import com.yahoo.elide.core.filter.dialect.RSQLFilterDialect; @@ -23,16 +26,19 @@ import com.yahoo.elide.core.sort.SortingImpl; import com.yahoo.elide.core.type.ClassType; import com.yahoo.elide.datastores.jpql.query.RootCollectionFetchQueryBuilder; + import example.Author; import example.Book; import example.Chapter; import example.Editor; import example.Publisher; + import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import java.util.HashMap; +import java.util.List; import java.util.Map; @TestInstance(TestInstance.Lifecycle.PER_CLASS) @@ -45,6 +51,8 @@ public class RootCollectionFetchQueryBuilderTest { private static final String PERIOD = "."; private static final String NAME = "name"; private static final String FIRSTNAME = "firstName"; + private static final String PRICE = "price"; + private static final String TOTAL = "total"; private RSQLFilterDialect filterParser; @BeforeAll @@ -132,11 +140,11 @@ public void testRootFetchWithJoinFilter() throws ParseException { String expected = "SELECT example_Author FROM example.Author AS example_Author " - + "LEFT JOIN example_Author.books example_Author_books " - + "LEFT JOIN example_Author_books.chapters example_Author_books_chapters " - + "LEFT JOIN example_Author_books.publisher example_Author_books_publisher " - + "WHERE (example_Author_books_chapters.title IN (:books_chapters_title_XXX, :books_chapters_title_XXX) " - + "OR example_Author_books_publisher.name IN (:books_publisher_name_XXX)) "; + + "LEFT JOIN example_Author.books example_Author_books " + + "LEFT JOIN example_Author_books.chapters example_Author_books_chapters " + + "LEFT JOIN example_Author_books.publisher example_Author_books_publisher " + + "WHERE (example_Author_books_chapters.title IN (:books_chapters_title_XXX, :books_chapters_title_XXX) " + + "OR example_Author_books_publisher.name IN (:books_publisher_name_XXX)) "; String actual = query.getQueryText(); actual = actual.replaceFirst(":books_chapters_title_\\w\\w\\w\\w+", ":books_chapters_title_XXX"); @@ -172,11 +180,11 @@ public void testDistinctRootFetchWithToManyJoinFilterAndPagination() throws Pars String expected = "SELECT DISTINCT example_Author FROM example.Author AS example_Author " - + "LEFT JOIN example_Author.books example_Author_books " - + "LEFT JOIN example_Author_books.chapters example_Author_books_chapters " - + "LEFT JOIN example_Author_books.publisher example_Author_books_publisher " - + "WHERE (example_Author_books_chapters.title IN (:books_chapters_title_XXX, :books_chapters_title_XXX) " - + "OR example_Author_books_publisher.name IN (:books_publisher_name_XXX))"; + + "LEFT JOIN example_Author.books example_Author_books " + + "LEFT JOIN example_Author_books.chapters example_Author_books_chapters " + + "LEFT JOIN example_Author_books.publisher example_Author_books_publisher " + + "WHERE (example_Author_books_chapters.title IN (:books_chapters_title_XXX, :books_chapters_title_XXX) " + + "OR example_Author_books_publisher.name IN (:books_publisher_name_XXX))"; String actual = query.getQueryText(); actual = actual.trim().replaceAll(" +", " "); @@ -213,7 +221,7 @@ public void testRootFetchWithSortingAndFilters() { String expected = "SELECT example_Book FROM example.Book AS example_Book" - + " WHERE example_Book.id IN (:id_XXX) order by example_Book.title asc"; + + " WHERE example_Book.id IN (:id_XXX) order by example_Book.title asc"; String actual = query.getQueryText(); actual = actual.trim().replaceAll(" +", " "); @@ -314,4 +322,76 @@ public void testRootFetchWithToOneRelationIncluded() { assertEquals(expected, actual); } + + @Test + public void testRootFetchWithRelationshipSortingFiltersAndPagination() { + Map sorting = new HashMap<>(); + sorting.put(PUBLISHER + PERIOD + EDITOR + PERIOD + FIRSTNAME, Sorting.SortOrder.desc); + + Path.PathElement idBook = new Path.PathElement(Book.class, Chapter.class, "chapters"); + Path.PathElement idChapter = new Path.PathElement(Chapter.class, long.class, "id"); + Path idPath = new Path(List.of(new Path.PathElement[]{idBook, idChapter})); + + FilterPredicate chapterIdPredicate = new InPredicate(idPath, 1); + + PaginationImpl pagination = new PaginationImpl(ClassType.of(Book.class), 0, 3, 10, 10, true, false); + + EntityProjection entityProjection = EntityProjection + .builder().type(Book.class) + .pagination(pagination) + .sorting(new SortingImpl(sorting, Book.class, dictionary)) + .filterExpression(chapterIdPredicate) + .build(); + + RootCollectionFetchQueryBuilder builder = new RootCollectionFetchQueryBuilder( + entityProjection, + dictionary, + new TestSessionWrapper() + ); + + assertThrows(InvalidValueException.class, () -> { + TestQueryWrapper build = (TestQueryWrapper) builder.build(); + }); + } + + @Test + public void testRootFetchWithRelationshipSortingFiltersAndPaginationOnEmbedded() { + Map sorting = new HashMap<>(); + sorting.put(PRICE + PERIOD + TOTAL, Sorting.SortOrder.desc); + + Path.PathElement idBook = new Path.PathElement(Book.class, Chapter.class, "chapters"); + Path.PathElement idChapter = new Path.PathElement(Chapter.class, long.class, "id"); + Path idPath = new Path(List.of(new Path.PathElement[]{idBook, idChapter})); + + FilterPredicate chapterIdPredicate = new InPredicate(idPath, 1); + + PaginationImpl pagination = new PaginationImpl(ClassType.of(Book.class), 0, 3, 10, 10, true, false); + + EntityProjection entityProjection = EntityProjection + .builder().type(Book.class) + .pagination(pagination) + .sorting(new SortingImpl(sorting, Book.class, dictionary)) + .filterExpression(chapterIdPredicate) + .build(); + + RootCollectionFetchQueryBuilder builder = new RootCollectionFetchQueryBuilder( + entityProjection, + dictionary, + new TestSessionWrapper() + ); + + TestQueryWrapper query = (TestQueryWrapper) builder.build(); + + String expected = + "SELECT DISTINCT example_Book FROM example.Book AS example_Book" + + " LEFT JOIN example_Book.chapters example_Book_chapters" + + " WHERE example_Book_chapters.id IN (:chapters_id_XXX)" + + " order by example_Book.price.total desc"; + + String actual = query.getQueryText(); + actual = actual.trim().replaceAll(" +", " "); + actual = actual.replaceFirst(":chapters_id_\\w+", ":chapters_id_XXX"); + + assertEquals(expected, actual); + } }