chore: re-factor DeleteFileController (#582)

epam · Nov 25, 2024 · fe426b8 · fe426b8
1 parent 17fcbe6
commit fe426b8
Show file tree

Hide file tree

Showing 10 changed files with 74 additions and 108 deletions.
diff --git a/server/src/main/java/com/epam/aidial/core/server/AiDial.java b/server/src/main/java/com/epam/aidial/core/server/AiDial.java
@@ -125,15 +125,16 @@ void start() throws Exception {
             NotificationService notificationService = new NotificationService(resourceService, encryptionService);
             ApplicationService applicationService = new ApplicationService(vertx, client, redis,
                     encryptionService, resourceService, lockService, generator, settings("applications"));
+            ResourceOperationService resourceOperationService = new ResourceOperationService(applicationService,
+                    resourceService, invitationService, shareService, lockService);
             PublicationService publicationService = new PublicationService(encryptionService, resourceService, accessService,
-                    ruleService, notificationService, applicationService, generator, clock);
+                    ruleService, notificationService, applicationService, resourceOperationService, generator, clock);
             RateLimiter rateLimiter = new RateLimiter(vertx, resourceService);
 
             ApiKeyStore apiKeyStore = new ApiKeyStore(resourceService, vertx);
             ConfigStore configStore = new FileConfigStore(vertx, settings("config"), apiKeyStore, upstreamRouteProvider);
 
             TokenStatsTracker tokenStatsTracker = new TokenStatsTracker(vertx, resourceService);
-            ResourceOperationService resourceOperationService = new ResourceOperationService(applicationService, resourceService, invitationService, shareService);
 
             HeartbeatService heartbeatService = new HeartbeatService(
                     vertx, settings("resources").getLong("heartbeatPeriod"));

diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ControllerSelector.java b/server/src/main/java/com/epam/aidial/core/server/controller/ControllerSelector.java
@@ -273,7 +273,7 @@ public class ControllerSelector {
         });
         // DELETE routes
         delete(PATTERN_FILES, (proxy, context, pathMatcher) -> {
-            DeleteFileController controller = new DeleteFileController(proxy, context);
+            ResourceController controller = new ResourceController(proxy, context, false);
             String path = context.getRequest().path();
             return () -> controller.handle(resourcePath(path));
         });

diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/DeleteFileController.java b/server/src/main/java/com/epam/aidial/core/server/controller/DeleteFileController.java
diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/DownloadFileController.java b/server/src/main/java/com/epam/aidial/core/server/controller/DownloadFileController.java
@@ -4,7 +4,6 @@
 import com.epam.aidial.core.server.ProxyContext;
 import com.epam.aidial.core.server.util.ProxyUtil;
 import com.epam.aidial.core.server.vertx.stream.InputStreamReader;
-import com.epam.aidial.core.storage.http.HttpException;
 import com.epam.aidial.core.storage.http.HttpStatus;
 import com.epam.aidial.core.storage.resource.ResourceDescriptor;
 import com.epam.aidial.core.storage.util.EtagHeader;

diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ResourceController.java b/server/src/main/java/com/epam/aidial/core/server/controller/ResourceController.java
@@ -225,31 +225,16 @@ private Future<?> deleteResource(ResourceDescriptor descriptor) {
             return context.respond(HttpStatus.BAD_REQUEST, "Folder not allowed: " + descriptor.getUrl());
         }
 
-        vertx.executeBlocking(() -> {
-                    EtagHeader etag = ProxyUtil.etag(context.getRequest());
-                    String bucketName = descriptor.getBucketName();
-                    String bucketLocation = descriptor.getBucketLocation();
+        EtagHeader etag = ProxyUtil.etag(context.getRequest());
 
-                    return lockService.underBucketLock(bucketLocation, () -> {
-                        invitationService.cleanUpResourceLink(bucketName, bucketLocation, descriptor);
-                        shareService.revokeSharedResource(bucketName, bucketLocation, descriptor);
-
-                        boolean deleted = true;
-
-                        if (descriptor.getType() == ResourceTypes.APPLICATION) {
-                            applicationService.deleteApplication(descriptor, etag);
-                        } else {
-                           deleted = service.deleteResource(descriptor, etag);
-                        }
-
-                        if (!deleted) {
-                            throw new ResourceNotFoundException();
-                        }
-
-                        return null;
-                    });
-                }, false)
-                .onSuccess(ignore -> context.respond(HttpStatus.OK))
+        vertx.executeBlocking(() -> proxy.getResourceOperationService().deleteResource(descriptor, etag), false)
+                .onSuccess(deleted -> {
+                    if (deleted) {
+                        context.respond(HttpStatus.OK);
+                    } else {
+                        context.respond(HttpStatus.NOT_FOUND, "Not found: " + descriptor.getUrl());
+                    }
+                })
                 .onFailure(error -> handleError(descriptor, error));
 
         return Future.succeededFuture();

