You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cayenne.apache.org by nt...@apache.org on 2017/02/07 12:28:27 UTC
cayenne git commit: CAY-1551 orderings with "+." in the path fail
when performing in memory sort
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 <st...@gmail.com>
Authored: Tue Feb 7 15:15:22 2017 +0300
Committer: Nikita Timofeev <st...@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");