lahirujayathilake commented on code in PR #522:
URL: https://github.com/apache/airavata/pull/522#discussion_r2152719067


##########
modules/research-framework/research-service/src/main/java/org/apache/airavata/research/service/handlers/ResourceHandler.java:
##########
@@ -177,6 +180,23 @@ public Resource getResourceById(String id) {
         return opResource.get();
     }
 
+    public boolean deleteResourceById(String id) {
+        Optional<Resource> opResource = resourceRepository.findById(id);
+
+        if (opResource.isEmpty()) {
+            throw new RuntimeException("Resource not found: " + id);
+        }
+
+        String userEmail = UserContext.userId();
+        System.out.println("Received request to delete with user: " + 
userEmail);
+        if (!opResource.get().getAuthors().contains(userEmail.toLowerCase())) {

Review Comment:
   @ganning127 shall we add a TODO comment here saying that only the owner 
should be able to delete a resource?
   Once we integrate roles and sharing we should restrict this feature



##########
modules/research-framework/research-service/src/main/java/org/apache/airavata/research/service/controller/ResourceController.java:
##########
@@ -29,25 +29,13 @@
 import org.apache.airavata.research.service.enums.ResourceTypeEnum;
 import org.apache.airavata.research.service.handlers.ProjectHandler;
 import org.apache.airavata.research.service.handlers.ResourceHandler;
-import org.apache.airavata.research.service.model.entity.DatasetResource;
-import org.apache.airavata.research.service.model.entity.ModelResource;
-import org.apache.airavata.research.service.model.entity.NotebookResource;
-import org.apache.airavata.research.service.model.entity.Project;
-import org.apache.airavata.research.service.model.entity.RepositoryResource;
-import org.apache.airavata.research.service.model.entity.Resource;
+import org.apache.airavata.research.service.model.entity.*;

Review Comment:
   @ganning127 revert these changes. Don't use wild card imports



##########
modules/research-framework/research-service/src/main/java/org/apache/airavata/research/service/handlers/ResourceHandler.java:
##########
@@ -177,6 +180,23 @@ public Resource getResourceById(String id) {
         return opResource.get();
     }
 
+    public boolean deleteResourceById(String id) {
+        Optional<Resource> opResource = resourceRepository.findById(id);
+
+        if (opResource.isEmpty()) {
+            throw new RuntimeException("Resource not found: " + id);
+        }
+
+        String userEmail = UserContext.userId();
+        System.out.println("Received request to delete with user: " + 
userEmail);

Review Comment:
   remove system outs, if needed either use info or debug log



##########
modules/research-framework/research-service/src/main/java/org/apache/airavata/research/service/handlers/ResourceHandler.java:
##########
@@ -177,6 +180,23 @@ public Resource getResourceById(String id) {
         return opResource.get();
     }
 
+    public boolean deleteResourceById(String id) {
+        Optional<Resource> opResource = resourceRepository.findById(id);
+
+        if (opResource.isEmpty()) {
+            throw new RuntimeException("Resource not found: " + id);

Review Comment:
   use `jakarta.persistence.EntityNotFoundException` and better to log this



##########
modules/research-framework/research-service/src/main/java/org/apache/airavata/research/service/handlers/ResourceHandler.java:
##########
@@ -177,6 +180,23 @@ public Resource getResourceById(String id) {
         return opResource.get();
     }
 
+    public boolean deleteResourceById(String id) {
+        Optional<Resource> opResource = resourceRepository.findById(id);
+
+        if (opResource.isEmpty()) {
+            throw new RuntimeException("Resource not found: " + id);
+        }
+
+        String userEmail = UserContext.userId();
+        System.out.println("Received request to delete with user: " + 
userEmail);
+        if (!opResource.get().getAuthors().contains(userEmail.toLowerCase())) {
+            throw new RuntimeException("User " + userEmail + " not authorized 
to delete resource");

Review Comment:
   better to log these, indicating resource type, id, and user



-- 
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-unsubscr...@airavata.apache.org

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

Reply via email to