necouchman commented on code in PR #809:
URL: https://github.com/apache/guacamole-client/pull/809#discussion_r1157095270


##########
guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryResource.java:
##########
@@ -386,46 +417,210 @@ public Map<String, ExternalType> getObjects(
 
     /**
      * Applies the given object patches, updating the underlying directory
-     * accordingly. This operation currently only supports deletion of objects
-     * through the "remove" patch operation. The path of each patch operation 
is
-     * of the form "/ID" where ID is the identifier of the object being
-     * modified.
+     * accordingly. This operation supports addition and removal of objects
+     * through the "add" and "remove" patch operation. The path of each patch
+     * operation is of the form "/ID" where ID is the identifier of the object
+     * being modified. In the case of object creation, the identifier is
+     * ignored, as the identifier will be automatically provided. This 
operation
+     * is atomic.
      *
      * @param patches
      *     The patches to apply for this request.
      *
      * @throws GuacamoleException
-     *     If an error occurs while deleting the objects.
+     *     If an error occurs while adding, updating, or removing objects.
+     *
+     * @return
+     *     A response describing the outcome of each patch. Only the identifier
+     *     of each patched object will be included in the response, not the
+     *     full object.
      */
     @PATCH
-    public void patchObjects(List<APIPatch<String>> patches)
+    public APIPatchResponse patchObjects(List<APIPatch<ExternalType>> patches)
             throws GuacamoleException {
 
-        // Apply each operation specified within the patch
-        for (APIPatch<String> patch : patches) {
+        // An outcome for each patch included in the request. This list
+        // may include both success and failure responses, though the
+        // presense of any failure would indicated that the entire
+        // request has failed and no changes have been made.
+        List<APIPatchOutcome> patchOutcomes = new ArrayList<>();
 
-            // Only remove is supported
-            if (patch.getOp() != APIPatch.Operation.remove)
-                throw new GuacamoleUnsupportedException("Only the \"remove\" "
-                        + "operation is supported.");
+        // Perform all requested operations atomically
+        directory.tryAtomically(new AtomicDirectoryOperation<InternalType>() {
 
-            // Retrieve and validate path
-            String path = patch.getPath();
-            if (!path.startsWith("/"))
-                throw new GuacamoleClientException("Patch paths must start 
with \"/\".");
+            @Override
+            public void executeOperation(boolean atomic, 
Directory<InternalType> directory)
+                    throws GuacamoleException {
+
+                // If the underlying directory implentation does not support
+                // atomic operations, abort the patch operation. This REST
+                // endpoint requires that operations be performed atomically.
+                if (!atomic)
+                    throw new GuacamoleUnsupportedException(
+                            "Atomic operations are not supported. " +
+                            "The patch cannot be executed.");
+
+                // Keep a list of all objects that have been successfully
+                // added or removed
+                Collection<InternalType> addedObjects = new ArrayList<>();
+                Collection<String> removedIdentifiers = new ArrayList<>();
+
+                // A list of all responses associated with the successful
+                // creation of new objects
+                List<APIPatchOutcome> creationSuccesses = new ArrayList<>();
+
+                // True if any operation in the patch failed. Any failure will
+                // fail the request, though won't result in immediate stoppage
+                // since more errors may yet be uncovered.
+                boolean failed = false;
+
+                // Apply each operation specified within the patch
+                for (APIPatch<ExternalType> patch : patches) {
+
+                    // Retrieve and validate path
+                    String path = patch.getPath();
+                    if (!path.startsWith("/"))
+                        throw new GuacamoleClientException("Patch paths must 
start with \"/\".");
+
+                    if(patch.getOp() == APIPatch.Operation.add) {

Review Comment:
   No strong preference, just curious.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to