Skip to content

Commit

Permalink
[Blazebit#711] Test and fix for wrong pagination for partly constanti…
Browse files Browse the repository at this point in the history 
…fied embedded id pagination
  • Loading branch information
@beikov
beikov committed Jan 2, 2019
1 parent 539b478 commit 92c749b
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 27 deletions.
13 changes: 8 additions & 5 deletions core/impl/src/main/java/com/blazebit/persistence/impl/EmbeddableSplittingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -120,7 +121,7 @@ protected boolean collectSplittedOffExpressions(Expression expression) {
}
String fieldPrefix = field == null ? "" : field + ".";
ExtendedManagedType<?> managedType = metamodel.getManagedType(ExtendedManagedType.class, baseNode.getJavaType());
Set<String> orderedAttributes = new TreeSet<>();
Map<String, Boolean> orderedAttributes = new TreeMap<>();
EntityType<?> ownerType;
if (baseNode.getParentTreeNode() == null && field == null) {
ownerType = baseNode.getEntityType();
Expand All @@ -142,15 +143,17 @@ protected boolean collectSplittedOffExpressions(Expression expression) {
} else {
ownedAttributes = managedType.getOwnedSingularAttributes();
}
orderedAttributes.addAll(JpaUtils.getEmbeddedPropertyPaths((Map<String, ExtendedAttribute<?, ?>>) ownedAttributes, prefix, false, false));
for (String embeddedPropertyPath : JpaUtils.getEmbeddedPropertyPaths((Map<String, ExtendedAttribute<?, ?>>) ownedAttributes, prefix, false, false)) {
orderedAttributes.put(embeddedPropertyPath, Boolean.FALSE);
}
}

// Signal the caller that the expression was eliminated
if (orderedAttributes.isEmpty()) {
return true;
}

for (String orderedAttribute : orderedAttributes) {
for (String orderedAttribute : orderedAttributes.keySet()) {
splittedOffExpressions.add(splittingVisitor.splitOff(expression, expressionToSplit, orderedAttribute));
}
}
Expand Down Expand Up @@ -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<String> orderedAttributes) {
protected void addAttributes(EntityType<?> ownerType, String elementCollectionPath, String fieldPrefix, String prefix, SingularAttribute<?, ?> singularAttribute, Map<String, Boolean> orderedAttributes) {
String attributeName;
if (prefix.isEmpty()) {
attributeName = singularAttribute.getName();
Expand Down Expand Up @@ -226,7 +229,7 @@ protected void addAttributes(EntityType<?> ownerType, String elementCollectionPa
}
}
} else {
orderedAttributes.add(attributeName);
orderedAttributes.put(attributeName, Boolean.FALSE);
}
}

Expand Down
82 changes: 62 additions & 20 deletions ...impl/src/main/java/com/blazebit/persistence/impl/FunctionalDependencyAnalyzerVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -74,7 +73,7 @@
class FunctionalDependencyAnalyzerVisitor extends EmbeddableSplittingVisitor {

private final ConstantifiedJoinNodeAttributeCollector constantifiedJoinNodeAttributeCollector;
private final Map<Object, Set<String>> uniquenessMissingJoinNodeAttributes;
private final Map<Object, Map<String, Boolean>> uniquenessMissingJoinNodeAttributes;
private final Map<Object, List<ResolvedExpression>> uniquenessFormingJoinNodeExpressions;
private final Map<Object, List<ResolvedExpression>> functionalDependencyRootExpressions;
private Object lastJoinNode;
Expand Down Expand Up @@ -260,13 +259,13 @@ public Boolean visit(PathExpression expr) {
boolean nonConstantParent = true;
Set<String> constantifiedAttributes = constantifiedJoinNodeAttributeCollector.getConstantifiedJoinNodeAttributes().get(baseNode);
if (constantifiedAttributes != null) {
Set<String> orderedAttributes = new HashSet<>();
Map<String, Boolean> 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));
Expand All @@ -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<String> orderedAttributes = getUniquenessMissingAttributes(baseNodeKey, managedType);
Map<String, Boolean> orderedAttributes = getUniquenessMissingAttributes(baseNodeKey, managedType);

// We remove for every id attribute from the initialized set of id attribute names
String prefix;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}
}
Expand All @@ -358,6 +358,48 @@ public Boolean visit(PathExpression expr) {
return true;
}

private static boolean hasNonConstantifiedAttribute(Map<String, Boolean> orderedAttributes) {
for (Map.Entry<String, Boolean> entry : orderedAttributes.entrySet()) {
if (entry.getValue() == Boolean.FALSE) {
return true;
}
}
return false;
}

private static void initConstantifiedAttributes(Map<String, Boolean> orderedAttributes, Set<String> constantifiedAttributes) {
for (String constantifiedAttribute : constantifiedAttributes) {
if (orderedAttributes.containsKey(constantifiedAttribute)) {
orderedAttributes.put(constantifiedAttribute, Boolean.TRUE);
}
}
}

private String getSingleNonConstantifiedAttribute(Map<String, Boolean> 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<String, Boolean> 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<? extends ExtendedAttribute<?, ?>> columnEquivalentAttributes) {
StringBuilder sb = null;
for (ExtendedAttribute<?, ?> columnEquivalentAttribute : columnEquivalentAttributes) {
Expand Down Expand Up @@ -386,18 +428,18 @@ private boolean equalsAny(String attribute, Set<? extends ExtendedAttribute<?, ?
return false;
}

private Set<String> getUniquenessMissingAttributes(Object baseNodeKey, ExtendedManagedType<?> managedType) {
Set<String> orderedAttributes = uniquenessMissingJoinNodeAttributes.get(baseNodeKey);
private Map<String, Boolean> getUniquenessMissingAttributes(Object baseNodeKey, ExtendedManagedType<?> managedType) {
Map<String, Boolean> 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();
Expand All @@ -415,14 +457,14 @@ private Set<String> getUniquenessMissingAttributes(Object baseNodeKey, ExtendedM
}
Set<String> 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<String> orderedAttributes) {
private boolean removeAttribute(String prefix, SingularAttribute<?, ?> singularAttribute, Map<String, Boolean> orderedAttributes) {
String attributeName;
if (prefix.isEmpty()) {
attributeName = singularAttribute.getName();
Expand All @@ -440,7 +482,7 @@ private boolean removeAttribute(String prefix, SingularAttribute<?, ?> singularA
}
return removed;
} else {
return orderedAttributes.remove(attributeName);
return orderedAttributes.remove(attributeName) != null;
}
}

Expand Down
10 changes: 8 additions & 2 deletions core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedTypedQueryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,14 @@ private PagedList<X> 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);
Expand Down
37 changes: 37 additions & 0 deletions .../testsuite/src/test/java/com/blazebit/persistence/testsuite/PaginationEmbeddedIdTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.junit.experimental.categories.Category;

import javax.persistence.EntityManager;
import javax.persistence.Tuple;

import static org.junit.Assert.assertEquals;

Expand Down Expand Up @@ -70,18 +71,34 @@ 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);

entity1.getEmbeddable().getElementCollection().put("test", new NameObject("test", "b", intIdEntity1));
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));
}
});
}
Expand Down Expand Up @@ -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<Tuple> 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<Tuple> 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<Tuple> list = pcb.getResultList();
assertEquals(2, list.size());
}
}

0 comments on commit 92c749b

Please sign in to comment.