From b14a85581aa90f3ba1062fc366c1a33f2be0d609 Mon Sep 17 00:00:00 2001
From: Alok Joshi <AJoshi19@slb.com>
Date: Thu, 30 Mar 2023 21:18:34 -0500
Subject: [PATCH] MR suggested changes

---
 docs/tutorial/StorageService.md               | 21 +++++++++----------
 .../opengroup/osdu/storage/api/PatchApi.java  |  2 +-
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/docs/tutorial/StorageService.md b/docs/tutorial/StorageService.md
index d6e36d77c..b381ad0f8 100644
--- a/docs/tutorial/StorageService.md
+++ b/docs/tutorial/StorageService.md
@@ -666,21 +666,21 @@ Records patch API has the following response codes:
 |:-----|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | 200  | The update operation succeeds fully, all records’ data and/or metadata are updated.                                                                                               |                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                |
 | 206  | The update operation succeeds partially. Some records are not updated due to different reasons, including records not found or user does not have permission to edit the records. |                                                                                                                                                                                                                                                                                                                                                                                                                      
-| 400  | The update operation fails when the remove operation makes Legal Tags or ACLs empty.                                                                                              |
+| 400  | The update operation fails when the input validation fails. Please check below section for more details.                                                                          |
 
 ### Input Validation <a name="patch-input-validation"></a>
 To remain compliant with the domain data models and business requirements, we perform certain input validation
 on the request payload. Please see below table for details:
 
