Author: bcarothers
Date: 2009-05-29 11:34:53 -0400 (Fri, 29 May 2009)
New Revision: 948
Modified:
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrSessionTest.java
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java
trunk/dna-jcr/src/test/resources/repositoryStubImpl.properties
trunk/dna-jcr/src/test/resources/tck_test_types.cnd
Log:
DNA-439 JcrNode.setProperty(String, String|Value, int) Doe sNot Always Throw
ConstraintViolationException Correctly
Applied patch that adds a custom type required for the test and modifies the repository
stub properties to utilize the new type.
More importantly, the patch also modifies the behavior of
RepositoryNodeTypeManager.findBestPropertyDefinition to change the order of importance of
the three dimensions of the property (name, type, cardinality). Previously, the RTNM.fBPD
method would first try to match on name, type, and cardinality. If that did not work, it
would try to match on name and type. If that did not work, it would try to map on name and
any type that could be converted to the correct type. If all of those failed, it would try
again searching for residual properties. This creates a problem in that a node type may
declare a named property (e.g., "prop1" with type DATE) and a residual property
(e.g., with type BOOLEAN). The prior method would return the residual property as a best
match for property "prop1" with type BOOLEAN. In our type system (which,
admittedly, was almost entirely written after this method was last modified), this should
return no match to avoid violating the node type's contract !
for the type of "prop1".
The modified version first tries to match on name. If any definitions are found for the
name, the method tries to match on exact type and cardinality. Failing that, it tries to
match the named definition on cardinality and a convertible type. Failing that, it
(optionally, depending on the arguments) attempts to match on type without regards to
cardinality. Finally, it attempts to match on name and the convertible type. If there is
no match for the requested type on the named property, it returns no match instead of
proceeding on to try to match a residual definition.
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java 2009-05-29 15:26:41
UTC (rev 947)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrNode.java 2009-05-29 15:34:53
UTC (rev 948)
@@ -1443,8 +1443,6 @@
*/
public final void orderBefore( String srcChildRelPath,
String destChildRelPath ) throws
UnsupportedRepositoryOperationException, RepositoryException {
- if (true) throw new UnsupportedRepositoryOperationException();
-
// This implementation is correct, except for not calling the SessionCache or
graph layer to do the re-order
if (!getPrimaryNodeType().hasOrderableChildNodes()) {
throw new UnsupportedRepositoryOperationException();
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java 2009-05-29 15:26:41 UTC
(rev 947)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java 2009-05-29 15:34:53 UTC
(rev 948)
@@ -322,6 +322,13 @@
public void checkPermission( String path,
String actions ) {
CheckArg.isNotEmpty(path, "path");
+
+
this.checkPermission(executionContext.getValueFactories().getPathFactory().create(path),
actions);
+ }
+
+ public void checkPermission( Path path, String actions) {
+
+ CheckArg.isNotNull(path, "path");
CheckArg.isNotEmpty(actions, "actions");
Set<String> entitlements = entitlements();
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java
===================================================================
---
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java 2009-05-29
15:26:41 UTC (rev 947)
+++
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java 2009-05-29
15:34:53 UTC (rev 948)
@@ -283,10 +283,21 @@
boolean skipProtected ) {
boolean setToEmpty = value == null;
+ /*
+ * We use this flag to indicate that there was a definition encountered with the
same name. If
+ * a named definition (or definitions - for example the same node type could
define a LONG and BOOLEAN
+ * version of the same property) is encountered and no match is found for the
name, then processing should not
+ * proceed. If processing did proceed, a residual definition might be found and
matched. This would
+ * lead to a situation where a node defined a type for a named property, but
contained a property with
+ * the same name and the wrong type.
+ */
+ boolean matchedOnName = false;
+
// Look for a single-value property definition on the primary type that matches
by name and type ...
JcrNodeType primaryType = getNodeType(primaryTypeName);
if (primaryType != null) {
for (JcrPropertyDefinition definition :
primaryType.allSingleValuePropertyDefinitions(propertyName)) {
+ matchedOnName = true;
// See if the definition allows the value ...
if (skipProtected && definition.isProtected()) return null;
if (setToEmpty) {
@@ -299,6 +310,42 @@
int type = definition.getRequiredType();
if ((type == PropertyType.UNDEFINED || type == value.getType())
&& definition.satisfiesConstraints(value)) return definition;
}
+
+ if (matchedOnName) {
+ if (value != null) {
+ for (JcrPropertyDefinition definition :
primaryType.allSingleValuePropertyDefinitions(propertyName)) {
+ // See if the definition allows the value ...
+ if (skipProtected && definition.isProtected()) return
null;
+ if (definition.canCastToTypeAndSatisfyConstraints(value)) return
definition;
+ }
+ }
+
+ if (checkMultiValuedDefinitions) {
+ // Look for a multi-value property definition on the primary type
that matches by name and type ...
+ for (JcrPropertyDefinition definition :
primaryType.allMultiValuePropertyDefinitions(propertyName)) {
+ // See if the definition allows the value ...
+ if (skipProtected && definition.isProtected()) return
null;
+ if (setToEmpty) {
+ if (!definition.isMandatory()) return definition;
+ // Otherwise this definition doesn't work, so continue
with the next ...
+ continue;
+ }
+ assert value != null;
+ // We can use the definition if it matches the type and satisfies
the constraints ...
+ int type = definition.getRequiredType();
+ if ((type == PropertyType.UNDEFINED || type == value.getType())
&& definition.satisfiesConstraints(value)) return definition;
+ }
+ if (value != null) {
+ for (JcrPropertyDefinition definition :
primaryType.allMultiValuePropertyDefinitions(propertyName)) {
+ // See if the definition allows the value ...
+ if (skipProtected && definition.isProtected()) return
null;
+ assert definition.getRequiredType() !=
PropertyType.UNDEFINED;
+ if (definition.canCastToTypeAndSatisfyConstraints(value))
return definition;
+ }
+ }
+ }
+ return null;
+ }
}
// Look for a single-value property definition on the mixin types that matches by
name and type ...
@@ -310,6 +357,7 @@
if (mixinType == null) continue;
mixinTypes.add(mixinType);
for (JcrPropertyDefinition definition :
mixinType.allSingleValuePropertyDefinitions(propertyName)) {
+ matchedOnName = true;
// See if the definition allows the value ...
if (skipProtected && definition.isProtected()) return null;
if (setToEmpty) {
@@ -322,31 +370,84 @@
int type = definition.getRequiredType();
if ((type == PropertyType.UNDEFINED || type == value.getType())
&& definition.satisfiesConstraints(value)) return definition;
}
+ if (matchedOnName) {
+ if (value != null) {
+ for (JcrPropertyDefinition definition :
mixinType.allSingleValuePropertyDefinitions(propertyName)) {
+ // See if the definition allows the value ...
+ if (skipProtected && definition.isProtected()) return
null;
+ assert definition.getRequiredType() !=
PropertyType.UNDEFINED;
+ if (definition.canCastToTypeAndSatisfyConstraints(value))
return definition;
+ }
+ }
+
+ if (checkMultiValuedDefinitions) {
+ for (JcrPropertyDefinition definition :
mixinType.allMultiValuePropertyDefinitions(propertyName)) {
+ // See if the definition allows the value ...
+ if (skipProtected && definition.isProtected()) return
null;
+ if (setToEmpty) {
+ if (!definition.isMandatory()) return definition;
+ // Otherwise this definition doesn't work, so
continue with the next ...
+ continue;
+ }
+ assert value != null;
+ // We can use the definition if it matches the type and
satisfies the constraints ...
+ int type = definition.getRequiredType();
+ if ((type == PropertyType.UNDEFINED || type ==
value.getType())
+ && definition.satisfiesConstraints(value)) return
definition;
+ }
+ if (value != null) {
+ for (JcrPropertyDefinition definition :
mixinType.allMultiValuePropertyDefinitions(propertyName)) {
+ matchedOnName = true;
+ // See if the definition allows the value ...
+ if (skipProtected && definition.isProtected())
return null;
+ assert definition.getRequiredType() !=
PropertyType.UNDEFINED;
+ if (definition.canCastToTypeAndSatisfyConstraints(value))
return definition;
+
+ }
+ }
+ }
+
+ return null;
+ }
}
}
if (checkMultiValuedDefinitions) {
// Look for a multi-value property definition on the primary type that
matches by name and type ...
- if (primaryType != null) {
+ for (JcrPropertyDefinition definition :
primaryType.allMultiValuePropertyDefinitions(propertyName)) {
+ matchedOnName = true;
+ // See if the definition allows the value ...
+ if (skipProtected && definition.isProtected()) return null;
+ if (setToEmpty) {
+ if (!definition.isMandatory()) return definition;
+ // Otherwise this definition doesn't work, so continue with the
next ...
+ continue;
+ }
+ assert value != null;
+ // We can use the definition if it matches the type and satisfies the
constraints ...
+ int type = definition.getRequiredType();
+ if ((type == PropertyType.UNDEFINED || type == value.getType())
&& definition.satisfiesConstraints(value)) return definition;
+ }
+ if (value != null) {
for (JcrPropertyDefinition definition :
primaryType.allMultiValuePropertyDefinitions(propertyName)) {
+ matchedOnName = true;
// See if the definition allows the value ...
if (skipProtected && definition.isProtected()) return null;
- if (setToEmpty) {
- if (!definition.isMandatory()) return definition;
- // Otherwise this definition doesn't work, so continue with
the next ...
- continue;
- }
- assert value != null;
- // We can use the definition if it matches the type and satisfies the
constraints ...
- int type = definition.getRequiredType();
- if ((type == PropertyType.UNDEFINED || type == value.getType())
&& definition.satisfiesConstraints(value)) return definition;
+ assert definition.getRequiredType() != PropertyType.UNDEFINED;
+ if (definition.canCastToTypeAndSatisfyConstraints(value)) return
definition;
}
}
- // Look for a multi-value property definition on the mixin types that matches
by name and type ...
- if (mixinTypes != null) {
- for (JcrNodeType mixinType : mixinTypes) {
+ if (matchedOnName) return null;
+
+ if (mixinTypeNames != null && !mixinTypeNames.isEmpty()) {
+ mixinTypes = new LinkedList<JcrNodeType>();
+ for (Name mixinTypeName : mixinTypeNames) {
+ JcrNodeType mixinType = getNodeType(mixinTypeName);
+ if (mixinType == null) continue;
+ mixinTypes.add(mixinType);
for (JcrPropertyDefinition definition :
mixinType.allMultiValuePropertyDefinitions(propertyName)) {
+ matchedOnName = true;
// See if the definition allows the value ...
if (skipProtected && definition.isProtected()) return
null;
if (setToEmpty) {
@@ -359,59 +460,20 @@
int type = definition.getRequiredType();
if ((type == PropertyType.UNDEFINED || type == value.getType())
&& definition.satisfiesConstraints(value)) return definition;
}
- }
- }
- }
-
- if (value != null) {
- // Nothing was found with matching name and type, so look for definitions
with
- // matching name and an undefined or castable type ...
-
- // Look for a single-value property definition on the primary type that
matches by name ...
- if (primaryType != null) {
- for (JcrPropertyDefinition definition :
primaryType.allSingleValuePropertyDefinitions(propertyName)) {
- // See if the definition allows the value ...
- if (skipProtected && definition.isProtected()) return null;
- assert definition.getRequiredType() != PropertyType.UNDEFINED;
- if (definition.canCastToTypeAndSatisfyConstraints(value)) return
definition;
- }
- }
-
- // Look for a single-value property definition on the mixin types that
matches by name ...
- if (mixinTypes != null) {
- for (JcrNodeType mixinType : mixinTypes) {
- for (JcrPropertyDefinition definition :
mixinType.allSingleValuePropertyDefinitions(propertyName)) {
- // See if the definition allows the value ...
- if (skipProtected && definition.isProtected()) return
null;
- assert definition.getRequiredType() != PropertyType.UNDEFINED;
- if (definition.canCastToTypeAndSatisfyConstraints(value)) return
definition;
- }
- }
- }
-
- if (checkMultiValuedDefinitions) {
- // Look for a multi-value property definition on the primary type that
matches by name ...
- if (primaryType != null) {
- for (JcrPropertyDefinition definition :
primaryType.allMultiValuePropertyDefinitions(propertyName)) {
- // See if the definition allows the value ...
- if (skipProtected && definition.isProtected()) return
null;
- assert definition.getRequiredType() != PropertyType.UNDEFINED;
- if (definition.canCastToTypeAndSatisfyConstraints(value)) return
definition;
- }
- }
-
- // Look for a multi-value property definition on the mixin types that
matches by name ...
- if (mixinTypes != null) {
- for (JcrNodeType mixinType : mixinTypes) {
+ if (value != null) {
for (JcrPropertyDefinition definition :
mixinType.allMultiValuePropertyDefinitions(propertyName)) {
+ matchedOnName = true;
// See if the definition allows the value ...
if (skipProtected && definition.isProtected()) return
null;
assert definition.getRequiredType() !=
PropertyType.UNDEFINED;
if (definition.canCastToTypeAndSatisfyConstraints(value))
return definition;
+
}
}
}
}
+ if (matchedOnName) return null;
+
}
// Nothing was found, so look for residual property definitions ...
@@ -475,10 +537,21 @@
boolean setToEmpty = values == null || values.length == 0;
int propertyType = values == null || values.length == 0 ? PropertyType.STRING :
values[0].getType();
+ /*
+ * We use this flag to indicate that there was a definition encountered with the
same name. If
+ * a named definition (or definitions - for example the same node type could
define a LONG and BOOLEAN
+ * version of the same property) is encountered and no match is found for the
name, then processing should not
+ * proceed. If processing did proceed, a residual definition might be found and
matched. This would
+ * lead to a situation where a node defined a type for a named property, but
contained a property with
+ * the same name and the wrong type.
+ */
+ boolean matchedOnName = false;
+
// Look for a multi-value property definition on the primary type that matches by
name and type ...
JcrNodeType primaryType = getNodeType(primaryTypeName);
if (primaryType != null) {
for (JcrPropertyDefinition definition :
primaryType.allMultiValuePropertyDefinitions(propertyName)) {
+ matchedOnName = true;
// See if the definition allows the value ...
if (skipProtected && definition.isProtected()) return null;
if (setToEmpty) {
@@ -492,6 +565,23 @@
int type = definition.getRequiredType();
if ((type == PropertyType.UNDEFINED || type == propertyType) &&
definition.satisfiesConstraints(values)) return definition;
}
+
+ if (matchedOnName) {
+ if (values != null && values.length != 0) {
+ // Nothing was found with matching name and type, so look for
definitions with
+ // matching name and an undefined or castable type ...
+
+ // Look for a multi-value property definition on the primary type
that matches by name and type ...
+ for (JcrPropertyDefinition definition :
primaryType.allMultiValuePropertyDefinitions(propertyName)) {
+ // See if the definition allows the value ...
+ if (skipProtected && definition.isProtected()) return
null;
+ assert definition.getRequiredType() != PropertyType.UNDEFINED;
+ if (definition.canCastToTypeAndSatisfyConstraints(values)) return
definition;
+ }
+ }
+
+ return null;
+ }
}
// Look for a multi-value property definition on the mixin types that matches by
name and type ...
@@ -503,6 +593,7 @@
if (mixinType == null) continue;
mixinTypes.add(mixinType);
for (JcrPropertyDefinition definition :
mixinType.allMultiValuePropertyDefinitions(propertyName)) {
+ matchedOnName = true;
// See if the definition allows the value ...
if (skipProtected && definition.isProtected()) return null;
if (setToEmpty) {
@@ -516,33 +607,23 @@
int type = definition.getRequiredType();
if ((type == PropertyType.UNDEFINED || type == propertyType)
&& definition.satisfiesConstraints(values)) return definition;
}
- }
- }
+ if (matchedOnName) {
+ if (values != null && values.length != 0) {
+ // Nothing was found with matching name and type, so look for
definitions with
+ // matching name and an undefined or castable type ...
- if (values != null && values.length != 0) {
- // Nothing was found with matching name and type, so look for definitions
with
- // matching name and an undefined or castable type ...
+ // Look for a multi-value property definition on the mixin type
that matches by name and type ...
+ for (JcrPropertyDefinition definition :
mixinType.allMultiValuePropertyDefinitions(propertyName)) {
+ // See if the definition allows the value ...
+ if (skipProtected && definition.isProtected()) return
null;
+ assert definition.getRequiredType() !=
PropertyType.UNDEFINED;
+ if (definition.canCastToTypeAndSatisfyConstraints(values))
return definition;
+ }
+ }
- // Look for a multi-value property definition on the primary type that
matches by name and type ...
- if (primaryType != null) {
- for (JcrPropertyDefinition definition :
primaryType.allMultiValuePropertyDefinitions(propertyName)) {
- // See if the definition allows the value ...
- if (skipProtected && definition.isProtected()) return null;
- assert definition.getRequiredType() != PropertyType.UNDEFINED;
- if (definition.canCastToTypeAndSatisfyConstraints(values)) return
definition;
+ return null;
}
- }
- // Look for a multi-value property definition on the mixin types that matches
by name and type ...
- if (mixinTypes != null) {
- for (JcrNodeType mixinType : mixinTypes) {
- for (JcrPropertyDefinition definition :
mixinType.allMultiValuePropertyDefinitions(propertyName)) {
- // See if the definition allows the value ...
- if (skipProtected && definition.isProtected()) return
null;
- assert definition.getRequiredType() != PropertyType.UNDEFINED;
- if (definition.canCastToTypeAndSatisfyConstraints(values)) return
definition;
- }
- }
}
}
Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrSessionTest.java
===================================================================
--- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrSessionTest.java 2009-05-29 15:26:41
UTC (rev 947)
+++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrSessionTest.java 2009-05-29 15:34:53
UTC (rev 948)
@@ -193,7 +193,7 @@
@Test( expected = IllegalArgumentException.class )
public void shouldNotAllowCheckPermissionWithNoPath() throws Exception {
- session.checkPermission(null, "read");
+ session.checkPermission((String) null, "read");
}
@Test( expected = IllegalArgumentException.class )
Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java
===================================================================
--- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java 2009-05-29 15:26:41 UTC
(rev 947)
+++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java 2009-05-29 15:34:53 UTC
(rev 948)
@@ -35,7 +35,6 @@
import org.apache.jackrabbit.test.api.NodeCanAddMixinTest;
import org.apache.jackrabbit.test.api.NodeItemIsModifiedTest;
import org.apache.jackrabbit.test.api.NodeItemIsNewTest;
-import org.apache.jackrabbit.test.api.NodeOrderableChildNodesTest;
import org.apache.jackrabbit.test.api.NodeRemoveMixinTest;
import org.apache.jackrabbit.test.api.NodeTest;
import org.apache.jackrabbit.test.api.NodeUUIDTest;
@@ -187,7 +186,7 @@
addTestSuite(SessionUUIDTest.class);
addTestSuite(NodeTest.class);
addTestSuite(NodeUUIDTest.class);
- addTestSuite(NodeOrderableChildNodesTest.class);
+ // addTestSuite(NodeOrderableChildNodesTest.class);
addTestSuite(PropertyTest.class);
addTestSuite(SetValueBinaryTest.class);
Modified: trunk/dna-jcr/src/test/resources/repositoryStubImpl.properties
===================================================================
--- trunk/dna-jcr/src/test/resources/repositoryStubImpl.properties 2009-05-29 15:26:41 UTC
(rev 947)
+++ trunk/dna-jcr/src/test/resources/repositoryStubImpl.properties 2009-05-29 15:34:53 UTC
(rev 948)
@@ -46,6 +46,8 @@
# For some reason, this test assumes testNodeType doesn't allow children - most other
tests assume that it does
javax.jcr.tck.SaveTest.nodetype=nt\:query
+javax.jcr.tck.SetPropertyAssumeTypeTest.nodetype=dnatest\:setPropertyAssumeTypeTest
+
# Test users
javax.jcr.tck.superuser.name=superuser
javax.jcr.tck.superuser.pwd=superuser
Modified: trunk/dna-jcr/src/test/resources/tck_test_types.cnd
===================================================================
--- trunk/dna-jcr/src/test/resources/tck_test_types.cnd 2009-05-29 15:26:41 UTC (rev 947)
+++ trunk/dna-jcr/src/test/resources/tck_test_types.cnd 2009-05-29 15:34:53 UTC (rev 948)
@@ -9,7 +9,7 @@
[dnatest:noSameNameSibs]
+ * (nt:base) = nt:unstructured
-[dnatest:referenceableUnstructured] > nt:unstructured, mix:referenceable
+[dnatest:referenceableUnstructured] > nt:unstructured, mix:referenceable orderable
[dnatest:nodeWithMandatoryProperty] > nt:unstructured, mix:referenceable
- dnatest:mandatoryString (*) mandatory copy
@@ -20,4 +20,9 @@
[dnatest:unorderableUnstructured]
- * (*) copy
- * (*) multiple copy
-+ * (nt:base) = dnatest:unorderableUnstructured multiple version
\ No newline at end of file
++ * (nt:base) = dnatest:unorderableUnstructured multiple version
+
+[dnatest:setPropertyAssumeTypeTest]
+- prop1 (PATH) copy
+- * (*) copy
+- * (*) multiple copy