add ut for drop partition

jerry-024 · Dec 27, 2024 · f0bddf0 · f0bddf0
1 parent 35aa8ec
commit f0bddf0
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 14 deletions.
diff --git a/paimon-core/src/main/java/org/apache/paimon/rest/RESTCatalog.java b/paimon-core/src/main/java/org/apache/paimon/rest/RESTCatalog.java
@@ -390,23 +390,26 @@ public void createPartition(Identifier identifier, Map<String, String> partition
     }
 
     @Override
-    // todo: if table not exist how to drop partition
     public void dropPartition(Identifier identifier, Map<String, String> partitions)
             throws TableNotExistException, PartitionNotExistException {
         checkNotSystemTable(identifier, "dropPartition");
         dropPartitionMetadata(identifier, partitions);
         Table table = getTable(identifier);
         if (table != null) {
-            FileStoreTable fileStoreTable = (FileStoreTable) table;
-            try (FileStoreCommit commit =
-                    fileStoreTable
-                            .store()
-                            .newCommit(
-                                    createCommitUser(
-                                            fileStoreTable.coreOptions().toConfiguration()))) {
-                commit.dropPartitions(
-                        Collections.singletonList(partitions), BatchWriteBuilder.COMMIT_IDENTIFIER);
-            }
+            cleanPartitionsInFileSystem(table, partitions);
+        }
+    }
+
+    @VisibleForTesting
+    void cleanPartitionsInFileSystem(Table table, Map<String, String> partitions) {
+        FileStoreTable fileStoreTable = (FileStoreTable) table;
+        try (FileStoreCommit commit =
+                fileStoreTable
+                        .store()
+                        .newCommit(
+                                createCommitUser(fileStoreTable.coreOptions().toConfiguration()))) {
+            commit.dropPartitions(
+                    Collections.singletonList(partitions), BatchWriteBuilder.COMMIT_IDENTIFIER);
         }
     }
 
@@ -517,16 +520,17 @@ protected GetTableResponse getTableResponse(Identifier identifier)
     }
 
     protected SuccessResponse dropPartitionMetadata(
-            Identifier identifier, Map<String, String> partitions) throws TableNotExistException {
+            Identifier identifier, Map<String, String> partitions)
+            throws TableNoPermissionException {
         try {
             DropPartitionRequest request = new DropPartitionRequest(partitions);
             return client.delete(
                     resourcePaths.partitions(
                             identifier.getDatabaseName(), identifier.getTableName()),
                     request,
                     headers());
-        } catch (NoSuchResourceException e) {
-            throw new TableNotExistException(identifier);
+        } catch (NoSuchResourceException ignore) {
+            return new SuccessResponse();
         } catch (ForbiddenException e) {
             throw new TableNoPermissionException(identifier, e);
         }

diff --git a/paimon-core/src/test/java/org/apache/paimon/rest/RESTCatalogTest.java b/paimon-core/src/test/java/org/apache/paimon/rest/RESTCatalogTest.java
@@ -62,6 +62,7 @@
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyList;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -390,6 +391,79 @@ public void testCreatePartitionWhenTableNoPermissionException() throws Exception
                                 Identifier.create(databaseName, "table"), partitionSpec));
     }
 
+    @Test
+    public void testDropPartition() throws Exception {
+        String databaseName = MockRESTMessage.databaseName();
+        Map<String, String> partitionSpec = new HashMap<>();
+        GetTableResponse response = MockRESTMessage.getTableResponse();
+        partitionSpec.put(response.getSchema().primaryKeys().get(0), "1");
+        mockResponse(mapper.writeValueAsString(new SuccessResponse()), 200);
+        mockResponse(mapper.writeValueAsString(response), 200);
+        doNothing().when(mockRestCatalog).cleanPartitionsInFileSystem(any(), any());
+        assertDoesNotThrow(
+                () ->
+                        mockRestCatalog.dropPartition(
+                                Identifier.create(databaseName, "table"), partitionSpec));
+        verify(mockRestCatalog, times(1)).dropPartitionMetadata(any(), any());
+        verify(mockRestCatalog, times(1)).getTable(any());
+        verify(mockRestCatalog, times(1)).cleanPartitionsInFileSystem(any(), any());
+    }
+
+    @Test
+    public void testDropPartitionWhenPartitionNoExist() throws Exception {
+        String databaseName = MockRESTMessage.databaseName();
+        Map<String, String> partitionSpec = new HashMap<>();
+        GetTableResponse response = MockRESTMessage.getTableResponse();
+        partitionSpec.put(response.getSchema().primaryKeys().get(0), "1");
+        mockResponse(mapper.writeValueAsString(new SuccessResponse()), 404);
+        mockResponse(mapper.writeValueAsString(response), 200);
+        doNothing().when(mockRestCatalog).cleanPartitionsInFileSystem(any(), any());
+        assertDoesNotThrow(
+                () ->
+                        mockRestCatalog.dropPartition(
+                                Identifier.create(databaseName, "table"), partitionSpec));
+        verify(mockRestCatalog, times(1)).dropPartitionMetadata(any(), any());
+        verify(mockRestCatalog, times(1)).getTable(any());
+        verify(mockRestCatalog, times(1)).cleanPartitionsInFileSystem(any(), any());
+    }
+
+    @Test
+    public void testDropPartitionWhenTableNoPermission() throws Exception {
+        String databaseName = MockRESTMessage.databaseName();
+        Map<String, String> partitionSpec = new HashMap<>();
+        GetTableResponse response = MockRESTMessage.getTableResponse();
+        partitionSpec.put(response.getSchema().primaryKeys().get(0), "1");
+        mockResponse(mapper.writeValueAsString(new SuccessResponse()), 403);
+        doNothing().when(mockRestCatalog).cleanPartitionsInFileSystem(any(), any());
+        assertThrows(
+                Catalog.TableNoPermissionException.class,
+                () ->
+                        mockRestCatalog.dropPartition(
+                                Identifier.create(databaseName, "table"), partitionSpec));
+        verify(mockRestCatalog, times(1)).dropPartitionMetadata(any(), any());
+        verify(mockRestCatalog, times(0)).getTable(any());
+        verify(mockRestCatalog, times(0)).cleanPartitionsInFileSystem(any(), any());
+    }
+
+    @Test
+    public void testDropPartitionWhenTableNoExist() throws Exception {
+        String databaseName = MockRESTMessage.databaseName();
+        Map<String, String> partitionSpec = new HashMap<>();
+        GetTableResponse response = MockRESTMessage.getTableResponse();
+        partitionSpec.put(response.getSchema().primaryKeys().get(0), "1");
+        mockResponse(mapper.writeValueAsString(new SuccessResponse()), 200);
+        mockResponse("", 404);
+        doNothing().when(mockRestCatalog).cleanPartitionsInFileSystem(any(), any());
+        assertThrows(
+                Catalog.TableNotExistException.class,
+                () ->
+                        mockRestCatalog.dropPartition(
+                                Identifier.create(databaseName, "table"), partitionSpec));
+        verify(mockRestCatalog, times(1)).dropPartitionMetadata(any(), any());
+        verify(mockRestCatalog, times(1)).getTable(any());
+        verify(mockRestCatalog, times(0)).cleanPartitionsInFileSystem(any(), any());
+    }
+
     @Test
     public void testListPartitionsWhenMetastorePartitionedIsTrue() throws Exception {
         Options options = mockInitOptions();