Author: enorman
Date: Sun Aug 22 16:19:41 2010
New Revision: 987925
URL: http://svn.apache.org/viewvc?rev=987925&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=987925&r1=987924&r2=987925&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 16:19:41 2010
@@ -227,9 +227,9 @@ public class SlingPropertyValueHandler {
));
}
} else if (values.length == 1) {
- final String removePath = removePropertyIfExists(parent,
prop.getName());
- // if the provided value is the empty string, we don't have to do
anything.
+ // 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 ) {
changes.add(Modification.onDeleted(removePath));
}
@@ -287,8 +287,6 @@ public class SlingPropertyValueHandler {
changes.add(Modification.onModified(p.getPath()));
}
} else {
- removePropertyIfExists(parent, prop.getName());
-
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=987925&r1=987924&r2=987925&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 16:19:41 2010
@@ -17,21 +17,37 @@
package org.apache.sling.launchpad.webapp.integrationtest.servlets.post;
import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
+import javax.jcr.Repository;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.SimpleCredentials;
+import javax.jcr.observation.Event;
+import javax.jcr.observation.EventIterator;
+import javax.jcr.observation.EventListener;
+import javax.jcr.observation.ObservationManager;
import javax.servlet.http.HttpServletResponse;
+import org.apache.commons.httpclient.Credentials;
+import org.apache.commons.httpclient.NameValuePair;
+import org.apache.commons.httpclient.UsernamePasswordCredentials;
+import org.apache.jackrabbit.commons.JcrUtils;
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.HttpTestBase;
+import
org.apache.sling.launchpad.webapp.integrationtest.AbstractAuthenticatedTest;
import org.apache.sling.servlets.post.SlingPostConstants;
/** Test node updates via the MicrojaxPostServlet */
-public class PostServletUpdateTest extends HttpTestBase {
+public class PostServletUpdateTest extends AbstractAuthenticatedTest {
public static final String TEST_BASE_PATH = "/sling-tests";
private String postUrl;
+ private String testUserId = null;
@Override
protected void setUp() throws Exception {
@@ -39,7 +55,23 @@ public class PostServletUpdateTest exten
postUrl = HTTP_BASE_URL + TEST_BASE_PATH + "/" +
System.currentTimeMillis();
}
- public void testPostPathIsUnique() throws IOException {
+
+ /* (non-Javadoc)
+ * @see
org.apache.sling.commons.testing.integration.HttpTestBase#tearDown()
+ */
+ @Override
+ protected void tearDown() throws Exception {
+ if (testUserId != null) {
+ //remove the test user if it exists.
+ String postUrl = HTTP_BASE_URL +
"/system/userManager/user/" + testUserId + ".delete.html";
+ List<NameValuePair> postParams = new
ArrayList<NameValuePair>();
+ assertAuthenticatedAdminPostStatus(postUrl,
HttpServletResponse.SC_OK, postParams, null);
+ }
+ super.tearDown();
+ }
+
+
+public void testPostPathIsUnique() throws IOException {
assertHttpStatus(postUrl, HttpServletResponse.SC_NOT_FOUND,
"Path must not exist before test: " + postUrl);
}
@@ -151,4 +183,143 @@ public class PostServletUpdateTest exten
json = new JSONObject(content);
assertTrue("no jcr:mixinTypes expected after clearing it",
!json.has("jcr:mixinTypes"));
}
+
+ /**
+ * Test for SLING-897 fix:
+ * 1. Updating a property requires jcr:modifyProperties privilege on node.
+ * 2. When changing an existing property observers should receive a
PROPERTY_CHANGED event instead
+ * of a PROPERTY_REMOVED event and a PROPERTY_ADDED event
+ */
+ public void testUpdatePropertyPrivilegesAndEvents() throws IOException,
JSONException, RepositoryException, InterruptedException {
+ //1. Create user as admin (OK)
+ // curl -F:name=myuser -Fpwd=password -FpwdConfirm=password
http://admin:ad...@localhost:8080/system/userManager/user.create.html
+ testUserId = createTestUser();
+
+ //2. Create node as admin (OK)
+ // curl -F:name=node -FpropOne=propOneValue1 -FpropOne=propOneValue2
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);
+
+ //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
+ List<NameValuePair> postParams = new ArrayList<NameValuePair>();
+ postParams.add(new NameValuePair("propOne", "propOneValueChanged"));
+ 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>();
+ nodeAceProperties.put("principalId", testUserId);
+ nodeAceProperties.put("privil...@jcr:modifyProperties", "granted");
+ testClient.createNode(testNodeUrl + ".modifyAce.html",
nodeAceProperties);
+
+ //use a davex session to verify the correct JCR events are delivered
+ Repository repository = JcrUtils.getRepository(HTTP_BASE_URL +
"/server/");
+ Session jcrSession = null;
+ TestEventListener listener = new TestEventListener();
+ ObservationManager observationManager = null;
+ try {
+ jcrSession = repository.login(new SimpleCredentials("admin",
"admin".toCharArray()));
+ observationManager =
jcrSession.getWorkspace().getObservationManager();
+ String testNodePath =
testNodeUrl.substring(HTTP_BASE_URL.length());
+ observationManager.addEventListener(listener,
+ Event.PROPERTY_ADDED |
Event.PROPERTY_CHANGED | Event.PROPERTY_REMOVED, //event types
+ testNodePath, //absPath
+ true, //isDeep
+ null, //uuid
+ null, //nodeTypeName new String[]
{"mws:folder"}
+ false); //noLocal
+
+ //5. Attempt to update properties of node (OK)
+ // curl -FpropOne=propOneValueChanged
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);
+
+ //wait for the expected JCR events to be delivered
+ for (int second = 0; second < 15; second++) {
+ if (listener.getEventBundlesProcessed() > 0) {
+ break;
+ }
+ 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(),
+ 1, listener.changedProperties.size());
+ } finally {
+ //cleanup
+ if (observationManager != null) {
+ observationManager.removeEventListener(listener);
+ }
+ jcrSession.logout();
+ repository = null;
+ }
+ }
+
+ protected class TestEventListener implements EventListener {
+ protected List<String> changedProperties = new
ArrayList<String>();
+ protected List<String> addedProperties = new
ArrayList<String>();
+ protected List<String> removedProperties = new
ArrayList<String>();
+
+ protected int eventBundlesProcessed = 0;
+
+ public void onEvent(EventIterator eventIterator) {
+ try {
+ while (eventIterator.hasNext()) {
+ Event event = eventIterator.nextEvent();
+ int type = event.getType();
+ switch (type) {
+ case Event.PROPERTY_CHANGED:
+
changedProperties.add(event.getPath());
+ break;
+ case Event.PROPERTY_ADDED:
+
addedProperties.add(event.getPath());
+ break;
+ case Event.PROPERTY_REMOVED:
+
removedProperties.add(event.getPath());
+ break;
+ }
+ }
+ eventBundlesProcessed++;
+ } catch (RepositoryException e) {
+ fail(e.getMessage());
+ }
+ }
+
+ public int getEventBundlesProcessed() {
+ return eventBundlesProcessed;
+ }
+
+ public void clear() {
+ changedProperties.clear();
+ addedProperties.clear();
+ removedProperties.clear();
+ eventBundlesProcessed = 0;
+ }
+
+ /* (non-Javadoc)
+ * @see java.lang.Object#toString()
+ */
+ @Override
+ public String toString() {
+ return "TestEventListener [addedProperties=" +
Arrays.toString(addedProperties.toArray())
+ + ", changedProperties=" +
Arrays.toString(changedProperties.toArray())
+ + ", removedProperties=" +
Arrays.toString(removedProperties.toArray()) + "]";
+ }
+ }
+
}
\ No newline at end of file
