Author: enorman
Date: Sun Aug 22 23:16:01 2010
New Revision: 987959
URL: http://svn.apache.org/viewvc?rev=987959&view=rev
Log:
SLING-897 Updating a property fires the wrong JCR events
Modified:
sling/trunk/bundles/servlets/post/src/main/java/org/apache/sling/servlets/post/impl/helper/SlingPropertyValueHandler.java
sling/trunk/launchpad/integration-tests/src/main/java/org/apache/sling/launchpad/webapp/integrationtest/servlets/post/PostServletUpdateTest.java
Modified:
sling/trunk/bundles/servlets/post/src/main/java/org/apache/sling/servlets/post/impl/helper/SlingPropertyValueHandler.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/post/src/main/java/org/apache/sling/servlets/post/impl/helper/SlingPropertyValueHandler.java?rev=987959&r1=987958&r2=987959&view=diff
==============================================================================
---
sling/trunk/bundles/servlets/post/src/main/java/org/apache/sling/servlets/post/impl/helper/SlingPropertyValueHandler.java
(original)
+++
sling/trunk/bundles/servlets/post/src/main/java/org/apache/sling/servlets/post/impl/helper/SlingPropertyValueHandler.java
Sun Aug 22 23:16:01 2010
@@ -222,12 +222,28 @@ public class SlingPropertyValueHandler {
} else if (values.length == 0) {
// do not create new prop here, but clear existing
if (parent.hasProperty(prop.getName())) {
- changes.add(Modification.onModified(
- parent.setProperty(prop.getName(), "").getPath()
- ));
+ if
(parent.getProperty(prop.getName()).getDefinition().isMultiple()) {
+ //the existing property is multi-valued, so
just delete it?
+ final String removePath = removePropertyIfExists(parent,
prop.getName());
+ if ( removePath != null ) {
+ changes.add(Modification.onDeleted(removePath));
+ }
+ } else {
+ changes.add(Modification.onModified(
+ parent.setProperty(prop.getName(), "").getPath()
+ ));
+ }
}
} else if (values.length == 1) {
- // if the provided value is the empty string, just remove the
existing property (if any).
+ //if a MultiValueTypeHint is supplied, or the current value is
multiple,
+ // store the updated property as multi-value.
+ boolean storePropertyAsMultiValued = prop.hasMultiValueTypeHint();
+ if (!prop.hasMultiValueTypeHint() &&
parent.hasProperty(prop.getName()) ) {
+ //no type hint supplied, so check the current property
definition
+ storePropertyAsMultiValued =
parent.getProperty(prop.getName()).getDefinition().isMultiple();
+ }
+
+ // if the provided value is the empty string, just remove the
existing property (if any).
if ( values[0].length() == 0 ) {
final String removePath = removePropertyIfExists(parent,
prop.getName());
if ( removePath != null ) {
@@ -239,7 +255,7 @@ public class SlingPropertyValueHandler {
// try conversion
Calendar c = dateParser.parse(values[0]);
if (c != null) {
- if ( prop.hasMultiValueTypeHint() ) {
+ if ( storePropertyAsMultiValued ) {
final Value[] array = new Value[1];
array[0] =
parent.getSession().getValueFactory().createValue(c);
changes.add(Modification.onModified(
@@ -255,7 +271,7 @@ public class SlingPropertyValueHandler {
} else if (isReferencePropertyType(type)) {
Value v = referenceParser.parse(values[0], valFac,
isWeakReference(type));
if (v != null) {
- if ( prop.hasMultiValueTypeHint() ) {
+ if ( storePropertyAsMultiValued ) {
final Value[] array = new Value[] { v };
changes.add(Modification.onModified(
parent.setProperty(prop.getName(),
array).getPath()
@@ -271,12 +287,17 @@ public class SlingPropertyValueHandler {
}
// fall back to default behaviour
-
final Property p;
if ( type == PropertyType.UNDEFINED ) {
- p = parent.setProperty(prop.getName(), values[0]);
+ if ( storePropertyAsMultiValued ) {
+ final Value[] array = new Value[1];
+ array[0] =
parent.getSession().getValueFactory().createValue(values[0]);
+ p = parent.setProperty(prop.getName(), array);
+ } else {
+ p = parent.setProperty(prop.getName(), values[0]);
+ }
} else {
- if ( prop.hasMultiValueTypeHint() ) {
+ if ( storePropertyAsMultiValued ) {
final Value[] array = new Value[1];
array[0] =
parent.getSession().getValueFactory().createValue(values[0], type);
p = parent.setProperty(prop.getName(), array);
@@ -287,6 +308,17 @@ public class SlingPropertyValueHandler {
changes.add(Modification.onModified(p.getPath()));
}
} else {
+ if (parent.hasProperty(prop.getName())) {
+ if
(!parent.getProperty(prop.getName()).getDefinition().isMultiple()) {
+ //the existing property is single-valued, so we
have to delete it before setting the
+ // multi-value variation
+ final String removePath = removePropertyIfExists(parent,
prop.getName());
+ if ( removePath != null ) {
+ changes.add(Modification.onDeleted(removePath));
+ }
+ }
+ }
+
if (type == PropertyType.DATE) {
// try conversion
Value[] c = dateParser.parse(values, valFac);
Modified:
sling/trunk/launchpad/integration-tests/src/main/java/org/apache/sling/launchpad/webapp/integrationtest/servlets/post/PostServletUpdateTest.java
URL:
http://svn.apache.org/viewvc/sling/trunk/launchpad/integration-tests/src/main/java/org/apache/sling/launchpad/webapp/integrationtest/servlets/post/PostServletUpdateTest.java?rev=987959&r1=987958&r2=987959&view=diff
==============================================================================
---
sling/trunk/launchpad/integration-tests/src/main/java/org/apache/sling/launchpad/webapp/integrationtest/servlets/post/PostServletUpdateTest.java
(original)
+++
sling/trunk/launchpad/integration-tests/src/main/java/org/apache/sling/launchpad/webapp/integrationtest/servlets/post/PostServletUpdateTest.java
Sun Aug 22 23:16:01 2010
@@ -40,6 +40,7 @@ import org.apache.jackrabbit.commons.Jcr
import org.apache.sling.commons.json.JSONArray;
import org.apache.sling.commons.json.JSONException;
import org.apache.sling.commons.json.JSONObject;
+import org.apache.sling.commons.testing.integration.NameValuePairList;
import
org.apache.sling.launchpad.webapp.integrationtest.AbstractAuthenticatedTest;
import org.apache.sling.servlets.post.SlingPostConstants;
@@ -196,22 +197,38 @@ public void testPostPathIsUnique() throw
testUserId = createTestUser();
//2. Create node as admin (OK)
- // curl -F:name=node -FpropOne=propOneValue1 -FpropOne=propOneValue2
http://admin:ad...@localhost:8080/test/
+ // curl -F:nameHint=node -FpropOne=propOneValue1
-FpropOne=propOneValue2 -FpropTwo=propTwoValue
http://admin:ad...@localhost:8080/test/
final String createTestNodeUrl = postUrl +
SlingPostConstants.DEFAULT_CREATE_SUFFIX;
- Map<String, String> nodeProperties = new HashMap<String, String>();
- nodeProperties.put(SlingPostConstants.RP_NODE_NAME_HINT, getName());
- nodeProperties.put("propOne", "propOneValue1");
- nodeProperties.put("propOne", "propOneValue2");
- String testNodeUrl = testClient.createNode(createTestNodeUrl,
nodeProperties);
+ NameValuePairList clientNodeProperties = new NameValuePairList();
+ clientNodeProperties.add(SlingPostConstants.RP_NODE_NAME_HINT,
getName());
+ clientNodeProperties.add("propOne", "propOneValue1");
+ clientNodeProperties.add("propOne", "propOneValue2");
+ clientNodeProperties.add("propTwo", "propTwoValue");
+ String testNodeUrl = testClient.createNode(createTestNodeUrl,
clientNodeProperties, null, false);
+ String content = getContent(testNodeUrl + ".json", CONTENT_TYPE_JSON);
+ JSONObject json = new JSONObject(content);
+ Object propOneObj = json.opt("propOne");
+ assertTrue(propOneObj instanceof JSONArray);
+ assertEquals(2, ((JSONArray)propOneObj).length());
+ assertEquals("propOneValue1", ((JSONArray)propOneObj).get(0));
+ assertEquals("propOneValue2", ((JSONArray)propOneObj).get(1));
+
+ Object propTwoObj = json.opt("propTwo");
+ assertTrue(propTwoObj instanceof String);
+ assertEquals("propTwoValue", propTwoObj);
+
+
//3. Attempt to update property of node as testUser (500:
javax.jcr.AccessDeniedException: /test/node/propOne: not allowed to add or
modify item)
- // curl -FpropOne=propOneValueChanged
http://myuser:passw...@localhost:8080/test/node
+ // curl -FpropOne=propOneValueChanged -FpropTwo=propTwoValueChanged1
-FpropTwo=propTwoValueChanged2 http://myuser:passw...@localhost:8080/test/node
List<NameValuePair> postParams = new ArrayList<NameValuePair>();
postParams.add(new NameValuePair("propOne", "propOneValueChanged"));
+ postParams.add(new NameValuePair("propTwo", "propTwoValueChanged1"));
+ postParams.add(new NameValuePair("propTwo", "propTwoValueChanged2"));
Credentials testUserCreds = new
UsernamePasswordCredentials(testUserId, "testPwd");
String expectedMessage = "Expected
javax.jcr.AccessDeniedException";
assertAuthenticatedPostStatus(testUserCreds, testNodeUrl,
HttpServletResponse.SC_INTERNAL_SERVER_ERROR, postParams, expectedMessage);
-
+
//4. Grant jcr:modifyProperties rights to testUser as admin (OK)
// curl -FprincipalId=myuser -fprivil...@jcr:modifyProperties=granted
http://admin:ad...@localhost:8080/test/node.modifyAce.html
Map<String, String> nodeAceProperties = new HashMap<String, String>();
@@ -233,19 +250,27 @@ public void testPostPathIsUnique() throw
testNodePath, //absPath
true, //isDeep
null, //uuid
- null, //nodeTypeName new String[]
{"mws:folder"}
+ null, //nodeTypeName
false); //noLocal
//5. Attempt to update properties of node (OK)
- // curl -FpropOne=propOneValueChanged
http://myuser:passw...@localhost:8080/test/node
+ // curl -FpropOne=propOneValueChanged
-FpropTwo=propTwoValueChanged1 -FpropTwo=propTwoValueChanged2
http://myuser:passw...@localhost:8080/test/node
assertAuthenticatedPostStatus(testUserCreds, testNodeUrl,
HttpServletResponse.SC_OK, postParams, expectedMessage);
//verify the change happened
- String content = getContent(testNodeUrl + ".json",
CONTENT_TYPE_JSON);
- JSONObject json = new JSONObject(content);
- Object propOneObj = json.opt("propOne");
- assertEquals("propOneValueChanged", propOneObj);
+ String afterUpdateContent = getContent(testNodeUrl + ".json",
CONTENT_TYPE_JSON);
+ JSONObject afterUpdateJson = new JSONObject(afterUpdateContent);
+ Object afterUpdatePropOneObj = afterUpdateJson.opt("propOne");
+ assertTrue(afterUpdatePropOneObj instanceof JSONArray);
+ assertEquals(1, ((JSONArray)afterUpdatePropOneObj).length());
+ assertEquals("propOneValueChanged",
((JSONArray)afterUpdatePropOneObj).get(0));
+ Object afterUpdatePropTwoObj = afterUpdateJson.opt("propTwo");
+ assertTrue(afterUpdatePropTwoObj instanceof JSONArray);
+ assertEquals(2, ((JSONArray)afterUpdatePropTwoObj).length());
+ assertEquals("propTwoValueChanged1",
((JSONArray)afterUpdatePropTwoObj).get(0));
+ assertEquals("propTwoValueChanged2",
((JSONArray)afterUpdatePropTwoObj).get(1));
+
//wait for the expected JCR events to be delivered
for (int second = 0; second < 15; second++) {
if (listener.getEventBundlesProcessed() > 0) {
@@ -254,12 +279,15 @@ public void testPostPathIsUnique() throw
Thread.sleep(1000);
}
- assertEquals("Property Added Event was not expected: "
+ listener.toString(),
- 0, listener.removedProperties.size());
- assertEquals("Property Removed Event was not expected:
" + listener.toString(),
- 0, listener.addedProperties.size());
- assertEquals("Property Changed Event was expected: " +
listener.toString(),
+ assertEquals("One property added event was expected: "
+ listener.toString(),
+ 1, listener.addedProperties.size());
+ assertEquals(testNodePath + "/propTwo",
listener.addedProperties.get(0));
+ assertEquals("One property removed event was expected:
" + listener.toString(),
+ 1, listener.removedProperties.size());
+ assertEquals(testNodePath + "/propTwo",
listener.removedProperties.get(0));
+ assertEquals("One property changed event was expected:
" + listener.toString(),
1, listener.changedProperties.size());
+ assertEquals(testNodePath + "/propOne",
listener.changedProperties.get(0));
} finally {
//cleanup
if (observationManager != null) {
