add ut for alter database

apache · Dec 13, 2024 · 9edbd55 · 9edbd55
1 parent 8b97cf7
commit 9edbd55
Show file tree

Hide file tree

Showing 10 changed files with 171 additions and 15 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
@@ -34,7 +34,7 @@
 import org.apache.paimon.rest.exceptions.NoSuchResourceException;
 import org.apache.paimon.rest.requests.AlterDatabaseRequest;
 import org.apache.paimon.rest.requests.CreateDatabaseRequest;
-import org.apache.paimon.rest.responses.AlertDatabaseResponse;
+import org.apache.paimon.rest.responses.AlterDatabaseResponse;
 import org.apache.paimon.rest.responses.ConfigResponse;
 import org.apache.paimon.rest.responses.CreateDatabaseResponse;
 import org.apache.paimon.rest.responses.DatabaseName;
@@ -188,18 +188,20 @@ public void alterDatabase(String name, List<DatabaseChange> changes, boolean ign
             Map<String, String> insertProperties = insertProperties2removeProperties.getLeft();
             Set<String> removeProperties = insertProperties2removeProperties.getRight();
             AlterDatabaseRequest request =
-                    new AlterDatabaseRequest(removeProperties, insertProperties);
-            AlertDatabaseResponse response =
+                    new AlterDatabaseRequest(new ArrayList<>(removeProperties), insertProperties);
+            AlterDatabaseResponse response =
                     client.post(
                             resourcePaths.database(name),
                             request,
-                            AlertDatabaseResponse.class,
+                            AlterDatabaseResponse.class,
                             headers());
             if (response.getUpdated().isEmpty()) {
                 throw new IllegalStateException("Failed to update properties");
             }
         } catch (NoSuchResourceException e) {
-            throw new DatabaseNotExistException(name);
+            if (!ignoreIfNotExists) {
+                throw new DatabaseNotExistException(name);
+            }
         }
     }
 

diff --git a/paimon-core/src/main/java/org/apache/paimon/rest/requests/AlterDatabaseRequest.java b/paimon-core/src/main/java/org/apache/paimon/rest/requests/AlterDatabaseRequest.java
@@ -24,8 +24,8 @@
 import org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonGetter;
 import org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonProperty;
 
