Repository: cayenne Updated Branches: refs/heads/master 8e43461e4 -> 29428fcb5
CAY-1551 orderings with "+." in the path fail when performing in memory sort Project: http://git-wip-us.apache.org/repos/asf/cayenne/repo Commit: http://git-wip-us.apache.org/repos/asf/cayenne/commit/29428fcb Tree: http://git-wip-us.apache.org/repos/asf/cayenne/tree/29428fcb Diff: http://git-wip-us.apache.org/repos/asf/cayenne/diff/29428fcb Branch: refs/heads/master Commit: 29428fcb57630241fa58dc6afdb9371016612ad9 Parents: 8e43461 Author: Nikita Timofeev <stari...@gmail.com> Authored: Tue Feb 7 15:15:22 2017 +0300 Committer: Nikita Timofeev <stari...@gmail.com> Committed: Tue Feb 7 15:15:22 2017 +0300 ---------------------------------------------------------------------- .../java/org/apache/cayenne/map/Entity.java | 153 +++++++++---------- .../apache/cayenne/reflect/PropertyUtils.java | 25 +-- .../apache/cayenne/exp/parser/ASTObjPathIT.java | 10 ++ .../cayenne/exp/parser/ASTObjPathTest.java | 4 +- .../org/apache/cayenne/query/OrderingTest.java | 17 +++ .../cayenne/reflect/PropertyUtilsTest.java | 38 +++-- 6 files changed, 138 insertions(+), 109 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cayenne/blob/29428fcb/cayenne-server/src/main/java/org/apache/cayenne/map/Entity.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/map/Entity.java b/cayenne-server/src/main/java/org/apache/cayenne/map/Entity.java index e687964..d62bc4a 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/map/Entity.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/map/Entity.java @@ -55,8 +55,8 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial protected String name; protected DataMap dataMap; - protected SortedMap<String, Attribute> attributes; - protected SortedMap<String, Relationship> relationships; + protected final SortedMap<String, Attribute> attributes = new TreeMap<>(); + protected final SortedMap<String, Relationship> relationships = new TreeMap<>(); /** * Creates an unnamed Entity. @@ -69,9 +69,6 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial * Creates a named Entity. */ public Entity(String name) { - attributes = new TreeMap<String, Attribute>(); - relationships = new TreeMap<String, Relationship>(); - setName(name); } @@ -96,8 +93,9 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial } public void setParent(Object parent) { - if (parent != null && !(parent instanceof DataMap)) + if (parent != null && !(parent instanceof DataMap)) { throw new IllegalArgumentException("Expected null or DataMap, got: " + parent); + } setDataMap((DataMap) parent); } @@ -129,37 +127,37 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial * attribute has no name, IllegalArgumentException is thrown. */ public void addAttribute(Attribute attribute) { - if (attribute.getName() == null) + if (attribute.getName() == null) { throw new IllegalArgumentException("Attempt to insert unnamed attribute."); + } // block overrides - // TODO: change method signature to return replaced attribute and make sure the - // Modeler handles it... + // TODO: change method signature to return replaced attribute and make sure the Modeler handles it... Object existingAttribute = attributes.get(attribute.getName()); if (existingAttribute != null) { - if (existingAttribute == attribute) + if (existingAttribute == attribute) { return; - else - throw new IllegalArgumentException("An attempt to override attribute '" - + attribute.getName() - + "'"); + } else { + throw new IllegalArgumentException("An attempt to override attribute '" + attribute.getName() + "'"); + } } // Check that there aren't any relationships with the same name as the given // attribute. Object existingRelationship = relationships.get(attribute.getName()); - if (existingRelationship != null) + if (existingRelationship != null) { throw new IllegalArgumentException( - "Attribute name conflict with existing relationship '" - + attribute.getName() - + "'"); + "Attribute name conflict with existing relationship '" + attribute.getName() + "'"); + } attributes.put(attribute.getName(), attribute); attribute.setEntity(this); } - /** Removes an attribute named <code>attrName</code>. */ + /** + * Removes an attribute named <code>attrName</code>. + */ public void removeAttribute(String attrName) { attributes.remove(attrName); } @@ -185,10 +183,13 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial return relationships.get(relName); } - /** Adds new relationship to the entity. */ + /** + * Adds new relationship to the entity. + */ public void addRelationship(Relationship relationship) { - if (relationship.getName() == null) + if (relationship.getName() == null) { throw new IllegalArgumentException("Attempt to insert unnamed relationship."); + } // block overrides @@ -196,29 +197,29 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial // Modeler handles it... Object existingRelationship = relationships.get(relationship.getName()); if (existingRelationship != null) { - if (existingRelationship == relationship) + if (existingRelationship == relationship) { return; - else + } else { throw new IllegalArgumentException( - "An attempt to override relationship '" - + relationship.getName() - + "'"); + "An attempt to override relationship '" + relationship.getName() + "'"); + } } // Check that there aren't any attributes with the same name as the given // relationship. Object existingAttribute = attributes.get(relationship.getName()); - if (existingAttribute != null) + if (existingAttribute != null) { throw new IllegalArgumentException( - "Relationship name conflict with existing attribute '" - + relationship.getName() - + "'"); + "Relationship name conflict with existing attribute '" + relationship.getName() + "'"); + } relationships.put(relationship.getName(), relationship); relationship.setSourceEntity(this); } - /** Removes a relationship named <code>attrName</code>. */ + /** + * Removes a relationship named <code>attrName</code>. + */ public void removeRelationship(String relName) { relationships.remove(relName); } @@ -244,12 +245,14 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial * @since 1.1 */ public Relationship getAnyRelationship(Entity targetEntity) { - if (getRelationships().isEmpty()) + if (getRelationships().isEmpty()) { return null; + } for (Relationship r : getRelationships()) { - if (r.getTargetEntity() == targetEntity) + if (r.getTargetEntity() == targetEntity) { return r; + } } return null; } @@ -287,9 +290,7 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial * * @since 1.1 */ - public abstract Expression translateToRelatedEntity( - Expression expression, - String relationshipPath); + public abstract Expression translateToRelatedEntity(Expression expression, String relationshipPath); /** * Convenience method returning the last component in the path iterator. If the last @@ -304,7 +305,6 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial for (PathComponent component : resolvePath(path, aliasMap)) { if (component.isLast()) { - // resolve aliases if needed return lastPathComponent(component); } @@ -314,8 +314,7 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial } @SuppressWarnings("unchecked") - private PathComponent lastPathComponent( - PathComponent<Attribute, Relationship> component) { + private PathComponent lastPathComponent(PathComponent<Attribute, Relationship> component) { if (!component.isAlias()) { return component; @@ -327,8 +326,7 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial } } - throw new IllegalStateException("Invalid last path component: " - + component.getName()); + throw new IllegalStateException("Invalid last path component: " + component.getName()); } /** @@ -356,8 +354,7 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial * return an Iterator, but an attempt to read the first invalid path component will * result in ExpressionException. */ - public abstract Iterator<CayenneMapEntry> resolvePathComponents(Expression pathExp) - throws ExpressionException; + public abstract Iterator<CayenneMapEntry> resolvePathComponents(Expression pathExp) throws ExpressionException; /** * Returns an Iterator over the path components that contains a sequence of Attributes @@ -365,81 +362,69 @@ public abstract class Entity implements CayenneMapEntry, XMLSerializable, Serial * entity, this method will still return an Iterator, but an attempt to read the first * invalid path component will result in ExpressionException. */ - public Iterator<CayenneMapEntry> resolvePathComponents(String path) - throws ExpressionException { + public Iterator<CayenneMapEntry> resolvePathComponents(String path) throws ExpressionException { return new PathIterator(path); } - // An iterator resolving mapping components represented by the path string. - // This entity is assumed to be the root of the path. + /** + * An iterator resolving mapping components represented by the path string. + * This entity is assumed to be the root of the path. + */ final class PathIterator implements Iterator<CayenneMapEntry> { - private StringTokenizer toks; - private Entity currentEnt; - private String path; + private final StringTokenizer tokens; + private final String path; + private Entity currentEntity; PathIterator(String path) { - super(); - currentEnt = Entity.this; - toks = new StringTokenizer(path, PATH_SEPARATOR); + currentEntity = Entity.this; + tokens = new StringTokenizer(path, PATH_SEPARATOR); this.path = path; } public boolean hasNext() { - return toks.hasMoreTokens(); + return tokens.hasMoreTokens(); } public CayenneMapEntry next() { - String pathComp = toks.nextToken(); + String pathComp = tokens.nextToken(); + if(pathComp.endsWith(OUTER_JOIN_INDICATOR)) { + pathComp = pathComp.substring(0, pathComp.length() - 1); + } // see if this is an attribute - Attribute attr = currentEnt.getAttribute(pathComp); + Attribute attr = currentEntity.getAttribute(pathComp); if (attr != null) { // do a sanity check... - if (toks.hasMoreTokens()) - throw new ExpressionException( - "Attribute must be the last component of the path: '" - + pathComp - + "'.", - path, - null); - + if (tokens.hasMoreTokens()) { + throw new ExpressionException("Attribute must be the last component of the path: '%s'.", + path, null, pathComp); + } return attr; } - Relationship rel = currentEnt.getRelationship(pathComp); + Relationship rel = currentEntity.getRelationship(pathComp); if (rel != null) { - currentEnt = rel.getTargetEntity(); - if (currentEnt != null || !toks.hasMoreTokens()) { //otherwise an exception will be thrown + currentEntity = rel.getTargetEntity(); + if (currentEntity != null || !tokens.hasMoreTokens()) { //otherwise an exception will be thrown return rel; } } - String entityName = (currentEnt != null) ? currentEnt.getName() : "(?)"; - - // build error message - StringBuilder buf = new StringBuilder(); - buf - .append("Can't resolve path component: [") - .append(entityName) - .append('.') - .append(pathComp) - .append("]."); - throw new ExpressionException(buf.toString(), path, null); + String entityName = (currentEntity != null) ? currentEntity.getName() : "(?)"; + throw new ExpressionException("Can't resolve path component: [%s.%s].", path, null, entityName, pathComp); } public void remove() { - throw new UnsupportedOperationException( - "'remove' operation is not supported."); + throw new UnsupportedOperationException("'remove' operation is not supported."); } } final MappingNamespace getNonNullNamespace() { MappingNamespace parent = getDataMap(); - if (parent == null) - throw new CayenneRuntimeException("Entity '" - + getName() - + "' has no parent MappingNamespace (such as DataMap)"); + if (parent == null) { + throw new CayenneRuntimeException("Entity '%s' has no parent MappingNamespace (such as DataMap)", getName()); + } return parent; } http://git-wip-us.apache.org/repos/asf/cayenne/blob/29428fcb/cayenne-server/src/main/java/org/apache/cayenne/reflect/PropertyUtils.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/reflect/PropertyUtils.java b/cayenne-server/src/main/java/org/apache/cayenne/reflect/PropertyUtils.java index f9d840f..c701331 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/reflect/PropertyUtils.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/reflect/PropertyUtils.java @@ -176,19 +176,19 @@ public class PropertyUtils { String className = type.getName(); if ("byte".equals(className)) { - return Byte.valueOf((byte) 0); + return (byte) 0; } else if ("int".equals(className)) { - return Integer.valueOf(0); + return 0; } else if ("long".equals(className)) { - return Long.valueOf(0); + return 0L; } else if ("short".equals(className)) { - return Short.valueOf((short) 0); + return (short) 0; } else if ("char".equals(className)) { - return Character.valueOf((char) 0); + return (char) 0; } else if ("double".equals(className)) { - return new Double(0.0d); + return 0.0d; } else if ("float".equals(className)) { - return new Float(0.0f); + return 0.0f; } else if ("boolean".equals(className)) { return Boolean.FALSE; } @@ -205,11 +205,16 @@ public class PropertyUtils { private static final long serialVersionUID = 2056090443413498626L; - private String segmentName; - private Accessor nextAccessor; + private final String segmentName; + private final Accessor nextAccessor; public PathAccessor(String segmentName, Accessor nextAccessor) { - this.segmentName = segmentName; + // trim outer join component + if(segmentName.endsWith(Entity.OUTER_JOIN_INDICATOR)) { + this.segmentName = segmentName.substring(0, segmentName.length() - 1); + } else { + this.segmentName = segmentName; + } this.nextAccessor = nextAccessor; } http://git-wip-us.apache.org/repos/asf/cayenne/blob/29428fcb/cayenne-server/src/test/java/org/apache/cayenne/exp/parser/ASTObjPathIT.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/test/java/org/apache/cayenne/exp/parser/ASTObjPathIT.java b/cayenne-server/src/test/java/org/apache/cayenne/exp/parser/ASTObjPathIT.java index 2dc3c57..5955606 100644 --- a/cayenne-server/src/test/java/org/apache/cayenne/exp/parser/ASTObjPathIT.java +++ b/cayenne-server/src/test/java/org/apache/cayenne/exp/parser/ASTObjPathIT.java @@ -46,4 +46,14 @@ public class ASTObjPathIT extends ServerCase { assertTrue(target instanceof ObjAttribute); } + @Test + public void testEvaluate_ObjEntity_Outer() { + ASTObjPath node = new ASTObjPath("paintingArray+.paintingTitle"); + + ObjEntity ae = context.getEntityResolver().getObjEntity(Artist.class); + + Object target = node.evaluate(ae); + assertTrue(target instanceof ObjAttribute); + } + } http://git-wip-us.apache.org/repos/asf/cayenne/blob/29428fcb/cayenne-server/src/test/java/org/apache/cayenne/exp/parser/ASTObjPathTest.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/test/java/org/apache/cayenne/exp/parser/ASTObjPathTest.java b/cayenne-server/src/test/java/org/apache/cayenne/exp/parser/ASTObjPathTest.java index 655b10e..e63a549 100644 --- a/cayenne-server/src/test/java/org/apache/cayenne/exp/parser/ASTObjPathTest.java +++ b/cayenne-server/src/test/java/org/apache/cayenne/exp/parser/ASTObjPathTest.java @@ -69,10 +69,10 @@ public class ASTObjPathTest { TstBean b1 = new TstBean(); b1.setProperty2(1); - assertEquals(new Integer(1), node.evaluate(b1)); + assertEquals(1, node.evaluate(b1)); TstBean b2 = new TstBean(); b2.setProperty2(-3); - assertEquals(new Integer(-3), node.evaluate(b2)); + assertEquals(-3, node.evaluate(b2)); } } http://git-wip-us.apache.org/repos/asf/cayenne/blob/29428fcb/cayenne-server/src/test/java/org/apache/cayenne/query/OrderingTest.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/test/java/org/apache/cayenne/query/OrderingTest.java b/cayenne-server/src/test/java/org/apache/cayenne/query/OrderingTest.java index 66cb4a1..bc275dc 100644 --- a/cayenne-server/src/test/java/org/apache/cayenne/query/OrderingTest.java +++ b/cayenne-server/src/test/java/org/apache/cayenne/query/OrderingTest.java @@ -196,6 +196,23 @@ public class OrderingTest { assertEquals("three", ordered.get(2).getName()); } + /** + * CAY-1551 + */ + @Test + public void testOrderList_OuterRelated() { + List<B1> unordered = asList( + new B1().setName("three").setB2(new B2().setName("Z")), + new B1().setName("one").setB2(new B2().setName("A")), + new B1().setName("two").setB2(new B2().setName("M")) + ); + + List<B1> ordered = new Ordering("b2+.name", SortOrder.ASCENDING).orderedList(unordered); + assertEquals("one", ordered.get(0).getName()); + assertEquals("two", ordered.get(1).getName()); + assertEquals("three", ordered.get(2).getName()); + } + @Test public void testOrderList_Static() { List<TstBean> list = new ArrayList<>(6); http://git-wip-us.apache.org/repos/asf/cayenne/blob/29428fcb/cayenne-server/src/test/java/org/apache/cayenne/reflect/PropertyUtilsTest.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/test/java/org/apache/cayenne/reflect/PropertyUtilsTest.java b/cayenne-server/src/test/java/org/apache/cayenne/reflect/PropertyUtilsTest.java index 0769f25..860f001 100644 --- a/cayenne-server/src/test/java/org/apache/cayenne/reflect/PropertyUtilsTest.java +++ b/cayenne-server/src/test/java/org/apache/cayenne/reflect/PropertyUtilsTest.java @@ -115,11 +115,11 @@ public class PropertyUtilsTest { assertSame(o1.getByteArrayField(), PropertyUtils.getProperty(o1, "byteArrayField")); assertSame(o1.getIntegerField(), PropertyUtils.getProperty(o1, "integerField")); - assertEquals(new Integer(o1.getIntField()), PropertyUtils.getProperty(o1, "intField")); + assertEquals(o1.getIntField(), PropertyUtils.getProperty(o1, "intField")); assertSame(o1.getNumberField(), PropertyUtils.getProperty(o1, "numberField")); assertSame(o1.getObjectField(), PropertyUtils.getProperty(o1, "objectField")); assertSame(o1.getStringField(), PropertyUtils.getProperty(o1, "stringField")); - assertEquals(Boolean.valueOf(o1.isBooleanField()), PropertyUtils.getProperty(o1, "booleanField")); + assertEquals(o1.isBooleanField(), PropertyUtils.getProperty(o1, "booleanField")); } @Test @@ -128,10 +128,22 @@ public class PropertyUtilsTest { assertNull(PropertyUtils.getProperty(o1, "related.integerField")); TstJavaBean o1related = new TstJavaBean(); - o1related.setIntegerField(Integer.valueOf(44)); + o1related.setIntegerField(44); o1.setRelated(o1related); - assertEquals(Integer.valueOf(44), PropertyUtils.getProperty(o1, "related.integerField")); + assertEquals(44, PropertyUtils.getProperty(o1, "related.integerField")); + } + + @Test + public void testGetProperty_NestedOuter() { + TstJavaBean o1 = createBean(); + assertNull(PropertyUtils.getProperty(o1, "related+.integerField")); + + TstJavaBean o1related = new TstJavaBean(); + o1related.setIntegerField(42); + o1.setRelated(o1related); + + assertEquals(42, PropertyUtils.getProperty(o1, "related+.integerField")); } @Test @@ -141,11 +153,11 @@ public class PropertyUtilsTest { PropertyUtils.setProperty(o2, "byteArrayField", o1.getByteArrayField()); PropertyUtils.setProperty(o2, "integerField", o1.getIntegerField()); - PropertyUtils.setProperty(o2, "intField", new Integer(o1.getIntField())); + PropertyUtils.setProperty(o2, "intField", o1.getIntField()); PropertyUtils.setProperty(o2, "numberField", o1.getNumberField()); PropertyUtils.setProperty(o2, "objectField", o1.getObjectField()); PropertyUtils.setProperty(o2, "stringField", o1.getStringField()); - PropertyUtils.setProperty(o2, "booleanField", Boolean.valueOf(o1.isBooleanField())); + PropertyUtils.setProperty(o2, "booleanField", o1.isBooleanField()); } @Test @@ -165,7 +177,7 @@ public class PropertyUtilsTest { public void testSetProperty_Nested() { TstJavaBean o1 = createBean(); TstJavaBean o1related = new TstJavaBean(); - o1related.setIntegerField(Integer.valueOf(44)); + o1related.setIntegerField(44); o1.setRelated(o1related); PropertyUtils.setProperty(o1, "related.integerField", 55); @@ -260,7 +272,7 @@ public class PropertyUtilsTest { // arbitrary string/object to field PropertyUtils.setProperty(o1, "stringBuilderField", "abc"); - assertEquals(new StringBuilder("abc").toString(), o1.getStringBuilderField().toString()); + assertEquals("abc", o1.getStringBuilderField().toString()); } @Test @@ -353,10 +365,10 @@ public class PropertyUtilsTest { assertEquals(445, o1.getNumber()); } - protected TstJavaBean createBean() { + private TstJavaBean createBean() { TstJavaBean o1 = new TstJavaBean(); o1.setByteArrayField(new byte[] { 1, 2, 3 }); - o1.setIntegerField(new Integer(33)); + o1.setIntegerField(33); o1.setIntField(-44); o1.setNumberField(new BigDecimal("11111")); o1.setObjectField(new Object()); @@ -366,11 +378,11 @@ public class PropertyUtilsTest { return o1; } - protected Map<String, Object> createMap() { + private Map<String, Object> createMap() { Map<String, Object> o1 = new HashMap<>(); o1.put("byteArrayField", new byte[] { 1, 2, 3 }); - o1.put("integerField", new Integer(33)); - o1.put("intField", new Integer(-44)); + o1.put("integerField", 33); + o1.put("intField", -44); o1.put("numberField", new BigDecimal("11111")); o1.put("objectField", new Object()); o1.put("stringField", "aaaaa");