Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#711] Test and fix for wrong pagination for partly constantified embedded id pagination #714

Merged
merged 1 commit into from
Jan 3, 2019
Merged

[#711] Test and fix for wrong pagination for partly constantified embedded id pagination #714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions core/impl/src/main/java/com/blazebit/persistence/impl/EmbeddableSplittingVisitor.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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());
}
}