From c5ecc852959daaf575b45d27194795a6477086ef Mon Sep 17 00:00:00 2001 From: Ricco Chen Date: Mon, 28 Oct 2024 11:42:16 +0800 Subject: [PATCH] [#4968] improvement(api): Unify the modification behavior of the comment field (#5121) ### What changes were proposed in this pull request? - deprecated the removeComment change - the new comment of updateComment supports null and empty string ### Why are the changes needed? Fix: #4968 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Pass existing tests Co-authored-by: chenzeping.ricco --- .../org/apache/gravitino/file/FilesetChange.java | 7 ++++++- .../hadoop/TestHadoopCatalogOperations.java | 2 +- .../hadoop/integration/test/HadoopCatalogIT.java | 2 +- .../kafka/integration/test/CatalogKafkaIT.java | 4 ++++ .../gravitino/client/TestFilesetCatalog.java | 2 +- .../client/integration/test/CatalogIT.java | 6 +++++- .../client/integration/test/MetalakeIT.java | 8 +++++++- .../gravitino/client/integration/test/TagIT.java | 8 ++++++++ .../client-python/gravitino/api/fileset_change.py | 11 +++++++++-- .../gravitino/catalog/fileset_catalog.py | 2 +- .../dto/requests/catalog_update_request.py | 4 ++-- .../dto/requests/fileset_update_request.py | 15 +++++++-------- .../dto/requests/metalake_update_request.py | 9 ++------- .../tests/integration/test_catalog.py | 8 ++++++++ .../tests/integration/test_fileset_catalog.py | 2 +- .../tests/integration/test_metalake.py | 8 ++++++++ .../dto/requests/CatalogUpdateRequest.java | 12 ++---------- .../dto/requests/FilesetUpdateRequest.java | 14 +++----------- .../dto/requests/MetalakeUpdateRequest.java | 12 ++---------- .../dto/requests/TableUpdateRequest.java | 12 ++---------- .../gravitino/dto/requests/TagUpdateRequest.java | 6 ++---- .../dto/requests/TopicUpdateRequest.java | 12 ++---------- .../catalog/TestFilesetOperationDispatcher.java | 2 +- docs/manage-fileset-metadata-using-gravitino.md | 14 +++++++------- .../server/web/rest/TestFilesetOperations.java | 4 ++-- 25 files changed, 94 insertions(+), 92 deletions(-) diff --git a/api/src/main/java/org/apache/gravitino/file/FilesetChange.java b/api/src/main/java/org/apache/gravitino/file/FilesetChange.java index 2df992ece5b..6b79aed41ac 100644 --- a/api/src/main/java/org/apache/gravitino/file/FilesetChange.java +++ b/api/src/main/java/org/apache/gravitino/file/FilesetChange.java @@ -73,7 +73,9 @@ static FilesetChange removeProperty(String property) { * Creates a new fileset change to remove comment from the fileset. * * @return The fileset change. + * @deprecated Use {@link #updateComment(String)} with null value as the argument instead. */ + @Deprecated static FilesetChange removeComment() { return RemoveComment.getInstance(); } @@ -310,7 +312,10 @@ public String toString() { } } - /** A fileset change to remove comment from the fileset. */ + /** + * A fileset change to remove comment from the fileset. Use {@link UpdateFilesetComment} with null + * value as the argument instead. + */ final class RemoveComment implements FilesetChange { private static final RemoveComment INSTANCE = new RemoveComment(); diff --git a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java index 2b89180a8d1..9b5b61f27b0 100644 --- a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java +++ b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java @@ -770,7 +770,7 @@ public void testRemoveFilesetComment() throws IOException { String name = "fileset27"; Fileset fileset = createFileset(name, schemaName, comment, Fileset.Type.MANAGED, null, null); - FilesetChange change1 = FilesetChange.removeComment(); + FilesetChange change1 = FilesetChange.updateComment(null); try (SecureHadoopCatalogOperations ops = new SecureHadoopCatalogOperations(store)) { ops.initialize(Maps.newHashMap(), randomCatalogInfo(), HADOOP_PROPERTIES_METADATA); NameIdentifier filesetIdent = NameIdentifier.of("m1", "c1", schemaName, name); diff --git a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java index b272bd7a889..6cd10cbf24e 100644 --- a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java +++ b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java @@ -575,7 +575,7 @@ public void testFilesetRemoveComment() throws IOException { catalog .asFilesetCatalog() .alterFileset( - NameIdentifier.of(schemaName, filesetName), FilesetChange.removeComment()); + NameIdentifier.of(schemaName, filesetName), FilesetChange.updateComment(null)); assertFilesetExists(filesetName); // verify fileset is updated diff --git a/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java b/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java index dc91a3dda57..52ebb8dab0d 100644 --- a/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java +++ b/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java @@ -172,6 +172,10 @@ public void testCatalog() throws ExecutionException, InterruptedException { Assertions.assertEquals("new comment", alteredCatalog.comment()); Assertions.assertFalse(alteredCatalog.properties().containsKey("key1")); + Catalog updateNullComment = + metalake.alterCatalog(catalogName, CatalogChange.updateComment(null)); + Assertions.assertNull(updateNullComment.comment()); + // test drop catalog boolean dropped = metalake.dropCatalog(catalogName, true); Assertions.assertTrue(dropped); diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java b/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java index 446af40ed9b..f45adfab5f6 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java @@ -378,7 +378,7 @@ public void testAlterFileset() throws JsonProcessingException { assertFileset(mockFileset3, res3); // Test remove fileset comment - FilesetUpdateRequest req4 = new FilesetUpdateRequest.RemoveFilesetCommentRequest(); + FilesetUpdateRequest req4 = new FilesetUpdateRequest.UpdateFilesetCommentRequest(null); FilesetDTO mockFileset4 = mockFilesetDTO("new name", Fileset.Type.MANAGED, null, "mock location", ImmutableMap.of()); FilesetResponse resp4 = new FilesetResponse(mockFileset4); diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/CatalogIT.java b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/CatalogIT.java index 045c0ad694f..bb6394b6c72 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/CatalogIT.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/CatalogIT.java @@ -221,7 +221,7 @@ public void testCatalogAvailable() { CatalogNotInUseException.class, () -> filesetOps.alterFileset( - NameIdentifier.of("dummy", "dummy"), FilesetChange.removeComment())); + NameIdentifier.of("dummy", "dummy"), FilesetChange.updateComment(null))); Assertions.assertTrue(metalake.dropCatalog(catalogName), "catalog should be dropped"); Assertions.assertFalse(metalake.dropCatalog(catalogName), "catalog should be non-existent"); @@ -399,6 +399,10 @@ void testUpdateCatalogWithNullableComment() { metalake.alterCatalog(catalogName, CatalogChange.updateComment("new catalog comment")); Assertions.assertEquals("new catalog comment", updatedCatalog.comment()); + Catalog updateNullComment = + metalake.alterCatalog(catalogName, CatalogChange.updateComment(null)); + Assertions.assertNull(updateNullComment.comment()); + metalake.disableCatalog(catalogName); metalake.dropCatalog(catalogName); } diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/MetalakeIT.java b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/MetalakeIT.java index 3a650646416..7911f02cdf9 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/MetalakeIT.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/MetalakeIT.java @@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -219,6 +220,11 @@ public void testUpdateMetalakeWithNullableComment() { new MetalakeChange[] {MetalakeChange.updateComment("new metalake comment")}; GravitinoMetalake updatedMetalake = client.alterMetalake(metalakeNameA, changes); assertEquals("new metalake comment", updatedMetalake.comment()); + + GravitinoMetalake updateNullComment = + client.alterMetalake(metalakeNameA, MetalakeChange.updateComment(null)); + assertNull(updateNullComment.comment()); + assertTrue(client.dropMetalake(metalakeNameA, true)); } @@ -317,7 +323,7 @@ public void testMetalakeAvailable() { MetalakeNotInUseException.class, () -> filesetOps.alterFileset( - NameIdentifier.of("dummy", "dummy"), FilesetChange.removeComment())); + NameIdentifier.of("dummy", "dummy"), FilesetChange.updateComment(null))); Assertions.assertThrows( NonEmptyMetalakeException.class, () -> client.dropMetalake(metalakeName)); diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/TagIT.java b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/TagIT.java index bd95f2ae34e..4278d3f8861 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/TagIT.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/TagIT.java @@ -197,6 +197,14 @@ public void testCreateGetAndListTag() { Assertions.assertEquals(tag2, tag3); } + @Test + public void testNullableComment() { + String tagName = GravitinoITUtils.genRandomName("tag_it_tag"); + metalake.createTag(tagName, "comment", Collections.emptyMap()); + Tag alteredTag6 = metalake.alterTag(tagName, TagChange.updateComment(null)); + Assertions.assertNull(alteredTag6.comment()); + } + @Test public void testCreateAndAlterTag() { String tagName = GravitinoITUtils.genRandomName("tag_it_tag"); diff --git a/clients/client-python/gravitino/api/fileset_change.py b/clients/client-python/gravitino/api/fileset_change.py index 2be9bc56804..b723a8a29cc 100644 --- a/clients/client-python/gravitino/api/fileset_change.py +++ b/clients/client-python/gravitino/api/fileset_change.py @@ -81,8 +81,11 @@ def remove_comment(): Returns: The fileset change. + + Deprecated: + Please use `update_comment(str)` with null value as the argument instead. """ - return FilesetChange.RemoveComment() + return FilesetChange.UpdateFilesetComment(None) @dataclass class RenameFileset: @@ -279,7 +282,11 @@ def __str__(self): @dataclass class RemoveComment: - """A fileset change to remove comment from the fileset.""" + """A fileset change to remove comment from the fileset. + + Deprecated: + Please use `UpdateFilesetComment(str)` with null value as the argument instead. + """ def __eq__(self, other) -> bool: """Compares this RemoveComment instance with another object for equality. diff --git a/clients/client-python/gravitino/catalog/fileset_catalog.py b/clients/client-python/gravitino/catalog/fileset_catalog.py index cf91dbdfb5c..ffa252e621e 100644 --- a/clients/client-python/gravitino/catalog/fileset_catalog.py +++ b/clients/client-python/gravitino/catalog/fileset_catalog.py @@ -319,5 +319,5 @@ def to_fileset_update_request(change: FilesetChange): if isinstance(change, FilesetChange.RemoveProperty): return FilesetUpdateRequest.RemoveFilesetPropertyRequest(change.property()) if isinstance(change, FilesetChange.RemoveComment): - return FilesetUpdateRequest.RemoveFilesetCommentRequest() + return FilesetUpdateRequest.UpdateFilesetCommentRequest(None) raise ValueError(f"Unknown change type: {type(change).__name__}") diff --git a/clients/client-python/gravitino/dto/requests/catalog_update_request.py b/clients/client-python/gravitino/dto/requests/catalog_update_request.py index 1164db2990b..1ea4d9953c0 100644 --- a/clients/client-python/gravitino/dto/requests/catalog_update_request.py +++ b/clients/client-python/gravitino/dto/requests/catalog_update_request.py @@ -78,8 +78,8 @@ def catalog_change(self): return CatalogChange.update_comment(self._new_comment) def validate(self): - if not self._new_comment: - raise ValueError('"newComment" field is required and cannot be empty') + """Validates the fields of the request. Always pass.""" + pass @dataclass class SetCatalogPropertyRequest(CatalogUpdateRequestBase): diff --git a/clients/client-python/gravitino/dto/requests/fileset_update_request.py b/clients/client-python/gravitino/dto/requests/fileset_update_request.py index 66e001bee9c..9a640d2071d 100644 --- a/clients/client-python/gravitino/dto/requests/fileset_update_request.py +++ b/clients/client-python/gravitino/dto/requests/fileset_update_request.py @@ -79,13 +79,8 @@ def __init__(self, new_comment: str): self._new_comment = new_comment def validate(self): - """Validates the fields of the request. - - Raises: - IllegalArgumentException if the new comment is not set. - """ - if not self._new_comment: - raise ValueError('"new_comment" field is required and cannot be empty') + """Validates the fields of the request. Always pass.""" + pass def fileset_change(self): """Returns the fileset change""" @@ -149,7 +144,11 @@ def fileset_change(self): @dataclass class RemoveFilesetCommentRequest(FilesetUpdateRequestBase): - """Represents a request to remove comment from a Fileset.""" + """Represents a request to remove comment from a Fileset. + + Deprecated: + Please use `UpdateFilesetCommentRequest` with null value as the argument instead. + """ def __init__(self): super().__init__("removeComment") diff --git a/clients/client-python/gravitino/dto/requests/metalake_update_request.py b/clients/client-python/gravitino/dto/requests/metalake_update_request.py index 8e54fe360e1..79551e5dc0d 100644 --- a/clients/client-python/gravitino/dto/requests/metalake_update_request.py +++ b/clients/client-python/gravitino/dto/requests/metalake_update_request.py @@ -74,13 +74,8 @@ def __init__(self, new_comment: str): self._new_comment = new_comment def validate(self): - """Validates the fields of the request. - - Raises: - IllegalArgumentException if the new comment is not set. - """ - if not self._new_comment: - raise ValueError('"newComment" field is required and cannot be empty') + """Validates the fields of the request. Always pass.""" + pass def metalake_change(self): return MetalakeChange.update_comment(self._new_comment) diff --git a/clients/client-python/tests/integration/test_catalog.py b/clients/client-python/tests/integration/test_catalog.py index 64208315e6e..bd7933e00ae 100644 --- a/clients/client-python/tests/integration/test_catalog.py +++ b/clients/client-python/tests/integration/test_catalog.py @@ -125,6 +125,14 @@ def test_failed_create_catalog(self): with self.assertRaises(CatalogAlreadyExistsException): _ = self.create_catalog(self.catalog_name) + def test_nullable_comment_catalog(self): + self.create_catalog(self.catalog_name) + changes = (CatalogChange.update_comment(None),) + null_comment_catalog = self.gravitino_client.alter_catalog( + self.catalog_name, *changes + ) + self.assertIsNone(null_comment_catalog.comment()) + def test_alter_catalog(self): catalog = self.create_catalog(self.catalog_name) diff --git a/clients/client-python/tests/integration/test_fileset_catalog.py b/clients/client-python/tests/integration/test_fileset_catalog.py index 754735b16e8..ac3d0a82164 100644 --- a/clients/client-python/tests/integration/test_fileset_catalog.py +++ b/clients/client-python/tests/integration/test_fileset_catalog.py @@ -239,7 +239,7 @@ def test_alter_fileset(self): self.assertEqual(fileset_new.comment(), fileset_new_comment) fileset_comment_removed = catalog.as_fileset_catalog().alter_fileset( - self.fileset_ident, FilesetChange.remove_comment() + self.fileset_ident, FilesetChange.update_comment(None) ) self.assertEqual(fileset_comment_removed.name(), self.fileset_name) self.assertIsNone(fileset_comment_removed.comment()) diff --git a/clients/client-python/tests/integration/test_metalake.py b/clients/client-python/tests/integration/test_metalake.py index 75d3a06f26c..f2b14b67877 100644 --- a/clients/client-python/tests/integration/test_metalake.py +++ b/clients/client-python/tests/integration/test_metalake.py @@ -90,6 +90,14 @@ def test_failed_create_metalake(self): with self.assertRaises(MetalakeAlreadyExistsException): _ = self.create_metalake(self.metalake_name) + def test_nullable_comment_metalake(self): + self.create_metalake(self.metalake_name) + changes = (MetalakeChange.update_comment(None),) + null_comment_metalake = self.gravitino_admin_client.alter_metalake( + self.metalake_name, *changes + ) + self.assertIsNone(null_comment_metalake.comment()) + def test_alter_metalake(self): self.create_metalake(self.metalake_name) diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java index 9dfbb4efd2f..aabdf2901fa 100644 --- a/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java +++ b/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java @@ -117,17 +117,9 @@ public UpdateCatalogCommentRequest() { this(null); } - /** - * Validates the fields of the request. - * - * @throws IllegalArgumentException if the new comment is not set. - */ + /** Validates the fields of the request. Always pass. */ @Override - public void validate() throws IllegalArgumentException { - Preconditions.checkArgument( - StringUtils.isNotBlank(newComment), - "\"newComment\" field is required and cannot be empty"); - } + public void validate() throws IllegalArgumentException {} @Override public CatalogChange catalogChange() { diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java index c7aa79cb800..82915dcdc19 100644 --- a/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java +++ b/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java @@ -109,17 +109,9 @@ public FilesetChange filesetChange() { return FilesetChange.updateComment(newComment); } - /** - * Validates the request. - * - * @throws IllegalArgumentException if the request is invalid. - */ + /** Validates the fields of the request. Always pass. */ @Override - public void validate() throws IllegalArgumentException { - Preconditions.checkArgument( - StringUtils.isNotBlank(newComment), - "\"newComment\" field is required and cannot be empty"); - } + public void validate() throws IllegalArgumentException {} } /** The fileset update request for setting the properties of a fileset. */ @@ -194,7 +186,7 @@ class RemoveFilesetCommentRequest implements FilesetUpdateRequest { /** @return The fileset change. */ @Override public FilesetChange filesetChange() { - return FilesetChange.removeComment(); + return FilesetChange.updateComment(null); } /** diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/MetalakeUpdateRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/MetalakeUpdateRequest.java index a5b747aff77..6e01ace8052 100644 --- a/common/src/main/java/org/apache/gravitino/dto/requests/MetalakeUpdateRequest.java +++ b/common/src/main/java/org/apache/gravitino/dto/requests/MetalakeUpdateRequest.java @@ -117,17 +117,9 @@ public UpdateMetalakeCommentRequest() { this(null); } - /** - * Validates the fields of the request. - * - * @throws IllegalArgumentException if the new comment is not set. - */ + /** Validates the fields of the request. Always pass. */ @Override - public void validate() throws IllegalArgumentException { - Preconditions.checkArgument( - StringUtils.isNotBlank(newComment), - "\"newComment\" field is required and cannot be empty"); - } + public void validate() throws IllegalArgumentException {} @Override public MetalakeChange metalakeChange() { diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java index 936597f4dad..db1702ce6de 100644 --- a/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java +++ b/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java @@ -164,17 +164,9 @@ public UpdateTableCommentRequest() { this(null); } - /** - * Validates the request. - * - * @throws IllegalArgumentException If the request is invalid, this exception is thrown. - */ + /** Validates the fields of the request. Always pass. */ @Override - public void validate() throws IllegalArgumentException { - Preconditions.checkArgument( - StringUtils.isNotBlank(newComment), - "\"newComment\" field is required and cannot be empty"); - } + public void validate() throws IllegalArgumentException {} /** * Returns the table change. diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/TagUpdateRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/TagUpdateRequest.java index 5323ac56548..f27d8359e71 100644 --- a/common/src/main/java/org/apache/gravitino/dto/requests/TagUpdateRequest.java +++ b/common/src/main/java/org/apache/gravitino/dto/requests/TagUpdateRequest.java @@ -114,11 +114,9 @@ public TagChange tagChange() { return TagChange.updateComment(newComment); } + /** Validates the fields of the request. Always pass. */ @Override - public void validate() throws IllegalArgumentException { - Preconditions.checkArgument( - StringUtils.isNotBlank(newComment), "\"newComment\" must not be blank"); - } + public void validate() throws IllegalArgumentException {} } /** The tag update request for setting a tag property. */ diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/TopicUpdateRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/TopicUpdateRequest.java index 67103b2e5df..add5ae7e99b 100644 --- a/common/src/main/java/org/apache/gravitino/dto/requests/TopicUpdateRequest.java +++ b/common/src/main/java/org/apache/gravitino/dto/requests/TopicUpdateRequest.java @@ -72,17 +72,9 @@ public UpdateTopicCommentRequest() { this(null); } - /** - * Validates the request. - * - * @throws IllegalArgumentException If the request is invalid, this exception is thrown. - */ + /** Validates the fields of the request. Always pass. */ @Override - public void validate() throws IllegalArgumentException { - Preconditions.checkArgument( - StringUtils.isNotBlank(newComment), - "\"newComment\" field is required and cannot be empty"); - } + public void validate() throws IllegalArgumentException {} /** * Returns the topic change. diff --git a/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java b/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java index 7ceed9e2e17..4fa3cecbb3d 100644 --- a/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java +++ b/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java @@ -143,7 +143,7 @@ public void testCreateAndAlterFileset() { Assertions.assertEquals(fileset1.name(), alteredFileset2.name()); Assertions.assertEquals("new comment", alteredFileset2.comment()); - FilesetChange[] changes3 = new FilesetChange[] {FilesetChange.removeComment()}; + FilesetChange[] changes3 = new FilesetChange[] {FilesetChange.updateComment(null)}; Fileset alteredFileset3 = filesetOperationDispatcher.alterFileset(filesetIdent1, changes3); Assertions.assertEquals(fileset1.name(), alteredFileset3.name()); diff --git a/docs/manage-fileset-metadata-using-gravitino.md b/docs/manage-fileset-metadata-using-gravitino.md index de478efb731..fe1d3304070 100644 --- a/docs/manage-fileset-metadata-using-gravitino.md +++ b/docs/manage-fileset-metadata-using-gravitino.md @@ -389,13 +389,13 @@ fileset_new = catalog.as_fileset_catalog().alter_fileset(NameIdentifier.of("sche Currently, Gravitino supports the following changes to a fileset: -| Supported modification | JSON | Java | -|----------------------------|--------------------------------------------------------------|-----------------------------------------------| -| Rename a fileset | `{"@type":"rename","newName":"fileset_renamed"}` | `FilesetChange.rename("fileset_renamed")` | -| Update a comment | `{"@type":"updateComment","newComment":"new_comment"}` | `FilesetChange.updateComment("new_comment")` | -| Set a fileset property | `{"@type":"setProperty","property":"key1","value":"value1"}` | `FilesetChange.setProperty("key1", "value1")` | -| Remove a fileset property | `{"@type":"removeProperty","property":"key1"}` | `FilesetChange.removeProperty("key1")` | -| Remove comment | `{"@type":"removeComment"}` | `FilesetChange.removeComment()` | +| Supported modification | JSON | Java | +|-----------------------------|--------------------------------------------------------------|-----------------------------------------------| +| Rename a fileset | `{"@type":"rename","newName":"fileset_renamed"}` | `FilesetChange.rename("fileset_renamed")` | +| Update a comment | `{"@type":"updateComment","newComment":"new_comment"}` | `FilesetChange.updateComment("new_comment")` | +| Set a fileset property | `{"@type":"setProperty","property":"key1","value":"value1"}` | `FilesetChange.setProperty("key1", "value1")` | +| Remove a fileset property | `{"@type":"removeProperty","property":"key1"}` | `FilesetChange.removeProperty("key1")` | +| Remove comment (deprecated) | `{"@type":"removeComment"}` | `FilesetChange.removeComment()` | ### Drop a fileset diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java index 62375dc4b3e..4258346e49e 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java @@ -370,7 +370,7 @@ public void testUpdateFilesetComment() { @Test public void testRemoveFilesetComment() { - FilesetUpdateRequest req = new FilesetUpdateRequest.RemoveFilesetCommentRequest(); + FilesetUpdateRequest req = new FilesetUpdateRequest.UpdateFilesetCommentRequest(null); Fileset fileset = mockFileset("fileset1", Fileset.Type.MANAGED, null, "mock location", ImmutableMap.of()); assertUpdateFileset(new FilesetUpdatesRequest(ImmutableList.of(req)), fileset); @@ -387,7 +387,7 @@ public void testMultiUpdateRequest() { // remove k2 FilesetUpdateRequest req5 = new FilesetUpdateRequest.RemoveFilesetPropertiesRequest("k2"); // remove comment - FilesetUpdateRequest req6 = new FilesetUpdateRequest.RemoveFilesetCommentRequest(); + FilesetUpdateRequest req6 = new FilesetUpdateRequest.UpdateFilesetCommentRequest(null); Fileset fileset = mockFileset(