Performance Improvement to SCIM Group PATCH operation

wso2 · Jun 20, 2023 · a7d88ed · a7d88ed
1 parent 9218979
commit a7d88ed
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 30 deletions.
diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/extensions/UserManager.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/extensions/UserManager.java
@@ -220,6 +220,24 @@ default Group patchGroup(String groupId, String currentGroupName, Map<String, Li
         throw new NotImplementedException();
     }
 
+    /**
+     * Updates the group via PATCH.
+     *
+     * @param  groupId                 ID of the group.
+     * @param  currentGroupName        Current name of the group.
+     * @param  patchOperations         A map of patch operations.
+     *
+     * @throws CharonException         Charon exception.
+     * @throws BadRequestException     Bad request exception.
+     * @throws NotFoundException       Not found exception.
+     * @throws NotImplementedException Functionality no implemented exception.
+     */
+    default void patchGroup(String groupId, String currentGroupName, Map<String, List<PatchOperation>> patchOperations)
+            throws NotImplementedException, BadRequestException, CharonException, NotFoundException {
+
+        throw new NotImplementedException();
+    }
+
     public GroupsGetResponse listGroupsWithPost(SearchRequest searchRequest, Map<String, Boolean> requiredAttributes)
             throws NotImplementedException, BadRequestException, CharonException;
 

diff --git a/...ron-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/GroupResourceManager.java b/...ron-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/GroupResourceManager.java
@@ -706,28 +706,7 @@ public SCIMResponse updateWithPatchForAddRemoveOperations(String existingGroupId
                                                               String excludeAttributes) {
 
         try {
-            Map<String, List<PatchOperation>> patchOperations = new HashMap<>();
-
-            patchOperations.put(SCIMConstants.OperationalConstants.ADD, new ArrayList<>());
-            patchOperations.put(SCIMConstants.OperationalConstants.REMOVE, new ArrayList<>());
-            patchOperations.put(SCIMConstants.OperationalConstants.REPLACE, new ArrayList<>());
-
-            for (PatchOperation patchOperation : opList) {
-                switch (patchOperation.getOperation()) {
-                    case SCIMConstants.OperationalConstants.ADD:
-                        patchOperations.get(SCIMConstants.OperationalConstants.ADD).add(patchOperation);
-                        break;
-                    case SCIMConstants.OperationalConstants.REMOVE:
-                        patchOperations.get(SCIMConstants.OperationalConstants.REMOVE).add(patchOperation);
-                        break;
-                    case SCIMConstants.OperationalConstants.REPLACE:
-                        patchOperations.get(SCIMConstants.OperationalConstants.REPLACE).add(patchOperation);
-                        break;
-                    default:
-                        throw new BadRequestException("Unknown operation: " + patchOperation.getOperation(),
-                                ResponseCodeConstants.INVALID_SYNTAX);
-                }
-            }
+            Map<String, List<PatchOperation>> patchOperations = buildPatchOperationsMap(opList);
 
             SCIMResourceTypeSchema schema = SCIMResourceSchemaManager.getInstance().getGroupResourceSchema();
             String groupName = getGroupName(userManager, existingGroupId);
@@ -767,6 +746,45 @@ public SCIMResponse updateWithPatchForAddRemoveOperations(String existingGroupId
         }
     }
 
+    private Map<String, List<PatchOperation>> buildPatchOperationsMap(List<PatchOperation> opList)
+            throws BadRequestException {
+
+        Map<String, List<PatchOperation>> patchOperations = new HashMap<>();
+
+        patchOperations.put(SCIMConstants.OperationalConstants.ADD, new ArrayList<>());
+        patchOperations.put(SCIMConstants.OperationalConstants.REMOVE, new ArrayList<>());
+        patchOperations.put(SCIMConstants.OperationalConstants.REPLACE, new ArrayList<>());
+
+        for (PatchOperation patchOperation : opList) {
+            switch (patchOperation.getOperation()) {
+                case SCIMConstants.OperationalConstants.ADD:
+                    patchOperations.get(SCIMConstants.OperationalConstants.ADD).add(patchOperation);
+                    break;
+                case SCIMConstants.OperationalConstants.REMOVE:
+                    patchOperations.get(SCIMConstants.OperationalConstants.REMOVE).add(patchOperation);
+                    break;
+                case SCIMConstants.OperationalConstants.REPLACE:
+                    patchOperations.get(SCIMConstants.OperationalConstants.REPLACE).add(patchOperation);
+                    break;
+                default:
+                    throw new BadRequestException("Unknown operation: " + patchOperation.getOperation(),
+                            ResponseCodeConstants.INVALID_SYNTAX);
+            }
+        }
+        return patchOperations;
+    }
+
+    private void updateWithPatchForAddRemoveOperations(String existingGroupId, List<PatchOperation> opList,
+                                                       UserManager userManager) throws BadRequestException,
+            NotImplementedException, NotFoundException, CharonException {
+
+        Map<String, List<PatchOperation>> patchOperations = buildPatchOperationsMap(opList);
+        SCIMResourceTypeSchema schema = SCIMResourceSchemaManager.getInstance().getGroupResourceSchema();
+        String groupName = getGroupName(userManager, existingGroupId);
+        processGroupPatchOperations(patchOperations, schema);
+        userManager.patchGroup(existingGroupId, groupName, patchOperations);
+    }
+
     private String getGroupName(UserManager userManager, String groupId)
             throws NotImplementedException, BadRequestException, CharonException, NotFoundException {
 
@@ -953,16 +971,23 @@ public SCIMResponse updateWithPATCH(String existingGroupId, String patchRequest,
             SCIMResourceTypeSchema schema = SCIMResourceSchemaManager.getInstance().getGroupResourceSchema();
             Map<String, Boolean> requestAttributes = ResourceManagerUtil.getAllAttributeURIs(schema);
 
-            Group oldGroup = userManager.getGroup(existingGroupId, requestAttributes);
-            if (oldGroup == null) {
-                throw new NotFoundException("No group with the id : " + existingGroupId + " exists in the user store.");
-            }
+            List<PatchOperation> opList = getDecoder().decodeRequest(patchRequest);
 
-            // Make a copy of original group. This will be used to restore to the original condition if failure occurs.
-            Group originalGroup = (Group) CopyUtil.deepCopy(oldGroup);
-            Group patchedGroup = doPatchGroup(oldGroup, schema, patchRequest);
+            if (!isDeleteAllUsersOperationFound(opList)) {
+                updateWithPatchForAddRemoveOperations(existingGroupId, opList, userManager);
+            } else {
+                Group oldGroup = userManager.getGroup(existingGroupId, requestAttributes);
+                if (oldGroup == null) {
+                    throw new NotFoundException("No group with the id : " + existingGroupId
+                            + " exists in the user store.");
+                }
+
+                // Make a copy of original group. This will be used to restore the original condition if failure occurs.
+                Group originalGroup = (Group) CopyUtil.deepCopy(oldGroup);
+                Group patchedGroup = doPatchGroup(oldGroup, schema, patchRequest);
 
-            userManager.updateGroup(originalGroup, patchedGroup);
+                userManager.updateGroup(originalGroup, patchedGroup);
+            }
 
             // Build the 204 response.
             Map<String, String> httpHeaders = new HashMap<>();