-|                                                                              | Add                                                                              | Replace                                                                     | Remove                                                                      | Remarks                                                                                                                                                       |
-|------------------------------------------------------------------------------|----------------------------------------------------------------------------------|-----------------------------------------------------------------------------|-----------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| /kind                                                                        | Bad Request                                                                      | Replaces kind                                                               | Bad Request                                                                 | `kind` can only be replaced; `value` must be a raw string. Path must match exactly to `/kind`                                                                 |
-| /tags                                                                        | Replaces tags with `value`. Creates `/tags` if it doesn't exist                  | Replaces tags with `value`. `/tags` must exist                              | Removes tags, `value` is ignored. `/tags` must exist                        | `add` and `replace` behavior similar because `/tags` is an object member                                                                                      |
-| /tags/key                                                                    | Adds `"key" : "value"` to tags, `/tags` must exist                               | Replaces `/tags/key` with `value`. `/tags/key` must exist                   | Removes `"key" : "value"` from tags, `/tags/key` must exist                 |                                                                                                                                                               |
-| /acl/viewers OR /acl/owners OR /legal/legaltags OR /ancestry/parents         | Replaces the target array with value. Creates the attribute if it doesn't exist  | Replaces the target attribute with new value. Target location must exist    | Only `/ancestry/parents` can be removed                                     | In case of add or replace, Path should be an exact match and value must be an array of string values                                                          |
-| /acl/viewers/0 OR /acl/owners/0 OR /legal/legaltags/0 OR /ancestry/parents/0 | Adds value to the target index in the array                                      | Replaces value at the target index in the array. Target location must exist | Removes value at the target index in the array. Target location must exist  | Character `-` can be used to mention last index of the target array. For acl and legaltag, the target value must not be an empty array after applying Patch   |
-| /data                                                                        |                                                                                  |                                                                             |                                                                             | `/data` doesn't adhere to a rigid structure, therefore users must be cautious when modifying `/data`, as data corruption can cause indexing/search issues     |
-| /meta                                                                        |                                                                                  |                                                                             |                                                                             | if an update for `/meta`, it should be compliant with its structure (i.e. array of Map<String, Object>)                                                       |
+|                                                                              | Add                                                                              | Replace                                                                     | Remove                                                                      | Remarks                                                                                                                                                                                                                                            |
+|------------------------------------------------------------------------------|----------------------------------------------------------------------------------|-----------------------------------------------------------------------------|-----------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| /kind                                                                        | Bad Request                                                                      | Replaces kind                                                               | Bad Request                                                                 | `kind` can only be replaced; `value` must be a raw string & valid kind. Path must match exactly to `/kind`                                                                                                                                         |
+| /tags                                                                        | Replaces tags with `value`. Creates `/tags` if it doesn't exist                  | Replaces tags with `value`. `/tags` must exist                              | Removes tags, `value` is ignored. `/tags` must exist                        | `add` and `replace` behavior similar because `/tags` is an object member                                                                                                                                                                           |
+| /tags/key                                                                    | Adds `"key" : "value"` to tags, `/tags` must exist                               | Replaces `/tags/key` with `value`. `/tags/key` must exist                   | Removes `"key" : "value"` from tags, `/tags/key` must exist                 |                                                                                                                                                                                                                                                    |
+| /acl/viewers OR /acl/owners OR /legal/legaltags OR /ancestry/parents         | Replaces the target array with value. Creates the attribute if it doesn't exist  | Replaces the target attribute with new value. Target location must exist    | Only `/ancestry/parents` can be removed                                     | In case of add or replace, Path should be an exact match and value must be an array of string values                                                                                                                                               |
+| /acl/viewers/0 OR /acl/owners/0 OR /legal/legaltags/0 OR /ancestry/parents/0 | Adds value to the target index in the array                                      | Replaces value at the target index in the array. Target location must exist | Removes value at the target index in the array. Target location must exist  | Character `-` can be used to mention last index of the target array. For acl and legaltag, the target value must not be an empty array after applying Patch                                                                                        |
+| /data                                                                        |                                                                                  |                                                                             |                                                                             | `/data` doesn't adhere to a rigid structure, therefore users must be cautious when modifying `/data` attributes. Value type must adhere to attribute type defined in Schema service. Any type change can potentially cause indexing/search issues. |
+| /meta                                                                        |                                                                                  |                                                                             |                                                                             | if an update for `/meta`, it should be compliant with its structure (i.e. array of Map<String, Object>)                                                                                                                                            |
 
 
 Check out some examples below, but refer to the [Patch RFC spec](https://www.rfc-editor.org/rfc/rfc6902) for a comprehensive documentation on JsonPatch and more examples.
@@ -851,7 +851,6 @@ curl --request PATCH \
 |-----------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------|
 | Header `Content-Type`       | application/json                                                                                                                                                                                     | application/json-patch+json                         |
 | Supported Record properties | acl, tags, legaltags                                                                                                                                                                                 | acl, tags, legaltags, ancestry, kind, data, meta    |
-| Supported operations        | add, remove, replace                                                                                                                                                                                 | add, remove, replace                                |
 | `ops` field in payload      | array of [PatchOperation](https://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/blob/master/src/main/java/org/opengroup/osdu/core/common/model/storage/PatchOperation.java) | [JsonPatch](https://www.rfc-editor.org/rfc/rfc6902) |
 | Maximum number of records   | 500                                                                                                                                                                                                  | 100                                                 |
 
diff --git a/storage-core/src/main/java/org/opengroup/osdu/storage/api/PatchApi.java b/storage-core/src/main/java/org/opengroup/osdu/storage/api/PatchApi.java
index e6d1f35a2..82f8a1b52 100644
--- a/storage-core/src/main/java/org/opengroup/osdu/storage/api/PatchApi.java
+++ b/storage-core/src/main/java/org/opengroup/osdu/storage/api/PatchApi.java
@@ -109,7 +109,7 @@ public class PatchApi {
     @PatchMapping(consumes = "application/json-patch+json", produces = MediaType.APPLICATION_JSON_VALUE)
     @PreAuthorize("@authorizationFilter.hasRole('" + StorageRole.CREATOR + "', '" + StorageRole.ADMIN + "')")
     public ResponseEntity<PatchRecordsResponse> patchRecords(@Parameter(description = "x-collaboration") @RequestHeader(name = CollaborationFilter.X_COLLABORATION_HEADER_NAME, required = false) @Valid @ValidateCollaborationContext String collaborationDirectives,
-                                                             @Parameter(description = "Records to be pathced") @RequestBody @Valid PatchRecordsRequestModel patchRecordsRequest) {
+                                                             @Parameter(description = "Records to be patched") @RequestBody @Valid PatchRecordsRequestModel patchRecordsRequest) {
         Optional<CollaborationContext> collaborationContext = collaborationContextFactory.create(collaborationDirectives);
         PatchRecordsResponse response = this.patchRecordsService.patchRecords(patchRecordsRequest.getQuery().getIds(), patchRecordsRequest.getOps(), this.headers.getUserEmail(), collaborationContext);
         if (!response.getNotFoundRecordIds().isEmpty() || !response.getFailedRecordIds().isEmpty()) {
-- 
GitLab