You are viewing a plain text version of this content. The canonical link for it is here.
Posted to custos@airavata.apache.org by "ganning127 (via GitHub)" <gi...@apache.org> on 2025/01/21 23:04:06 UTC
[PR] Added patch functionality for groups [airavata-custos]
ganning127 opened a new pull request, #415:
URL: https://github.com/apache/airavata-custos/pull/415
With `PATCH`, you only need to pass in the relevant fields you want to change in the body of the request.
**Notes**
- If we don't pass in realm_roles at all (i.e. we don't want to change it), we get it in our handler as an empty list. If we pass in an empty realm_roles (i.e. we want to remove all realm_roles), we get it the SAME way (empty list). This applies for both client_roles, and group attributes. This makes it impossible to detect if users want to keep or remove all list attributes.
- In the current implementation, to remove all realm_roles or client_roles, you pass in ["custos-remove-all"] as the value
- Toe remove all attributes, pass in [{key: "custos-remove-all", value: "anything....asdfa", id: anything...asdfa}]
--
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: custos-unsubscribe@airavata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added patch functionality for groups [airavata-custos]
Posted by "ganning127 (via GitHub)" <gi...@apache.org>.
ganning127 commented on code in PR #415:
URL: https://github.com/apache/airavata-custos/pull/415#discussion_r1947494194
##########
api/src/main/java/org/apache/custos/api/group/GroupManagementController.java:
##########
@@ -267,7 +269,133 @@ public ResponseEntity<Group> updateGroup(@PathVariable("groupId") String groupId
Group updatedGroup = groupManagementService.updateGroup(groupRequest);
return ResponseEntity.ok(updatedGroup);
+ }
+
+ @PatchMapping("/groups/{groupId}")
+ @Operation(
+ summary = "Patch Group",
+ description = "Patches group with given ID. To remove all realm_roles or client_roles, pass in a list with \"custos-remove-all\" as the first list element. To remove all group attributes, pass in a list with \"custos-remove-all\" as the key of the first list element.",
+ requestBody = @io.swagger.v3.oas.annotations.parameters.RequestBody(
+ required = true,
+ content = @Content(
+ schemaProperties = {
+ @SchemaProperty(
+ name = "name",
+ schema = @Schema(
+ type = "string",
+ description = "Group Name"
+ )
+ ),
+ @SchemaProperty(
+ name = "description",
+ schema = @Schema(
+ type = "string",
+ description = "Group Description"
+ )
+ ),
+ @SchemaProperty(
+ name = "attributes",
+ array = @ArraySchema(
+ schema = @Schema(implementation = GroupAttribute.class),
+ arraySchema = @Schema(description = "List of Group Attributes")
+ )
+ ),
+ @SchemaProperty(
+ name = "realm_roles",
+ array = @ArraySchema(
+ schema = @Schema(implementation = String.class),
+ arraySchema = @Schema(description = "List of Realm Roles")
+ )
+ ),
+ @SchemaProperty(
+ name = "client_roles",
+ array = @ArraySchema(
+ schema = @Schema(implementation = String.class),
+ arraySchema = @Schema(description = "List of Client Roles")
+ )
+ )
+ }
+ )
+ ),
+ parameters = {
+ @Parameter(
+ name = "client_id",
+ in = ParameterIn.HEADER,
+ description = "The client ID initiating the group update request",
+ required = true,
+ schema = @Schema(type = "string")
+ ),
+ @Parameter(
+ name = "groupId",
+ in = ParameterIn.PATH,
+ description = "The ID of the group to update",
+ required = true,
+ schema = @Schema(type = "string")
+ )
+ },
+ responses = {
+ @ApiResponse(responseCode = "200", description = "Group updated successfully", content = @Content(schema = @Schema(implementation = Group.class))),
+ @ApiResponse(responseCode = "401", description = "Unauthorized Request", content = @Content()),
+ @ApiResponse(responseCode = "404", description = "When the associated Group cannot be found", content = @Content()),
+ @ApiResponse(responseCode = "500", description = "Internal Server Error", content = @Content())
+ }
+ )
+ public ResponseEntity<Group> patchGroup(@PathVariable("groupId") String groupId, @RequestBody Group request, @RequestHeader HttpHeaders headers) {
+ AuthClaim authClaim = authorize(headers);
+
+ GroupRequest exGroupReq = GroupRequest.newBuilder()
+ .setTenantId(authClaim.getTenantId())
+ .setClientId(authClaim.getIamAuthId())
+ .setClientSec(authClaim.getIamAuthSecret())
+ .setPerformedBy(authClaim.getPerformedBy() != null ? authClaim.getPerformedBy() : Constants.SYSTEM)
+ .setGroup(request.toBuilder().setId(groupId).build())
+ .build();
+
+ Group exGroup = groupManagementService.findGroup(exGroupReq);
+
+ Group.Builder mergedGroupBuilder = exGroup.toBuilder()
+ .setName(!request.getName().isEmpty() ? request.getName() : exGroup.getName())
+ .setDescription(!request.getDescription().isEmpty() ? request.getDescription() : exGroup.getDescription());
+
+ String REMOVE_ALL_TAG = "custos-remove-all";
+
+ List<String> clientRolesLst = request.getClientRolesList();
+ if (!clientRolesLst.isEmpty()) {
+ mergedGroupBuilder.clearClientRoles();
+ if (!clientRolesLst.get(0).equals(REMOVE_ALL_TAG)) {
+ mergedGroupBuilder.addAllClientRoles(request.getClientRolesList());
+ }
+ }
+
+ List<String> realmRolesLst = request.getRealmRolesList();
+ if (!realmRolesLst.isEmpty()) {
+ mergedGroupBuilder.clearRealmRoles();
+ if (!realmRolesLst.get(0).equals(REMOVE_ALL_TAG)) {
+ mergedGroupBuilder.addAllRealmRoles(request.getRealmRolesList());
+ }
+ }
+ List<GroupAttribute> groupAttributeList = request.getAttributesList();
+ if (!groupAttributeList.isEmpty()) {
+ mergedGroupBuilder.clearAttributes();
+ if (!groupAttributeList.get(0).getKey().equals(REMOVE_ALL_TAG)) {
+ mergedGroupBuilder.addAllAttributes(request.getAttributesList());
+ }
+ }
+
+ Group mergedGroup = mergedGroupBuilder.build();
+
+ GroupRequest updateGroupRequest = GroupRequest.newBuilder()
+ .setTenantId(authClaim.getTenantId())
+ .setClientId(authClaim.getIamAuthId())
+ .setClientSec(authClaim.getIamAuthSecret())
+ .setPerformedBy(authClaim.getPerformedBy() != null ? authClaim.getPerformedBy() : Constants.SYSTEM)
+ .setGroup(mergedGroup.toBuilder().setId(groupId).build())
+ .build();
+
Review Comment:
Done!
--
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: issues-unsubscribe@airavata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added patch functionality for groups [airavata-custos]
Posted by "lahirujayathilake (via GitHub)" <gi...@apache.org>.
lahirujayathilake merged PR #415:
URL: https://github.com/apache/airavata-custos/pull/415
--
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: custos-unsubscribe@airavata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added patch functionality for groups [airavata-custos]
Posted by "lahirujayathilake (via GitHub)" <gi...@apache.org>.
lahirujayathilake commented on code in PR #415:
URL: https://github.com/apache/airavata-custos/pull/415#discussion_r1946809063
##########
api/src/main/java/org/apache/custos/api/group/GroupManagementController.java:
##########
@@ -267,7 +269,133 @@ public ResponseEntity<Group> updateGroup(@PathVariable("groupId") String groupId
Group updatedGroup = groupManagementService.updateGroup(groupRequest);
return ResponseEntity.ok(updatedGroup);
+ }
+
+ @PatchMapping("/groups/{groupId}")
+ @Operation(
+ summary = "Patch Group",
+ description = "Patches group with given ID. To remove all realm_roles or client_roles, pass in a list with \"custos-remove-all\" as the first list element. To remove all group attributes, pass in a list with \"custos-remove-all\" as the key of the first list element.",
+ requestBody = @io.swagger.v3.oas.annotations.parameters.RequestBody(
+ required = true,
+ content = @Content(
+ schemaProperties = {
+ @SchemaProperty(
+ name = "name",
+ schema = @Schema(
+ type = "string",
+ description = "Group Name"
+ )
+ ),
+ @SchemaProperty(
+ name = "description",
+ schema = @Schema(
+ type = "string",
+ description = "Group Description"
+ )
+ ),
+ @SchemaProperty(
+ name = "attributes",
+ array = @ArraySchema(
+ schema = @Schema(implementation = GroupAttribute.class),
+ arraySchema = @Schema(description = "List of Group Attributes")
+ )
+ ),
+ @SchemaProperty(
+ name = "realm_roles",
+ array = @ArraySchema(
+ schema = @Schema(implementation = String.class),
+ arraySchema = @Schema(description = "List of Realm Roles")
+ )
+ ),
+ @SchemaProperty(
+ name = "client_roles",
+ array = @ArraySchema(
+ schema = @Schema(implementation = String.class),
+ arraySchema = @Schema(description = "List of Client Roles")
+ )
+ )
+ }
+ )
+ ),
+ parameters = {
+ @Parameter(
+ name = "client_id",
+ in = ParameterIn.HEADER,
+ description = "The client ID initiating the group update request",
+ required = true,
+ schema = @Schema(type = "string")
+ ),
+ @Parameter(
+ name = "groupId",
+ in = ParameterIn.PATH,
+ description = "The ID of the group to update",
+ required = true,
+ schema = @Schema(type = "string")
+ )
+ },
+ responses = {
+ @ApiResponse(responseCode = "200", description = "Group updated successfully", content = @Content(schema = @Schema(implementation = Group.class))),
+ @ApiResponse(responseCode = "401", description = "Unauthorized Request", content = @Content()),
+ @ApiResponse(responseCode = "404", description = "When the associated Group cannot be found", content = @Content()),
+ @ApiResponse(responseCode = "500", description = "Internal Server Error", content = @Content())
+ }
+ )
+ public ResponseEntity<Group> patchGroup(@PathVariable("groupId") String groupId, @RequestBody Group request, @RequestHeader HttpHeaders headers) {
+ AuthClaim authClaim = authorize(headers);
+
+ GroupRequest exGroupReq = GroupRequest.newBuilder()
+ .setTenantId(authClaim.getTenantId())
+ .setClientId(authClaim.getIamAuthId())
+ .setClientSec(authClaim.getIamAuthSecret())
+ .setPerformedBy(authClaim.getPerformedBy() != null ? authClaim.getPerformedBy() : Constants.SYSTEM)
+ .setGroup(request.toBuilder().setId(groupId).build())
+ .build();
+
+ Group exGroup = groupManagementService.findGroup(exGroupReq);
+
+ Group.Builder mergedGroupBuilder = exGroup.toBuilder()
+ .setName(!request.getName().isEmpty() ? request.getName() : exGroup.getName())
+ .setDescription(!request.getDescription().isEmpty() ? request.getDescription() : exGroup.getDescription());
+
+ String REMOVE_ALL_TAG = "custos-remove-all";
+
+ List<String> clientRolesLst = request.getClientRolesList();
+ if (!clientRolesLst.isEmpty()) {
+ mergedGroupBuilder.clearClientRoles();
+ if (!clientRolesLst.get(0).equals(REMOVE_ALL_TAG)) {
+ mergedGroupBuilder.addAllClientRoles(request.getClientRolesList());
+ }
+ }
+
+ List<String> realmRolesLst = request.getRealmRolesList();
+ if (!realmRolesLst.isEmpty()) {
+ mergedGroupBuilder.clearRealmRoles();
+ if (!realmRolesLst.get(0).equals(REMOVE_ALL_TAG)) {
+ mergedGroupBuilder.addAllRealmRoles(request.getRealmRolesList());
+ }
+ }
+ List<GroupAttribute> groupAttributeList = request.getAttributesList();
+ if (!groupAttributeList.isEmpty()) {
+ mergedGroupBuilder.clearAttributes();
+ if (!groupAttributeList.get(0).getKey().equals(REMOVE_ALL_TAG)) {
+ mergedGroupBuilder.addAllAttributes(request.getAttributesList());
+ }
+ }
+
+ Group mergedGroup = mergedGroupBuilder.build();
+
+ GroupRequest updateGroupRequest = GroupRequest.newBuilder()
+ .setTenantId(authClaim.getTenantId())
+ .setClientId(authClaim.getIamAuthId())
+ .setClientSec(authClaim.getIamAuthSecret())
+ .setPerformedBy(authClaim.getPerformedBy() != null ? authClaim.getPerformedBy() : Constants.SYSTEM)
+ .setGroup(mergedGroup.toBuilder().setId(groupId).build())
+ .build();
+
Review Comment:
better to move this logic into the group management service class. Always try to keep the controller class simple
--
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: issues-unsubscribe@airavata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added patch functionality for groups [airavata-custos]
Posted by "ganning127 (via GitHub)" <gi...@apache.org>.
ganning127 commented on PR #415:
URL: https://github.com/apache/airavata-custos/pull/415#issuecomment-2644539513
Thanks for the feedback, @lahirujayathilake! I've moved the business logic to the service class, and added a TODO for updating `ProtobufPropertiesModule.java` in the future.
--
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: issues-unsubscribe@airavata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org