Index: jcr-server/server/src/java/org/apache/jackrabbit/server/AbstractWebdavServlet.java =================================================================== --- jcr-server/server/src/java/org/apache/jackrabbit/server/AbstractWebdavServlet.java (revision 396482) +++ jcr-server/server/src/java/org/apache/jackrabbit/server/AbstractWebdavServlet.java (working copy) @@ -439,25 +439,15 @@ DavResource resource) throws IOException, DavException { - DavPropertySet setProperties = request.getPropPatchSetProperties(); - DavPropertyNameSet removeProperties = request.getPropPatchRemoveProperties(); - if (setProperties.isEmpty() && removeProperties.isEmpty()) { + List changeList = request.getPropPatchChangeList(); + if (changeList.isEmpty()) { response.sendError(DavServletResponse.SC_BAD_REQUEST); return; } MultiStatus ms = new MultiStatus(); - try { - MultiStatusResponse msr = resource.alterProperties(setProperties, removeProperties); - ms.addResponse(msr); - } catch (DavException e) { - /* NOTE: known bug with litmus, which expects the exception (e.g. 423) - to the set instead of the MultiStatus Code. RFC 2518 explicitely - expects the errors to be included in the multistatus. - Remove the catch if this turns out to be an problem with clients - in general. */ - ms.addResourceStatus(resource, e.getErrorCode(), DEPTH_0); - } + MultiStatusResponse msr = resource.alterProperties(changeList); + ms.addResponse(msr); response.sendMultiStatus(ms); } Index: jcr-server/server/src/java/org/apache/jackrabbit/webdav/simple/DavResourceImpl.java =================================================================== --- jcr-server/server/src/java/org/apache/jackrabbit/webdav/simple/DavResourceImpl.java (revision 396482) +++ jcr-server/server/src/java/org/apache/jackrabbit/webdav/simple/DavResourceImpl.java (working copy) @@ -51,6 +51,7 @@ import org.apache.jackrabbit.webdav.property.DavProperty; import org.apache.jackrabbit.webdav.property.DavPropertyIterator; import org.apache.jackrabbit.webdav.property.DavPropertyName; +import org.apache.jackrabbit.webdav.property.DavPropertyNameIterator; import org.apache.jackrabbit.webdav.property.DavPropertyNameSet; import org.apache.jackrabbit.webdav.property.DavPropertySet; import org.apache.jackrabbit.webdav.property.DefaultDavProperty; @@ -385,11 +386,10 @@ } /** - * @see DavResource#alterProperties(org.apache.jackrabbit.webdav.property.DavPropertySet, org.apache.jackrabbit.webdav.property.DavPropertyNameSet) + * @see DavResource#alterProperties(List) */ - public MultiStatusResponse alterProperties(DavPropertySet setProperties, - DavPropertyNameSet removePropertyNames) - throws DavException { + public MultiStatusResponse alterProperties(List changeList) + throws DavException { if (isLocked(this)) { throw new DavException(DavServletResponse.SC_LOCKED); } @@ -400,33 +400,36 @@ MultiStatusResponse msr = new MultiStatusResponse(getHref(), null); boolean success = true; - // loop over set and remove Sets and remember all properties and propertyNames + // loop over List and remember all properties and propertyNames // that have successfully been altered List successList = new ArrayList(); - DavPropertyIterator setIter = setProperties.iterator(); - while (setIter.hasNext()) { - DavProperty prop = setIter.nextProperty(); - try { - setJcrProperty(prop); - successList.add(prop); - } catch (RepositoryException e) { - msr.add(prop.getName(), new JcrDavException(e).getErrorCode()); - success = false; + for (Iterator it = changeList.iterator(); it.hasNext(); ) { + Object item = it.next(); + if (item instanceof DavPropertyName) { + DavPropertyName propName = (DavPropertyName)item; + try { + removeJcrProperty(propName); + successList.add(propName); + } catch (RepositoryException e) { + msr.add(propName, new JcrDavException(e).getErrorCode()); + success = false; + } } - } - - Iterator remNameIter = removePropertyNames.iterator(); - while (remNameIter.hasNext()) { - DavPropertyName propName = (DavPropertyName) remNameIter.next(); - try { - removeJcrProperty(propName); - successList.add(propName); - } catch (RepositoryException e) { - msr.add(propName, new JcrDavException(e).getErrorCode()); - success = false; + else if (item instanceof DavProperty) { + DavProperty prop = (DavProperty)item; + try { + setJcrProperty(prop); + successList.add(prop); + } catch (RepositoryException e) { + msr.add(prop.getName(), new JcrDavException(e).getErrorCode()); + success = false; + } } + else { + throw new IllegalArgumentException("unknown object in change list: " + item.getClass().getName()); + } } - + try { if (success) { // save all changes together (reverted in case this fails) @@ -461,7 +464,31 @@ throw new JcrDavException(e); } } + + /** + * @see DavResource#alterProperties(org.apache.jackrabbit.webdav.property.DavPropertySet, org.apache.jackrabbit.webdav.property.DavPropertyNameSet) + */ + public MultiStatusResponse alterProperties(DavPropertySet setProperties, + DavPropertyNameSet removePropertyNames) + throws DavException { + List changeList = new ArrayList(); + + if (removePropertyNames != null) { + for (DavPropertyNameIterator it = removePropertyNames.iterator(); it.hasNext(); ) { + changeList.add(it.next()); + } + } + + if (setProperties != null) { + for (DavPropertyIterator it = setProperties.iterator(); it.hasNext(); ) { + changeList.add(it.next()); + } + } + + return alterProperties(changeList); + } + /** * @see DavResource#getCollection() */ Index: jcr-server/server/src/java/org/apache/jackrabbit/webdav/jcr/DefaultItemResource.java =================================================================== --- jcr-server/server/src/java/org/apache/jackrabbit/webdav/jcr/DefaultItemResource.java (revision 396482) +++ jcr-server/server/src/java/org/apache/jackrabbit/webdav/jcr/DefaultItemResource.java (working copy) @@ -32,6 +32,7 @@ import org.apache.jackrabbit.webdav.property.DavProperty; import org.apache.jackrabbit.webdav.property.DavPropertyIterator; import org.apache.jackrabbit.webdav.property.DavPropertyName; +import org.apache.jackrabbit.webdav.property.DavPropertyNameIterator; import org.apache.jackrabbit.webdav.property.DavPropertyNameSet; import org.apache.jackrabbit.webdav.property.DavPropertySet; import org.apache.jackrabbit.webdav.property.DefaultDavProperty; @@ -46,6 +47,7 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.Iterator; +import java.util.List; /** * DefaultItemResource represents JCR property item. @@ -181,24 +183,57 @@ public MultiStatusResponse alterProperties(DavPropertySet setProperties, DavPropertyNameSet removePropertyNames) throws DavException { + + List changeList = new ArrayList(); + + if (removePropertyNames != null) { + for (DavPropertyNameIterator it = removePropertyNames.iterator(); it.hasNext(); ) { + changeList.add(it.next()); + } + } + + if (setProperties != null) { + for (DavPropertyIterator it = setProperties.iterator(); it.hasNext(); ) { + changeList.add(it.next()); + } + } - // altering any properties fails if an attempt is made to remove a property - if (removePropertyNames != null && !removePropertyNames.isEmpty()) { - Iterator it = removePropertyNames.iterator(); - while (it.hasNext()) { + return alterProperties(changeList); + } + + /** + * Loops over the given List and alters the properties accordingly. + * Changes are persisted at the end only according to the rules defined with + * the {@link #complete()} method.

+ * Please note: since there is only a single property than can be set + * from a client (i.e. jcr:value OR jcr:values) this method either succeeds + * or throws an exception, even if this violates RFC 2518. + * + * @param changeList + * @throws DavException + * @see DavResource#alterProperties(List) + */ + public MultiStatusResponse alterProperties(List changeList) + throws DavException { + + for (Iterator it = changeList.iterator(); it.hasNext(); ) { + Object item = it.next(); + if (item instanceof DavPropertyName) { + // altering any properties fails if an attempt is made to remove a property throw new DavException(DavServletResponse.SC_FORBIDDEN); } + else if (item instanceof DavProperty) { + DavProperty prop = (DavProperty)item; + internalSetProperty(prop); + } + else { + throw new IllegalArgumentException("unknown object in change list: " + item.getClass().getName()); + } } - - // only set/add >> existance of resource is checked inside internal method - DavPropertyIterator setIter = setProperties.iterator(); - while (setIter.hasNext()) { - DavProperty prop = setIter.nextProperty(); - internalSetProperty(prop); - } complete(); return new MultiStatusResponse(getHref(), DavServletResponse.SC_OK); } + /** * Method is not allowed. Index: jcr-server/server/src/java/org/apache/jackrabbit/webdav/jcr/AbstractResource.java =================================================================== --- jcr-server/server/src/java/org/apache/jackrabbit/webdav/jcr/AbstractResource.java (revision 396482) +++ jcr-server/server/src/java/org/apache/jackrabbit/webdav/jcr/AbstractResource.java (working copy) @@ -287,6 +287,16 @@ /** * Throws {@link DavServletResponse#SC_METHOD_NOT_ALLOWED} * + * @see DavResource#alterProperties(List) + */ + public MultiStatusResponse alterProperties(List changeList) + throws DavException { + throw new DavException(DavServletResponse.SC_METHOD_NOT_ALLOWED); + } + + /** + * Throws {@link DavServletResponse#SC_METHOD_NOT_ALLOWED} + * * @param destination * @throws DavException Always throws {@link DavServletResponse#SC_METHOD_NOT_ALLOWED} * @see DavResource#move(org.apache.jackrabbit.webdav.DavResource) Index: jcr-server/server/src/java/org/apache/jackrabbit/webdav/jcr/DefaultItemCollection.java =================================================================== --- jcr-server/server/src/java/org/apache/jackrabbit/webdav/jcr/DefaultItemCollection.java (revision 396482) +++ jcr-server/server/src/java/org/apache/jackrabbit/webdav/jcr/DefaultItemCollection.java (working copy) @@ -43,6 +43,7 @@ import org.apache.jackrabbit.webdav.property.DavProperty; import org.apache.jackrabbit.webdav.property.DavPropertyIterator; import org.apache.jackrabbit.webdav.property.DavPropertyName; +import org.apache.jackrabbit.webdav.property.DavPropertyNameIterator; import org.apache.jackrabbit.webdav.property.DavPropertyNameSet; import org.apache.jackrabbit.webdav.property.DavPropertySet; import org.apache.jackrabbit.webdav.property.DefaultDavProperty; @@ -73,6 +74,7 @@ import java.util.ArrayList; import java.util.Date; import java.util.Iterator; +import java.util.List; import java.util.Set; /** @@ -276,18 +278,55 @@ public MultiStatusResponse alterProperties(DavPropertySet setProperties, DavPropertyNameSet removePropertyNames) throws DavException { - DavPropertyIterator setIter = setProperties.iterator(); - while (setIter.hasNext()) { - DavProperty prop = setIter.nextProperty(); - // use the internal set method in order to prevent premature 'save' - internalSetProperty(prop); + List changeList = new ArrayList(); + + if (removePropertyNames != null) { + for (DavPropertyNameIterator it = removePropertyNames.iterator(); it.hasNext(); ) { + changeList.add(it.next()); + } } - Iterator remNameIter = removePropertyNames.iterator(); - while (remNameIter.hasNext()) { - DavPropertyName propName = (DavPropertyName) remNameIter.next(); - // use the internal set method in order to prevent premature 'save' - internalRemoveProperty(propName); + + if (setProperties != null) { + for (DavPropertyIterator it = setProperties.iterator(); it.hasNext(); ) { + changeList.add(it.next()); + } } + + return alterProperties(changeList); + } + + /** + * Loops over the given {@link List} and alters the properties accordingly. + * Changes are persisted at the end according to the rules defined with + * the {@link #complete()} method.

+ * Please note: since there is only a single property ({@link #JCR_MIXINNODETYPES} + * that can be set or removed with PROPPATCH, this method either succeeds + * or throws an exception, even if this violates RFC 2518. Thus no property + * specific multistatus will be created in case of an error. + * + * @param changeList + * @return + * @throws DavException + * @see DavResource#alterProperties(List) + */ + public MultiStatusResponse alterProperties(List changeList) + throws DavException { + + for (Iterator it = changeList.iterator(); it.hasNext(); ) { + Object item = it.next(); + if (item instanceof DavPropertyName) { + // use the internal set method in order to prevent premature 'save' + internalRemoveProperty((DavPropertyName)item); + } + else if (item instanceof DavProperty) { + // use the internal set method in order to prevent premature 'save' + internalSetProperty((DavProperty)item); + } + else { + throw new IllegalArgumentException("unknown object in change list: " + item.getClass().getName()); + } + } + // TODO: missing undo of successful set/remove if subsequent operation fails // NOTE, that this is relevant with transactions only. @@ -296,6 +335,7 @@ return new MultiStatusResponse(getHref(), DavServletResponse.SC_OK); } + /** * If the specified resource represents a collection, a new node is {@link Node#addNode(String) * added} to the item represented by this resource. If an input stream is specified Index: jcr-server/webdav/src/java/org/apache/jackrabbit/webdav/WebdavRequestImpl.java =================================================================== --- jcr-server/webdav/src/java/org/apache/jackrabbit/webdav/WebdavRequestImpl.java (revision 396482) +++ jcr-server/webdav/src/java/org/apache/jackrabbit/webdav/WebdavRequestImpl.java (working copy) @@ -65,8 +65,10 @@ import java.net.URI; import java.net.URISyntaxException; import java.security.Principal; +import java.util.ArrayList; import java.util.Enumeration; import java.util.Iterator; +import java.util.List; import java.util.Locale; import java.util.Map; @@ -94,6 +96,7 @@ private DavPropertyNameSet propfindProps; private DavPropertySet proppatchSet; private DavPropertyNameSet proppatchRemove; + private List proppatchList; /** * Creates a new DavServletRequest with the given parameters. @@ -362,6 +365,7 @@ * * @return the list of 'set' entries in the PROPPATCH request body * @see DavServletRequest#getPropPatchSetProperties() + * @deprecated use {@link #getPropPatchChangeList()} instead */ public DavPropertySet getPropPatchSetProperties() throws DavException { if (proppatchSet == null) { @@ -377,6 +381,7 @@ * * @return the list of 'remove' entries in the PROPPATCH request body * @see DavServletRequest#getPropPatchRemoveProperties() + * @deprecated use {@link #getPropPatchChangeList()} instead */ public DavPropertyNameSet getPropPatchRemoveProperties() throws DavException { if (proppatchRemove == null) { @@ -386,12 +391,30 @@ } /** + * Return a {@link List} of property change operations. Each entry + * is either of type {@link DavPropertyName}, indicating a <remove> + * operation, or of type {@link DavProperty}, indicating a <set> + * operation. Note that ordering is significant here. + * + * @return the list of change operations entries in the PROPPATCH request body + * @see DavServletRequest#getPropPatchChangeList() + */ + public List getPropPatchChangeList() throws DavException { + if (proppatchList == null) { + parsePropPatchRequest(); + } + return proppatchList; + } + + /** * Parse the PROPPATCH request body. */ private void parsePropPatchRequest() throws DavException { proppatchSet = new DavPropertySet(); proppatchRemove = new DavPropertyNameSet(); + proppatchList = new ArrayList(); + Document requestDocument = getRequestDocument(); if (requestDocument == null) { @@ -401,31 +424,41 @@ Element root = requestDocument.getDocumentElement(); if (!DomUtil.matches(root, XML_PROPERTYUPDATE, NAMESPACE)) { // we should also check for correct namespace + // TODO: the comment above may be outdated --jre log.warn("PropPatch-Request has no tag."); throw new DavException(DavServletResponse.SC_BAD_REQUEST, "PropPatch-Request has no tag."); } - - ElementIterator it = DomUtil.getChildren(root, XML_SET, NAMESPACE); - while (it.hasNext()) { - Element propEl = DomUtil.getChildElement(it.nextElement(), XML_PROP, NAMESPACE); - if (propEl != null) { - ElementIterator properties = DomUtil.getChildren(propEl); - while (properties.hasNext()) { - proppatchSet.add(DefaultDavProperty.createFromXml(properties.nextElement())); + + + for (ElementIterator it = DomUtil.getChildren(root); it.hasNext(); ) { + + Element el = it.nextElement(); + if (DomUtil.matches(el, XML_SET, NAMESPACE)) { + Element propEl = DomUtil.getChildElement(el, XML_PROP, NAMESPACE); + if (propEl != null) { + ElementIterator properties = DomUtil.getChildren(propEl); + while (properties.hasNext()) { + DavProperty davProp = DefaultDavProperty.createFromXml(properties.nextElement()); + proppatchSet.add(davProp); + proppatchList.add(davProp); + } } + } - } - - // get properties - it = DomUtil.getChildren(root, XML_REMOVE, NAMESPACE); - while (it.hasNext()) { - Element propEl = DomUtil.getChildElement(it.nextElement(), XML_PROP, NAMESPACE); - if (propEl != null) { - ElementIterator names = DomUtil.getChildren(propEl); - while (names.hasNext()) { - proppatchRemove.add(DavPropertyName.createFromXml(names.nextElement())); + else if (DomUtil.matches(el, XML_REMOVE, NAMESPACE)) { + Element propEl = DomUtil.getChildElement(el, XML_PROP, NAMESPACE); + if (propEl != null) { + ElementIterator properties = DomUtil.getChildren(propEl); + while (properties.hasNext()) { + DavProperty davProp = DefaultDavProperty.createFromXml(properties.nextElement()); + proppatchSet.add(davProp); + proppatchList.add(davProp.getName()); + } } } + else { + // unknown child elements are ignored + } } } Index: jcr-server/webdav/src/java/org/apache/jackrabbit/webdav/DavResource.java =================================================================== --- jcr-server/webdav/src/java/org/apache/jackrabbit/webdav/DavResource.java (revision 396482) +++ jcr-server/webdav/src/java/org/apache/jackrabbit/webdav/DavResource.java (working copy) @@ -28,6 +28,7 @@ import org.apache.jackrabbit.webdav.property.DavPropertySet; import java.io.IOException; +import java.util.List; /** * DavResource provides standard WebDAV functionality as specified @@ -180,10 +181,26 @@ * @throws DavException if an error occured. This may be the case if the * general state of the resource prevents any properties to be set or removed * (e.g. due to a lock). + * @deprecated use {@link #alterProperties(List)} instead */ public MultiStatusResponse alterProperties(DavPropertySet setProperties, DavPropertyNameSet removePropertyNames) throws DavException; /** + * Set/add and remove the specified properties from this resource. + * + * @param changeList list containing {@link DavPropertyName} objects (for + * properties to be removed) and {@link DavProperty} objects (for + * properties to be added/set). + * @return multistatus response listing the status resulting from + * setting and/or removing the specified properties, in order to allow a + * detailled multistatus response. + * @throws DavException if an error occured. This may be the case if the + * general state of the resource prevents any properties to be set or removed + * (e.g. due to a lock). + */ + public MultiStatusResponse alterProperties(List changeList) throws DavException; + + /** * Retrieve the resource this resource is internal member of. * * @return resource this resource is an internal member of. In case this resource Index: jcr-server/webdav/src/java/org/apache/jackrabbit/webdav/DavServletRequest.java =================================================================== --- jcr-server/webdav/src/java/org/apache/jackrabbit/webdav/DavServletRequest.java (revision 396482) +++ jcr-server/webdav/src/java/org/apache/jackrabbit/webdav/DavServletRequest.java (working copy) @@ -15,6 +15,8 @@ */ package org.apache.jackrabbit.webdav; +import java.util.List; + import org.apache.jackrabbit.webdav.property.DavPropertySet; import org.apache.jackrabbit.webdav.property.DavPropertyNameSet; import org.apache.jackrabbit.webdav.lock.LockInfo; @@ -155,6 +157,7 @@ * @return set of properties the client wanted to modify / create with a * PROPPATCH request. * @throws DavException In case of invalid request body + * @deprecated use {@link #getPropPatchChangeList()} instead */ public DavPropertySet getPropPatchSetProperties() throws DavException; @@ -170,10 +173,21 @@ * @return set of property names the client wanted to remove with a * PROPPATCH request. * @throws DavException In case of invalid request body + * @deprecated use {@link #getPropPatchChangeList()} instead */ public DavPropertyNameSet getPropPatchRemoveProperties() throws DavException; /** + * Return a {@link List} of property change operations. Each entry + * is either of type {@link DavPropertyName}, indicating a <remove> + * operation, or of type {@link DavProperty}, indicating a <set> + * operation. Note that ordering is significant here. + * @return {@link List} of property change operations + * @throws DavException In case of invalid request body + */ + public List getPropPatchChangeList() throws DavException; + + /** * Return the parsed 'lockinfo' request body, the {@link DavConstants#HEADER_TIMEOUT * Timeout header} and the {@link DavConstants#HEADER_DEPTH Depth header} * of a LOCK request as LockInfo object.