From 0d638ff57378e4871688462cfc596dc6fa3f6409 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Wed, 2 Jan 2019 12:14:17 +0100 Subject: [PATCH] [#711] Test and fix for wrong pagination for partly constantified embedded id pagination --- .../impl/EmbeddableSplittingVisitor.java | 13 +-- .../FunctionalDependencyAnalyzerVisitor.java | 82 ++++++++++++++----- .../impl/PaginatedTypedQueryImpl.java | 10 ++- .../testsuite/PaginationEmbeddedIdTest.java | 37 +++++++++ 4 files changed, 115 insertions(+), 27 deletions(-) diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/EmbeddableSplittingVisitor.java b/core/impl/src/main/java/com/blazebit/persistence/impl/EmbeddableSplittingVisitor.java index e4e5d9aa97..7a64ab432f 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/EmbeddableSplittingVisitor.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/EmbeddableSplittingVisitor.java @@ -59,6 +59,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import java.util.TreeSet; import static com.blazebit.persistence.parser.util.JpaMetamodelUtils.ATTRIBUTE_NAME_COMPARATOR; @@ -120,7 +121,7 @@ protected boolean collectSplittedOffExpressions(Expression expression) { } String fieldPrefix = field == null ? "" : field + "."; ExtendedManagedType managedType = metamodel.getManagedType(ExtendedManagedType.class, baseNode.getJavaType()); - Set orderedAttributes = new TreeSet<>(); + Map orderedAttributes = new TreeMap<>(); EntityType ownerType; if (baseNode.getParentTreeNode() == null && field == null) { ownerType = baseNode.getEntityType(); @@ -142,7 +143,9 @@ protected boolean collectSplittedOffExpressions(Expression expression) { } else { ownedAttributes = managedType.getOwnedSingularAttributes(); } - orderedAttributes.addAll(JpaUtils.getEmbeddedPropertyPaths((Map>) ownedAttributes, prefix, false, false)); + for (String embeddedPropertyPath : JpaUtils.getEmbeddedPropertyPaths((Map>) ownedAttributes, prefix, false, false)) { + orderedAttributes.put(embeddedPropertyPath, Boolean.FALSE); + } } // Signal the caller that the expression was eliminated @@ -150,7 +153,7 @@ protected boolean collectSplittedOffExpressions(Expression expression) { return true; } - for (String orderedAttribute : orderedAttributes) { + for (String orderedAttribute : orderedAttributes.keySet()) { splittedOffExpressions.add(splittingVisitor.splitOff(expression, expressionToSplit, orderedAttribute)); } } @@ -195,7 +198,7 @@ public Boolean visit(PathExpression expr) { return true; } - protected void addAttributes(EntityType ownerType, String elementCollectionPath, String fieldPrefix, String prefix, SingularAttribute singularAttribute, Set orderedAttributes) { + protected void addAttributes(EntityType ownerType, String elementCollectionPath, String fieldPrefix, String prefix, SingularAttribute singularAttribute, Map orderedAttributes) { String attributeName; if (prefix.isEmpty()) { attributeName = singularAttribute.getName(); @@ -226,7 +229,7 @@ protected void addAttributes(EntityType ownerType, String elementCollectionPa } } } else { - orderedAttributes.add(attributeName); + orderedAttributes.put(attributeName, Boolean.FALSE); } } diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/FunctionalDependencyAnalyzerVisitor.java b/core/impl/src/main/java/com/blazebit/persistence/impl/FunctionalDependencyAnalyzerVisitor.java index e7c1b9dd90..ba8fc839d4 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/FunctionalDependencyAnalyzerVisitor.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/FunctionalDependencyAnalyzerVisitor.java @@ -60,7 +60,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -74,7 +73,7 @@ class FunctionalDependencyAnalyzerVisitor extends EmbeddableSplittingVisitor { private final ConstantifiedJoinNodeAttributeCollector constantifiedJoinNodeAttributeCollector; - private final Map> uniquenessMissingJoinNodeAttributes; + private final Map> uniquenessMissingJoinNodeAttributes; private final Map> uniquenessFormingJoinNodeExpressions; private final Map> functionalDependencyRootExpressions; private Object lastJoinNode; @@ -260,13 +259,13 @@ public Boolean visit(PathExpression expr) { boolean nonConstantParent = true; Set constantifiedAttributes = constantifiedJoinNodeAttributeCollector.getConstantifiedJoinNodeAttributes().get(baseNode); if (constantifiedAttributes != null) { - Set orderedAttributes = new HashSet<>(); + Map orderedAttributes = new HashMap<>(); addAttributes(baseNode.getEntityType(), null, "", "", (SingularAttribute) attribute, orderedAttributes); - - // If the identifiers are constantified, we don't care if this is a one-to-one - orderedAttributes.removeAll(constantifiedAttributes); + initConstantifiedAttributes(orderedAttributes, constantifiedAttributes); orderedAttributes.remove(expr.getField()); - if (orderedAttributes.isEmpty() || orderedAttributes.size() == 1 && equalsAny(orderedAttributes.iterator().next(), extendedManagedType.getAttribute(expr.getField()).getColumnEquivalentAttributes())) { + String singleNonConstantifiedAttribute = getSingleNonConstantifiedAttribute(orderedAttributes); + // If the identifiers are constantified, we don't care if this is a one-to-one + if (singleNonConstantifiedAttribute != null && (singleNonConstantifiedAttribute.isEmpty() || equalsAny(singleNonConstantifiedAttribute, extendedManagedType.getAttribute(expr.getField()).getColumnEquivalentAttributes()))) { nonConstantParent = false; orderedAttributes.clear(); managedType = metamodel.getManagedType(ExtendedManagedType.class, JpaMetamodelUtils.resolveFieldClass(baseNode.getJavaType(), attribute)); @@ -284,7 +283,7 @@ public Boolean visit(PathExpression expr) { registerFunctionalDependencyRootExpression(baseNodeKey); // First we initialize the names of the id attributes as set for the join node - Set orderedAttributes = getUniquenessMissingAttributes(baseNodeKey, managedType); + Map orderedAttributes = getUniquenessMissingAttributes(baseNodeKey, managedType); // We remove for every id attribute from the initialized set of id attribute names String prefix; @@ -310,7 +309,7 @@ public Boolean visit(PathExpression expr) { lastJoinNode = baseNodeKey; // While there still are some attribute names left, we simply report that it isn't unique, yet - if (!orderedAttributes.isEmpty()) { + if (hasNonConstantifiedAttribute(orderedAttributes)) { return false; } @@ -340,12 +339,13 @@ public Boolean visit(PathExpression expr) { } else { extendedManagedType = metamodel.getManagedType(ExtendedManagedType.class, baseNode.getManagedType().getJavaType()); } - orderedAttributes = new HashSet<>(); + orderedAttributes = new HashMap<>(); addAttributes(baseNode.getEntityType(), null, "", "", (SingularAttribute) attr, orderedAttributes); - orderedAttributes.removeAll(constantifiedAttributes); + initConstantifiedAttributes(orderedAttributes, constantifiedAttributes); orderedAttributes.remove(subPath); + String singleNonConstantifiedAttribute = getSingleNonConstantifiedAttribute(orderedAttributes); // If the identifiers are constantified, we don't care if this is a one-to-one - if (orderedAttributes.isEmpty() || orderedAttributes.size() == 1 && extendedManagedType.getAttributes().containsKey(subPath) && equalsAny(orderedAttributes.iterator().next(), extendedManagedType.getAttribute(subPath).getColumnEquivalentAttributes())) { + if (singleNonConstantifiedAttribute != null && (singleNonConstantifiedAttribute.isEmpty() || extendedManagedType.getAttributes().containsKey(subPath) && equalsAny(singleNonConstantifiedAttribute, extendedManagedType.getAttribute(subPath).getColumnEquivalentAttributes()))) { continue; } } @@ -358,6 +358,48 @@ public Boolean visit(PathExpression expr) { return true; } + private static boolean hasNonConstantifiedAttribute(Map orderedAttributes) { + for (Map.Entry entry : orderedAttributes.entrySet()) { + if (entry.getValue() == Boolean.FALSE) { + return true; + } + } + return false; + } + + private static void initConstantifiedAttributes(Map orderedAttributes, Set constantifiedAttributes) { + for (String constantifiedAttribute : constantifiedAttributes) { + if (orderedAttributes.containsKey(constantifiedAttribute)) { + orderedAttributes.put(constantifiedAttribute, Boolean.TRUE); + } + } + } + + private String getSingleNonConstantifiedAttribute(Map orderedAttributes) { + // Null means there are multiple or no constantified attributes + if (orderedAttributes.isEmpty()) { + // The empty string signals no attributes + return ""; + } + + String attribute = null; + for (Map.Entry entry : orderedAttributes.entrySet()) { + if (entry.getValue() == Boolean.FALSE) { + if (attribute == null) { + attribute = entry.getKey(); + } else { + return null; + } + } + } + + // Is the case of having only constantified attributes even reasonable? +// if (attribute == null) { +// return ""; +// } + return attribute; + } + private boolean equalsAny(String attribute, Set> columnEquivalentAttributes) { StringBuilder sb = null; for (ExtendedAttribute columnEquivalentAttribute : columnEquivalentAttributes) { @@ -386,18 +428,18 @@ private boolean equalsAny(String attribute, Set getUniquenessMissingAttributes(Object baseNodeKey, ExtendedManagedType managedType) { - Set orderedAttributes = uniquenessMissingJoinNodeAttributes.get(baseNodeKey); + private Map getUniquenessMissingAttributes(Object baseNodeKey, ExtendedManagedType managedType) { + Map orderedAttributes = uniquenessMissingJoinNodeAttributes.get(baseNodeKey); if (orderedAttributes == null) { - orderedAttributes = new HashSet<>(); + orderedAttributes = new HashMap<>(); JoinNode baseNode; EntityType entityType; String prefix; if (baseNodeKey instanceof Map.Entry) { baseNode = (JoinNode) ((Map.Entry) baseNodeKey).getKey(); - String assocationName = (String) ((Map.Entry) baseNodeKey).getValue(); + String associationName = (String) ((Map.Entry) baseNodeKey).getValue(); entityType = baseNode.getEntityType(); - prefix = assocationName + "."; + prefix = associationName + "."; } else { baseNode = (JoinNode) baseNodeKey; entityType = baseNode.getEntityType(); @@ -415,14 +457,14 @@ private Set getUniquenessMissingAttributes(Object baseNodeKey, ExtendedM } Set constantifiedAttributes = constantifiedJoinNodeAttributeCollector.getConstantifiedJoinNodeAttributes().get(baseNodeKey); if (constantifiedAttributes != null) { - orderedAttributes.removeAll(constantifiedAttributes); + initConstantifiedAttributes(orderedAttributes, constantifiedAttributes); } uniquenessMissingJoinNodeAttributes.put(baseNodeKey, orderedAttributes); } return orderedAttributes; } - private boolean removeAttribute(String prefix, SingularAttribute singularAttribute, Set orderedAttributes) { + private boolean removeAttribute(String prefix, SingularAttribute singularAttribute, Map orderedAttributes) { String attributeName; if (prefix.isEmpty()) { attributeName = singularAttribute.getName(); @@ -440,7 +482,7 @@ private boolean removeAttribute(String prefix, SingularAttribute singularA } return removed; } else { - return orderedAttributes.remove(attributeName); + return orderedAttributes.remove(attributeName) != null; } } diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedTypedQueryImpl.java b/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedTypedQueryImpl.java index 37a8c87de6..314f49052d 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedTypedQueryImpl.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedTypedQueryImpl.java @@ -249,8 +249,14 @@ private PagedList getResultList(int queryFirstResult, int firstRow, long tota StringBuilder parameterNameBuilder = new StringBuilder(AbstractCommonQueryBuilder.ID_PARAM_NAME.length() + 10); parameterNameBuilder.append(AbstractCommonQueryBuilder.ID_PARAM_NAME).append('_'); int start = parameterNameBuilder.length(); - for (int i = 0; i < ids.size(); i++) { - Object[] tuple = (Object[]) ids.get(i); + Object[] empty = ids.size() < pageSize ? new Object[identifierCount] : null; + for (int i = 0; i < pageSize; i++) { + Object[] tuple; + if (ids.size() > i) { + tuple = (Object[]) ids.get(i); + } else { + tuple = empty; + } for (int j = 0; j < identifierCount; j++) { parameterNameBuilder.setLength(start); parameterNameBuilder.append(j).append('_').append(i); diff --git a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/PaginationEmbeddedIdTest.java b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/PaginationEmbeddedIdTest.java index 5b9e307c10..2f5d73cbdb 100644 --- a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/PaginationEmbeddedIdTest.java +++ b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/PaginationEmbeddedIdTest.java @@ -31,6 +31,7 @@ import org.junit.experimental.categories.Category; import javax.persistence.EntityManager; +import javax.persistence.Tuple; import static org.junit.Assert.assertEquals; @@ -70,11 +71,23 @@ public void work(EntityManager em) { entity2.getId().setValue("e2"); entity2.setVersion(1L); + EmbeddableTestEntity entity3 = new EmbeddableTestEntity(); + entity3.getId().setKey("e3"); + entity3.getId().setValue("a"); + entity3.setVersion(1L); + + EmbeddableTestEntity entity4 = new EmbeddableTestEntity(); + entity4.getId().setKey("e4"); + entity4.getId().setValue("a"); + entity4.setVersion(1L); + IntIdEntity intIdEntity1 = new IntIdEntity("i1", 1); IntIdEntity intIdEntity2 = new IntIdEntity("i2", 2); em.persist(entity1); em.persist(entity2); + em.persist(entity3); + em.persist(entity4); em.persist(intIdEntity1); em.persist(intIdEntity2); @@ -82,6 +95,10 @@ public void work(EntityManager em) { entity1.getEmbeddable().getElementCollection().put("test2", new NameObject("test", "b", intIdEntity2)); entity2.getEmbeddable().getElementCollection().put("test", new NameObject("test", "b", intIdEntity1)); entity2.getEmbeddable().getElementCollection().put("test2", new NameObject("test", "b", intIdEntity2)); + entity3.getEmbeddable().getElementCollection().put("test", new NameObject("test", "b", intIdEntity1)); + entity3.getEmbeddable().getElementCollection().put("test2", new NameObject("test", "b", intIdEntity2)); + entity4.getEmbeddable().getElementCollection().put("test", new NameObject("test", "b", intIdEntity1)); + entity4.getEmbeddable().getElementCollection().put("test2", new NameObject("test", "b", intIdEntity2)); } }); } @@ -210,4 +227,24 @@ public void keysetPaginateById3() { String queryString = crit.page(0, 1).getQueryString(); assertEquals(expectedObjectQuery, queryString); } + + @Test + // Test for #711 + public void paginateWithConstantifiedIdPart() { + CriteriaBuilder crit = cbf.create(em, Tuple.class) + .from(EmbeddableTestEntity.class, "e"); + crit.select("e.id").select("e.embeddable.elementCollection.primaryName"); + crit.where("e.id.key").eq("e3"); + crit.orderByAsc("e.id.key"); + crit.orderByAsc("e.id.value"); + PaginatedCriteriaBuilder pcb = crit.pageBy(0, 1, "e.id.key", "e.id.value"); + + String expectedObjectQuery = "SELECT e.id, elementCollection_1.primaryName FROM EmbeddableTestEntity e LEFT JOIN e.embeddable.elementCollection elementCollection_1" + + " WHERE (e.id.key = :ids_0_0 AND e.id.value = :ids_1_0)" + + " ORDER BY e.id.key ASC, e.id.value ASC"; + String queryString = pcb.getQueryString(); + assertEquals(expectedObjectQuery, queryString); + PagedList list = pcb.getResultList(); + assertEquals(2, list.size()); + } }