diff --git a/server/src/main/java/com/epam/aidial/core/server/service/PublicationService.java b/server/src/main/java/com/epam/aidial/core/server/service/PublicationService.java
@@ -60,6 +60,7 @@ public class PublicationService {
     private final RuleService ruleService;
     private final NotificationService notificationService;
     private final ApplicationService applicationService;
+    private final ResourceOperationService resourceOperationService;
     private final Supplier<String> ids;
     private final LongSupplier clock;
 
@@ -598,7 +599,7 @@ private void deleteReviewResources(List<Publication.Resource> resources) {
             String url = resource.getReviewUrl();
             ResourceDescriptor descriptor = ResourceDescriptorFactory.fromPrivateUrl(url, encryption);
             verifyResourceType(descriptor);
-            deleteResource(descriptor);
+            resourceOperationService.deleteResource(descriptor, EtagHeader.ANY);
         }
     }
 
@@ -607,19 +608,7 @@ private void deletePublicResources(List<Publication.Resource> resources) {
             String url = resource.getTargetUrl();
             ResourceDescriptor descriptor = ResourceDescriptorFactory.fromPublicUrl(url);
             verifyResourceType(descriptor);
-            deleteResource(descriptor);
-        }
-    }
-
-    private void deleteResource(ResourceDescriptor descriptor) {
-        try {
-            if (descriptor.getType() == ResourceTypes.APPLICATION) {
-                applicationService.deleteApplication(descriptor, EtagHeader.ANY);
-            } else {
-                resourceService.deleteResource(descriptor, EtagHeader.ANY);
-            }
-        } catch (ResourceNotFoundException e) {
-            // ignore
+            resourceOperationService.deleteResource(descriptor, EtagHeader.ANY);
         }
     }
 

diff --git a/server/src/main/java/com/epam/aidial/core/server/service/ResourceOperationService.java b/server/src/main/java/com/epam/aidial/core/server/service/ResourceOperationService.java
@@ -6,25 +6,34 @@
 import com.epam.aidial.core.storage.http.HttpException;
 import com.epam.aidial.core.storage.http.HttpStatus;
 import com.epam.aidial.core.storage.resource.ResourceDescriptor;
+import com.epam.aidial.core.storage.resource.ResourceType;
+import com.epam.aidial.core.storage.service.LockService;
 import com.epam.aidial.core.storage.service.ResourceService;
 import com.epam.aidial.core.storage.service.ResourceTopic;
 import com.epam.aidial.core.storage.util.EtagHeader;
 import lombok.AllArgsConstructor;
+import org.apache.commons.lang3.mutable.MutableObject;
 
 import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
 