+import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 /** Request for altering database. */
 public class AlterDatabaseRequest implements RESTRequest {
@@ -34,21 +34,21 @@ public class AlterDatabaseRequest implements RESTRequest {
     private static final String FIELD_UPDATES = "updates";
 
     @JsonProperty(FIELD_REMOVALS)
-    private Set<String> removals;
+    private List<String> removals;
 
     @JsonProperty(FIELD_UPDATES)
     private Map<String, String> updates;
 
     @JsonCreator
     public AlterDatabaseRequest(
-            @JsonProperty(FIELD_REMOVALS) Set<String> removals,
+            @JsonProperty(FIELD_REMOVALS) List<String> removals,
             @JsonProperty(FIELD_UPDATES) Map<String, String> updates) {
         this.removals = removals;
         this.updates = updates;
     }
 
     @JsonGetter(FIELD_REMOVALS)
-    public Set<String> getRemovals() {
+    public List<String> getRemovals() {
         return removals;
     }
 

diff --git a/...rest/responses/AlertDatabaseResponse.java → ...rest/responses/AlterDatabaseResponse.java b/...rest/responses/AlertDatabaseResponse.java → ...rest/responses/AlterDatabaseResponse.java
@@ -20,33 +20,49 @@
 
 import org.apache.paimon.rest.RESTResponse;
 
+import org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonCreator;
+import org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonGetter;
+import org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonProperty;
+
 import java.util.List;
 
 /** Response for altering database. */
-public class AlertDatabaseResponse implements RESTResponse {
+public class AlterDatabaseResponse implements RESTResponse {
+
+    private static final String FIELD_REMOVED = "removed";
+    private static final String FIELD_UPDATED = "updated";
+    private static final String FIELD_MISSING = "missing";
 
-    // List of namespace property keys that were removed
+    @JsonProperty(FIELD_REMOVED)
     private List<String> removed;
-    // List of namespace property keys that were added or updated
+
+    @JsonProperty(FIELD_UPDATED)
     private List<String> updated;
-    // List of properties that were requested for removal that were not found in the namespace's
-    // properties
+
+    @JsonProperty(FIELD_MISSING)
     private List<String> missing;
 
-    public AlertDatabaseResponse(List<String> removed, List<String> updated, List<String> missing) {
+    @JsonCreator
+    public AlterDatabaseResponse(
+            @JsonProperty(FIELD_REMOVED) List<String> removed,
+            @JsonProperty(FIELD_UPDATED) List<String> updated,
+            @JsonProperty(FIELD_MISSING) List<String> missing) {
         this.removed = removed;
         this.updated = updated;
         this.missing = missing;
     }
 
+    @JsonGetter(FIELD_REMOVED)
     public List<String> getRemoved() {
         return removed;
     }
 
+    @JsonGetter(FIELD_UPDATED)
     public List<String> getUpdated() {
         return updated;
     }
 
+    @JsonGetter(FIELD_MISSING)
     public List<String> getMissing() {
         return missing;
     }

diff --git a/paimon-core/src/test/java/org/apache/paimon/catalog/CachingCatalogTest.java b/paimon-core/src/test/java/org/apache/paimon/catalog/CachingCatalogTest.java
@@ -44,6 +44,7 @@
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 
 import java.io.FileNotFoundException;
 import java.time.Duration;
@@ -63,6 +64,8 @@
 import static org.apache.paimon.options.CatalogOptions.CACHE_MANIFEST_SMALL_FILE_THRESHOLD;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.when;
 
 class CachingCatalogTest extends CatalogTestBase {
 
@@ -86,6 +89,26 @@ public void testListDatabasesWhenNoDatabases() {
         assertThat(databases).contains("db");
     }
 
+    @Test
+    public void testInvalidateWhenDatabaseIsAltered() throws Exception {
+        Catalog mockcatalog = Mockito.mock(Catalog.class);
+        Catalog catalog = new CachingCatalog(mockcatalog);
+        String databaseName = "db";
+        boolean ignoreIfExists = false;
+        Database database = Database.of(databaseName);
+        Database secondDatabase = Database.of(databaseName);
+        when(mockcatalog.getDatabase(databaseName)).thenReturn(database, secondDatabase);
+        doNothing().when(mockcatalog).alterDatabase(databaseName, emptyList(), ignoreIfExists);
+        Database cachingDatabase = catalog.getDatabase(databaseName);
+        assertThat(cachingDatabase.name()).isEqualTo(databaseName);
+        catalog.alterDatabase(databaseName, emptyList(), ignoreIfExists);
+        Database newCachingDatabase = catalog.getDatabase(databaseName);
+        // same as secondDatabase means cache is invalidated, so call getDatabase again then return
+        // secondDatabase
+        assertThat(newCachingDatabase).isNotSameAs(database);
+        assertThat(newCachingDatabase).isSameAs(secondDatabase);
+    }
+
     @Test
     public void testInvalidateSystemTablesIfBaseTableIsModified() throws Exception {
         Catalog catalog = new CachingCatalog(this.catalog);

diff --git a/paimon-core/src/test/java/org/apache/paimon/catalog/CatalogTestBase.java b/paimon-core/src/test/java/org/apache/paimon/catalog/CatalogTestBase.java
@@ -54,6 +54,7 @@
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.Assert.assertEquals;
 
 /** Base test class of paimon catalog in {@link Catalog}. */
 public abstract class CatalogTestBase {
@@ -960,4 +961,32 @@ public void testTableUUID() throws Exception {
         assertThat(Long.parseLong(uuid.substring((identifier.getFullName() + ".").length())))
                 .isGreaterThan(0);
     }
+
+    protected void alterDatabaseAddPropertyWhenSupport() throws Exception {
+        // Alter database
+        String databaseName = "db_to_alter_add";
+        catalog.createDatabase(databaseName, false);
+        catalog.alterDatabase(
+                databaseName,
+                Lists.newArrayList(DatabaseChange.setProperty("key", "value")),
+                false);
+        Database db = catalog.getDatabase(databaseName);
+        assertEquals("value", db.options().get("key"));
+    }
+
+    protected void alterDatabaseRemovePropertyWhenSupport() throws Exception {
+        // Alter database
+        String databaseName = "db_to_alter_remove";
+        String key = "key";
+        String value = "value";
+        catalog.createDatabase(databaseName, false);
+        catalog.alterDatabase(
+                databaseName, Lists.newArrayList(DatabaseChange.setProperty(key, value)), false);
+        Database db = catalog.getDatabase(databaseName);
+        assertEquals(value, db.options().get(key));
+        catalog.alterDatabase(
+                databaseName, Lists.newArrayList(DatabaseChange.removeProperty(key)), false);
+        db = catalog.getDatabase(databaseName);
+        assertEquals(false, db.options().containsKey(key));
+    }
 }
diff --git a/paimon-core/src/test/java/org/apache/paimon/jdbc/JdbcCatalogTest.java b/paimon-core/src/test/java/org/apache/paimon/jdbc/JdbcCatalogTest.java
@@ -122,4 +122,14 @@ public void testSerializeTable() throws Exception {
                     }
                 });
     }
+
+    @Test
+    public void testAlterDatabaseAddProperty() throws Exception {
+        this.alterDatabaseAddPropertyWhenSupport();
+    }
+
+    @Test
+    public void testAlterDatabaseRemoveProperty() throws Exception {
+        this.alterDatabaseRemovePropertyWhenSupport();
+    }
 }
diff --git a/paimon-core/src/test/java/org/apache/paimon/rest/MockRESTMessage.java b/paimon-core/src/test/java/org/apache/paimon/rest/MockRESTMessage.java
@@ -18,13 +18,17 @@
 
 package org.apache.paimon.rest;
 
+import org.apache.paimon.rest.requests.AlterDatabaseRequest;
 import org.apache.paimon.rest.requests.CreateDatabaseRequest;
+import org.apache.paimon.rest.responses.AlterDatabaseResponse;
 import org.apache.paimon.rest.responses.CreateDatabaseResponse;
 import org.apache.paimon.rest.responses.DatabaseName;
 import org.apache.paimon.rest.responses.ErrorResponse;
 import org.apache.paimon.rest.responses.GetDatabaseResponse;
 import org.apache.paimon.rest.responses.ListDatabasesResponse;
 
+import org.apache.paimon.shade.guava30.com.google.common.collect.Lists;
+
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -68,4 +72,15 @@ public static ListDatabasesResponse listDatabasesResponse(String name) {
     public static ErrorResponse noSuchResourceExceptionErrorResponse() {
         return new ErrorResponse("message", 404, new ArrayList<>());
     }
+
+    public static AlterDatabaseRequest alterDatabaseRequest() {
+        Map<String, String> add = new HashMap<>();
+        add.put("add", "value");
+        return new AlterDatabaseRequest(Lists.newArrayList("remove"), add);
+    }
+
+    public static AlterDatabaseResponse alterDatabaseResponse() {
+        return new AlterDatabaseResponse(
+                Lists.newArrayList("remove"), Lists.newArrayList("add"), new ArrayList<>());
+    }
 }
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
@@ -22,6 +22,7 @@
 import org.apache.paimon.catalog.Database;
 import org.apache.paimon.options.CatalogOptions;
 import org.apache.paimon.options.Options;
+import org.apache.paimon.rest.responses.AlterDatabaseResponse;
 import org.apache.paimon.rest.responses.CreateDatabaseResponse;
 import org.apache.paimon.rest.responses.ErrorResponse;
 import org.apache.paimon.rest.responses.GetDatabaseResponse;
@@ -185,6 +186,33 @@ public void testDropDatabaseWhenCascadeIsFalseAndTablesExist() throws Exception
         verify(mockRestCatalog, times(1)).listTables(eq(name));
     }
 
+    @Test
+    public void testAlterDatabase() throws Exception {
+        String name = MockRESTMessage.databaseName();
+        AlterDatabaseResponse response = MockRESTMessage.alterDatabaseResponse();
+        mockResponse(mapper.writeValueAsString(response), 200);
+        assertDoesNotThrow(() -> mockRestCatalog.alterDatabase(name, new ArrayList<>(), true));
+    }
+
+    @Test
+    public void testAlterDatabaseWhenDatabaseNotExistAndIgnoreIfNotExistsIsFalse()
+            throws Exception {
+        String name = MockRESTMessage.databaseName();
+        ErrorResponse response = MockRESTMessage.noSuchResourceExceptionErrorResponse();
+        mockResponse(mapper.writeValueAsString(response), 404);
+        assertThrows(
+                Catalog.DatabaseNotExistException.class,
+                () -> mockRestCatalog.alterDatabase(name, new ArrayList<>(), false));
+    }
+
+    @Test
+    public void testAlterDatabaseWhenDatabaseNotExistAndIgnoreIfNotExistsIsTrue() throws Exception {
+        String name = MockRESTMessage.databaseName();
+        ErrorResponse response = MockRESTMessage.noSuchResourceExceptionErrorResponse();
+        mockResponse(mapper.writeValueAsString(response), 404);
+        assertDoesNotThrow(() -> mockRestCatalog.alterDatabase(name, new ArrayList<>(), true));
+    }
+
     private void mockResponse(String mockResponse, int httpCode) {
         MockResponse mockResponseObj =
                 new MockResponse()

diff --git a/paimon-core/src/test/java/org/apache/paimon/rest/RESTObjectMapperTest.java b/paimon-core/src/test/java/org/apache/paimon/rest/RESTObjectMapperTest.java
@@ -18,7 +18,9 @@
 
 package org.apache.paimon.rest;
 
+import org.apache.paimon.rest.requests.AlterDatabaseRequest;
 import org.apache.paimon.rest.requests.CreateDatabaseRequest;
+import org.apache.paimon.rest.responses.AlterDatabaseResponse;
 import org.apache.paimon.rest.responses.ConfigResponse;
 import org.apache.paimon.rest.responses.CreateDatabaseResponse;
 import org.apache.paimon.rest.responses.ErrorResponse;
@@ -103,4 +105,25 @@ public void listDatabaseResponseParseTest() throws Exception {
         assertEquals(response.getDatabases().size(), parseData.getDatabases().size());
         assertEquals(name, parseData.getDatabases().get(0).getName());
     }
+
+    @Test
+    public void alterDatabaseRequestParseTest() throws Exception {
+        AlterDatabaseRequest request = MockRESTMessage.alterDatabaseRequest();
+        String requestStr = mapper.writeValueAsString(request);
+        AlterDatabaseRequest parseData = mapper.readValue(requestStr, AlterDatabaseRequest.class);
+        assertEquals(request.getRemovals().size(), parseData.getRemovals().size());
+        assertEquals(request.getUpdates().size(), parseData.getUpdates().size());
+    }
+
+    @Test
+    public void alertDatabaseResponseParseTest() throws Exception {
+        String name = MockRESTMessage.databaseName();
+        AlterDatabaseResponse response = MockRESTMessage.alterDatabaseResponse();
+        String responseStr = mapper.writeValueAsString(response);
+        AlterDatabaseResponse parseData =
+                mapper.readValue(responseStr, AlterDatabaseResponse.class);
+        assertEquals(response.getRemoved().size(), parseData.getRemoved().size());
+        assertEquals(response.getUpdated().size(), parseData.getUpdated().size());
+        assertEquals(response.getMissing().size(), parseData.getMissing().size());
+    }
 }
diff --git a/paimon-hive/paimon-hive-catalog/src/test/java/org/apache/paimon/hive/HiveCatalogTest.java b/paimon-hive/paimon-hive-catalog/src/test/java/org/apache/paimon/hive/HiveCatalogTest.java
@@ -173,6 +173,16 @@ private void testHiveConfDirFromEnvImpl() {
         assertThat(hiveConf.get("hive.metastore.uris")).isEqualTo("dummy-hms");
     }
 
+    @Test
+    public void testAlterDatabaseAddProperty() throws Exception {
+        this.alterDatabaseAddPropertyWhenSupport();
+    }
+
+    @Test
+    public void testAlterDatabaseRemoveProperty() throws Exception {
+        this.alterDatabaseRemovePropertyWhenSupport();
+    }
+
     @Test
     public void testAddHiveTableParameters() {
         try {