+import static com.epam.aidial.core.server.data.ResourceTypes.APPLICATION;
+import static com.epam.aidial.core.server.data.ResourceTypes.CONVERSATION;
+import static com.epam.aidial.core.server.data.ResourceTypes.FILE;
+import static com.epam.aidial.core.server.data.ResourceTypes.PROMPT;
+
 @AllArgsConstructor
 public class ResourceOperationService {
-    private static final Set<ResourceTypes> ALLOWED_RESOURCES = Set.of(ResourceTypes.FILE, ResourceTypes.CONVERSATION,
-            ResourceTypes.PROMPT, ResourceTypes.APPLICATION);
+    private static final Set<ResourceTypes> ALLOWED_RESOURCES = Set.of(FILE, CONVERSATION,
+            PROMPT, APPLICATION);
 
     private final ApplicationService applicationService;
     private final ResourceService resourceService;
     private final InvitationService invitationService;
     private final ShareService shareService;
+    private final LockService lockService;
 
     public ResourceTopic.Subscription subscribeResources(Collection<ResourceDescriptor> resources,
                                                          Consumer<ResourceEvent> subscriber) {
@@ -47,7 +56,7 @@ public void moveResource(ResourceDescriptor source, ResourceDescriptor destinati
             throw new IllegalStateException("Unsupported type: " + source.getType());
         }
 
-        if (destination.getType() == ResourceTypes.APPLICATION) {
+        if (destination.getType() == APPLICATION) {
             applicationService.copyApplication(source, destination, overwriteIfExists, app -> {
                 if (ApplicationService.isActive(app)) {
                     throw new HttpException(HttpStatus.CONFLICT, "Application must be stopped: " + source.getUrl());
@@ -76,10 +85,51 @@ public void moveResource(ResourceDescriptor source, ResourceDescriptor destinati
             }
         }
 
-        if (destination.getType() == ResourceTypes.APPLICATION) {
+        if (destination.getType() == APPLICATION) {
             applicationService.deleteApplication(source, EtagHeader.ANY);
         } else {
             resourceService.deleteResource(source, EtagHeader.ANY);
         }
     }
+
+    public boolean deleteResource(ResourceDescriptor resource, EtagHeader etag) {
+        verifyResourceToDelete(resource);
+        MutableObject<Boolean> deleted = new MutableObject<>();
+        if (resource.isPrivate()) {
+            String bucketName = resource.getBucketName();
+            String bucketLocation = resource.getBucketLocation();
+            // links to dependent resources should be removed under user's bucket lock
+            lockService.underBucketLock(bucketLocation, () -> {
+                invitationService.cleanUpResourceLink(bucketName, bucketLocation, resource);
+                shareService.revokeSharedResource(bucketName, bucketLocation, resource);
+                deleted.setValue(deleteResourceInternally(resource, etag));
+                return null;
+            });
+        } else {
+            deleted.setValue(deleteResourceInternally(resource, etag));
+        }
+        return deleted.getValue();
+    }
+
+    private static void verifyResourceToDelete(ResourceDescriptor resource) {
+        ResourceType type = resource.getType();
+        if (!(APPLICATION == type || FILE == type
+                || CONVERSATION == type || type == PROMPT)) {
+            throw new IllegalArgumentException("Unsupported resource type to delete: " + type.name());
+        }
+    }
+
+    private boolean deleteResourceInternally(ResourceDescriptor resource, EtagHeader etag) {
+        if (resource.getType() == APPLICATION) {
+            try {
+                applicationService.deleteApplication(resource, etag);
+            } catch (ResourceNotFoundException e) {
+                return false;
+            }
+            return true;
+        } else {
+            return resourceService.deleteResource(resource, etag);
+        }
+    }
+
 }
diff --git a/server/src/test/java/com/epam/aidial/core/server/controller/ControllerSelectorTest.java b/server/src/test/java/com/epam/aidial/core/server/controller/ControllerSelectorTest.java
@@ -329,7 +329,7 @@ public void testSelectDeleteFileController() {
         assertEquals(2, lambda.getCapturedArgCount());
         Object arg1 = lambda.getCapturedArg(0);
         Object arg2 = lambda.getCapturedArg(1);
-        assertInstanceOf(DeleteFileController.class, arg1);
+        assertInstanceOf(ResourceController.class, arg1);
         assertEquals("/v1/files/bucket/folder1/file1.txt", arg2);
     }
 
@@ -345,7 +345,7 @@ public void testSelectDeleteFileController2() {
         assertEquals(2, lambda.getCapturedArgCount());
         Object arg1 = lambda.getCapturedArg(0);
         Object arg2 = lambda.getCapturedArg(1);
-        assertInstanceOf(DeleteFileController.class, arg1);
+        assertInstanceOf(ResourceController.class, arg1);
         assertEquals("/v1/files/bucket/fol%2Fder%201/file1%23.txt", arg2);
     }
 

diff --git a/server/src/test/java/com/epam/aidial/core/server/security/ApiKeyStoreTest.java b/server/src/test/java/com/epam/aidial/core/server/security/ApiKeyStoreTest.java
@@ -88,7 +88,7 @@ public static void afterAll() throws IOException {
     }
 
     @BeforeEach
-    public void beforeEach() throws Exception {
+    public void beforeEach() {
         RKeys keys = redissonClient.getKeys();
         for (String key : keys.getKeys()) {
             keys.delete(key);

diff --git a/server/src/test/java/com/epam/aidial/core/server/token/TokenStatsTrackerTest.java b/server/src/test/java/com/epam/aidial/core/server/token/TokenStatsTrackerTest.java
@@ -86,7 +86,7 @@ public static void afterAll() throws IOException {
     }
 
     @BeforeEach
-    public void beforeEach() throws Exception {
+    public void beforeEach() {
         RKeys keys = redissonClient.getKeys();
         for (String key : keys.getKeys()) {
             keys.delete(key);