From e5b3eb9d9d9894d574f5a282899b166e15e9a994 Mon Sep 17 00:00:00 2001 From: Shaofeng Shi Date: Fri, 28 Jun 2024 06:56:54 -0700 Subject: [PATCH] [#3699] refactor(API): Refactor client side FilesetCatalog to use relative path in NameIdentifier (#3802) ### What changes were proposed in this pull request? Currently, in the FilesetCatalog.java, the methods like "loadFileset", "createFileset", "updateFileset" all need a NameIdentifier parameter, which needs to be a fully-qualified (metalake.catalog.schema.fileset) name. But the "metalake" and "catalog" are not needed, as they already be provided when load the catalog. To make the API clear and easier to use, we will change it to use a relative NameIdentifier object (which is "schema.fileset") as the fileset's ID, so that the user doesn't need to provide the metalake and catalog names repeatedly. Please note, this only affects the client side. ### Why are the changes needed? To make the API simple and easy to understand. Fix: #3699 ### Does this PR introduce _any_ user-facing change? No behavior change, just method parameter. ### How was this patch tested? No introduce new class or method, so the change will be covered by all existing test cases. --------- Co-authored-by: qqqttt123 <148952220+qqqttt123@users.noreply.github.com> Co-authored-by: Heng Qin Co-authored-by: Liang Chun Co-authored-by: Lance Lin <103924768+LanceHsun@users.noreply.github.com> Co-authored-by: LanceLin Co-authored-by: Rory Co-authored-by: mchades Co-authored-by: Yuhui Co-authored-by: Qi Yu Co-authored-by: Junshen Tao <89704044+jtao1@users.noreply.github.com> Co-authored-by: noidname01 <55401762+noidname01@users.noreply.github.com> Co-authored-by: TimWang Co-authored-by: Ikko Eltociear Ashimine Co-authored-by: cai can <94670132+caican00@users.noreply.github.com> Co-authored-by: XiaoZ <57973980+xiaozcy@users.noreply.github.com> Co-authored-by: zhanghan18 Co-authored-by: rich7420 <101171023+rich7420@users.noreply.github.com> Co-authored-by: hanwxx <160711189+hanwxx@users.noreply.github.com> Co-authored-by: Eric Chang Co-authored-by: Jerry Shao Co-authored-by: FANNG Co-authored-by: Ziva Li Co-authored-by: xloya <982052490@qq.com> Co-authored-by: xiaojiebao Co-authored-by: caican SteNicholas Co-authored-by: Kang Co-authored-by: Xun Liu Co-authored-by: Qian Xia Co-authored-by: caican --- .../integration/test/HadoopCatalogIT.java | 59 +++++------------- .../test/HadoopUserAuthenticationIT.java | 6 +- .../test/HadoopUserImpersonationIT.java | 6 +- .../gravitino/client/FilesetCatalog.java | 61 ++++++++++++------- .../gravitino/client/TestFilesetCatalog.java | 46 ++++++++++---- .../hadoop/GravitinoVirtualFileSystem.java | 4 +- .../hadoop/GravitinoVirtualFileSystemIT.java | 21 +++---- .../test/web/ui/CatalogsPageTest.java | 2 +- 8 files changed, 102 insertions(+), 103 deletions(-) diff --git a/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java b/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java index 5cd33d8f959..3731625caa2 100644 --- a/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java +++ b/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java @@ -6,6 +6,7 @@ import com.datastrato.gravitino.Catalog; import com.datastrato.gravitino.NameIdentifier; +import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.Schema; import com.datastrato.gravitino.client.GravitinoMetalake; import com.datastrato.gravitino.exceptions.FilesetAlreadyExistsException; @@ -16,7 +17,6 @@ import com.datastrato.gravitino.integration.test.container.HiveContainer; import com.datastrato.gravitino.integration.test.util.AbstractIT; import com.datastrato.gravitino.integration.test.util.GravitinoITUtils; -import com.datastrato.gravitino.utils.NamespaceUtil; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import java.io.IOException; @@ -258,8 +258,7 @@ public void testExternalFileset() throws IOException { void testNameSpec() { String illegalName = "/%~?*"; - NameIdentifier nameIdentifier = - NameIdentifier.of(metalakeName, catalogName, schemaName, illegalName); + NameIdentifier nameIdentifier = NameIdentifier.of(schemaName, illegalName); Assertions.assertThrows( NoSuchFilesetException.class, () -> catalog.asFilesetCatalog().loadFileset(nameIdentifier)); @@ -282,9 +281,7 @@ public void testLoadFileset() throws IOException { // test load fileset Fileset loadFileset = - catalog - .asFilesetCatalog() - .loadFileset(NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName)); + catalog.asFilesetCatalog().loadFileset(NameIdentifier.of(schemaName, filesetName)); Assertions.assertEquals(fileset.name(), loadFileset.name(), "fileset should be loaded"); Assertions.assertEquals(fileset.comment(), loadFileset.comment(), "comment should be loaded"); Assertions.assertEquals(fileset.type(), loadFileset.type(), "type should be loaded"); @@ -298,10 +295,7 @@ public void testLoadFileset() throws IOException { // test load a fileset that not exist Assertions.assertThrows( NoSuchFilesetException.class, - () -> - catalog - .asFilesetCatalog() - .loadFileset(NameIdentifier.of(metalakeName, catalogName, schemaName, "not_exist")), + () -> catalog.asFilesetCatalog().loadFileset(NameIdentifier.of(schemaName, "not_exist")), "Should throw NoSuchFilesetException when fileset does not exist"); } @@ -320,16 +314,12 @@ public void testDropManagedFileset() throws IOException { // drop fileset boolean dropped = - catalog - .asFilesetCatalog() - .dropFileset(NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName)); + catalog.asFilesetCatalog().dropFileset(NameIdentifier.of(schemaName, filesetName)); Assertions.assertTrue(dropped, "fileset should be dropped"); // verify fileset is dropped Assertions.assertFalse( - catalog - .asFilesetCatalog() - .filesetExists(NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName)), + catalog.asFilesetCatalog().filesetExists(NameIdentifier.of(schemaName, filesetName)), "fileset should not be exists"); Assertions.assertFalse( hdfs.exists(new Path(storageLocation)), "storage location should be dropped"); @@ -351,16 +341,12 @@ public void testDropExternalFileset() throws IOException { // drop fileset boolean dropped = - catalog - .asFilesetCatalog() - .dropFileset(NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName)); + catalog.asFilesetCatalog().dropFileset(NameIdentifier.of(schemaName, filesetName)); Assertions.assertTrue(dropped, "fileset should be dropped"); // verify fileset is dropped Assertions.assertFalse( - catalog - .asFilesetCatalog() - .filesetExists(NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName)), + catalog.asFilesetCatalog().filesetExists(NameIdentifier.of(schemaName, filesetName)), "fileset should not be exists"); Assertions.assertTrue( hdfs.exists(new Path(storageLocation)), "storage location should not be dropped"); @@ -374,9 +360,7 @@ public void testListFilesets() throws IOException { // test no fileset exists NameIdentifier[] nameIdentifiers = - catalog - .asFilesetCatalog() - .listFilesets(NamespaceUtil.ofFileset(metalakeName, catalogName, schemaName)); + catalog.asFilesetCatalog().listFilesets(Namespace.of(schemaName)); Assertions.assertEquals(0, nameIdentifiers.length, "should have no fileset"); // create fileset1 @@ -407,9 +391,7 @@ public void testListFilesets() throws IOException { // list filesets NameIdentifier[] nameIdentifiers1 = - catalog - .asFilesetCatalog() - .listFilesets(NamespaceUtil.ofFileset(metalakeName, catalogName, schemaName)); + catalog.asFilesetCatalog().listFilesets(Namespace.of(schemaName)); Arrays.sort(nameIdentifiers1, Comparator.comparing(NameIdentifier::name)); Assertions.assertEquals(2, nameIdentifiers1.length, "should have 2 filesets"); Assertions.assertEquals(fileset1.name(), nameIdentifiers1[0].name()); @@ -432,8 +414,7 @@ public void testRenameFileset() throws IOException { catalog .asFilesetCatalog() .alterFileset( - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName), - FilesetChange.rename(newFilesetName)); + NameIdentifier.of(schemaName, filesetName), FilesetChange.rename(newFilesetName)); // verify fileset is updated Assertions.assertNotNull(newFileset, "fileset should be created"); @@ -463,7 +444,7 @@ public void testFilesetUpdateComment() throws IOException { catalog .asFilesetCatalog() .alterFileset( - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName), + NameIdentifier.of(schemaName, filesetName), FilesetChange.updateComment(newComment)); assertFilesetExists(filesetName); @@ -493,8 +474,7 @@ public void testFilesetSetProperties() throws IOException { catalog .asFilesetCatalog() .alterFileset( - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName), - FilesetChange.setProperty("k1", "v2")); + NameIdentifier.of(schemaName, filesetName), FilesetChange.setProperty("k1", "v2")); assertFilesetExists(filesetName); // verify fileset is updated @@ -523,8 +503,7 @@ public void testFilesetRemoveProperties() throws IOException { catalog .asFilesetCatalog() .alterFileset( - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName), - FilesetChange.removeProperty("k1")); + NameIdentifier.of(schemaName, filesetName), FilesetChange.removeProperty("k1")); assertFilesetExists(filesetName); // verify fileset is updated @@ -580,18 +559,12 @@ private Fileset createFileset( return catalog .asFilesetCatalog() .createFileset( - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName), - comment, - type, - storageLocation, - properties); + NameIdentifier.of(schemaName, filesetName), comment, type, storageLocation, properties); } private void assertFilesetExists(String filesetName) throws IOException { Assertions.assertTrue( - catalog - .asFilesetCatalog() - .filesetExists(NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName)), + catalog.asFilesetCatalog().filesetExists(NameIdentifier.of(schemaName, filesetName)), "fileset should be exists"); Assertions.assertTrue( hdfs.exists(new Path(storageLocation(filesetName))), "storage location should be exists"); diff --git a/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopUserAuthenticationIT.java b/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopUserAuthenticationIT.java index 4cf3399f32b..e138e52ba5a 100644 --- a/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopUserAuthenticationIT.java +++ b/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopUserAuthenticationIT.java @@ -197,15 +197,13 @@ public void testUserAuthentication() { catalog .asFilesetCatalog() .createFileset( - NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME, TABLE_NAME), + NameIdentifier.of(SCHEMA_NAME, TABLE_NAME), "comment", Fileset.Type.MANAGED, null, ImmutableMap.of()); - catalog - .asFilesetCatalog() - .dropFileset(NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME, TABLE_NAME)); + catalog.asFilesetCatalog().dropFileset(NameIdentifier.of(SCHEMA_NAME, TABLE_NAME)); catalog.asSchemas().alterSchema(SCHEMA_NAME, SchemaChange.setProperty("k1", "value1")); diff --git a/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopUserImpersonationIT.java b/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopUserImpersonationIT.java index 951f84758f8..538c0ba8df9 100644 --- a/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopUserImpersonationIT.java +++ b/catalogs/catalog-hadoop/src/test/java/com/datastrato/gravitino/catalog/hadoop/integration/test/HadoopUserImpersonationIT.java @@ -293,11 +293,7 @@ private Fileset createFileset( return catalog .asFilesetCatalog() .createFileset( - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName), - comment, - type, - storageLocation, - properties); + NameIdentifier.of(schemaName, filesetName), comment, type, storageLocation, properties); } private boolean checkFilePathExists(String pathString) throws Exception { diff --git a/clients/client-java/src/main/java/com/datastrato/gravitino/client/FilesetCatalog.java b/clients/client-java/src/main/java/com/datastrato/gravitino/client/FilesetCatalog.java index ad2dbf99777..69f8626df57 100644 --- a/clients/client-java/src/main/java/com/datastrato/gravitino/client/FilesetCatalog.java +++ b/clients/client-java/src/main/java/com/datastrato/gravitino/client/FilesetCatalog.java @@ -59,27 +59,32 @@ public com.datastrato.gravitino.file.FilesetCatalog asFilesetCatalog() /** * List the filesets in a schema namespace from the catalog. * - * @param namespace A schema namespace. - * @return An array of fileset identifiers in the namespace. + * @param namespace A schema namespace. This namespace should have 1 level, which is the schema + * name; + * @return An array of {@link NameIdentifier} of filesets under the given namespace. * @throws NoSuchSchemaException If the schema does not exist. */ @Override public NameIdentifier[] listFilesets(Namespace namespace) throws NoSuchSchemaException { checkFilesetNamespace(namespace); + Namespace fullNamespace = getFilesetFullNamespace(namespace); EntityListResponse resp = restClient.get( - formatFilesetRequestPath(namespace), + formatFilesetRequestPath(fullNamespace), EntityListResponse.class, Collections.emptyMap(), ErrorHandlers.filesetErrorHandler()); resp.validate(); - return resp.identifiers(); + return Arrays.stream(resp.identifiers()) + .map(ident -> NameIdentifier.of(ident.namespace().level(2), ident.name())) + .toArray(NameIdentifier[]::new); } /** - * Load fileset metadata by {@link NameIdentifier} from the catalog. + * Load fileset metadata by {@link NameIdentifier} from the catalog, which should be a + * "schema.fileset" style. * * @param ident A fileset identifier. * @return The fileset metadata. @@ -87,12 +92,12 @@ public NameIdentifier[] listFilesets(Namespace namespace) throws NoSuchSchemaExc */ @Override public Fileset loadFileset(NameIdentifier ident) throws NoSuchFilesetException { - checkFilesetNameIdentifer(ident); + checkFilesetNameIdentifier(ident); + + Namespace fullNamespace = getFilesetFullNamespace(ident.namespace()); FilesetResponse resp = restClient.get( - formatFilesetRequestPath(ident.namespace()) - + "/" - + RESTUtils.encodeString(ident.name()), + formatFilesetRequestPath(fullNamespace) + "/" + RESTUtils.encodeString(ident.name()), FilesetResponse.class, Collections.emptyMap(), ErrorHandlers.filesetErrorHandler()); @@ -109,7 +114,7 @@ public Fileset loadFileset(NameIdentifier ident) throws NoSuchFilesetException { * *

If the type of the fileset object is "EXTERNAL", the underlying storageLocation must be set. * - * @param ident A fileset identifier. + * @param ident A fileset identifier, which should be a "schema.fileset" style. * @param comment The comment of the fileset. * @param type The type of the fileset. * @param storageLocation The storage location of the fileset. @@ -126,7 +131,9 @@ public Fileset createFileset( String storageLocation, Map properties) throws NoSuchSchemaException, FilesetAlreadyExistsException { - checkFilesetNameIdentifer(ident); + checkFilesetNameIdentifier(ident); + + Namespace fullNamespace = getFilesetFullNamespace(ident.namespace()); FilesetCreateRequest req = FilesetCreateRequest.builder() .name(RESTUtils.encodeString(ident.name())) @@ -138,7 +145,7 @@ public Fileset createFileset( FilesetResponse resp = restClient.post( - formatFilesetRequestPath(ident.namespace()), + formatFilesetRequestPath(fullNamespace), req, FilesetResponse.class, Collections.emptyMap(), @@ -151,7 +158,7 @@ public Fileset createFileset( /** * Update a fileset metadata in the catalog. * - * @param ident A fileset identifier. + * @param ident A fileset identifier, which should be a "schema.fileset" style. * @param changes The changes to apply to the fileset. * @return The updated fileset metadata. * @throws NoSuchFilesetException If the fileset does not exist. @@ -160,7 +167,9 @@ public Fileset createFileset( @Override public Fileset alterFileset(NameIdentifier ident, FilesetChange... changes) throws NoSuchFilesetException, IllegalArgumentException { - checkFilesetNameIdentifer(ident); + checkFilesetNameIdentifier(ident); + + Namespace fullNamespace = getFilesetFullNamespace(ident.namespace()); List updates = Arrays.stream(changes) .map(DTOConverters::toFilesetUpdateRequest) @@ -170,7 +179,7 @@ public Fileset alterFileset(NameIdentifier ident, FilesetChange... changes) FilesetResponse resp = restClient.put( - formatFilesetRequestPath(ident.namespace()) + "/" + ident.name(), + formatFilesetRequestPath(fullNamespace) + "/" + ident.name(), req, FilesetResponse.class, Collections.emptyMap(), @@ -186,15 +195,17 @@ public Fileset alterFileset(NameIdentifier ident, FilesetChange... changes) *

The underlying files will be deleted if this fileset type is managed, otherwise, only the * metadata will be dropped. * - * @param ident A fileset identifier. + * @param ident A fileset identifier, which should be a "schema.fileset" style. * @return true If the fileset is dropped, false the fileset did not exist. */ @Override public boolean dropFileset(NameIdentifier ident) { - checkFilesetNameIdentifer(ident); + checkFilesetNameIdentifier(ident); + + Namespace fullNamespace = getFilesetFullNamespace(ident.namespace()); DropResponse resp = restClient.delete( - formatFilesetRequestPath(ident.namespace()) + "/" + ident.name(), + formatFilesetRequestPath(fullNamespace) + "/" + ident.name(), DropResponse.class, Collections.emptyMap(), ErrorHandlers.filesetErrorHandler()); @@ -221,8 +232,8 @@ static String formatFilesetRequestPath(Namespace ns) { */ static void checkFilesetNamespace(Namespace namespace) { Namespace.check( - namespace != null && namespace.length() == 3, - "Fileset namespace must be non-null and have 3 level, the input namespace is %s", + namespace != null && namespace.length() == 1, + "Fileset namespace must be non-null and have 1 level, the input namespace is %s", namespace); } @@ -231,13 +242,17 @@ static void checkFilesetNamespace(Namespace namespace) { * * @param ident The NameIdentifier to check */ - static void checkFilesetNameIdentifer(NameIdentifier ident) { - NameIdentifier.check(ident != null, "NameIdentifer must not be null"); + static void checkFilesetNameIdentifier(NameIdentifier ident) { + NameIdentifier.check(ident != null, "NameIdentifier must not be null"); NameIdentifier.check( - ident.name() != null && !ident.name().isEmpty(), "NameIdentifer name must not be empty"); + ident.name() != null && !ident.name().isEmpty(), "NameIdentifier name must not be empty"); checkFilesetNamespace(ident.namespace()); } + private Namespace getFilesetFullNamespace(Namespace tableNamespace) { + return Namespace.of(this.catalogNamespace().level(0), this.name(), tableNamespace.level(0)); + } + /** * Create a new builder for the fileset catalog. * diff --git a/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestFilesetCatalog.java b/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestFilesetCatalog.java index a82dbfba2c9..5fbdbdf92a7 100644 --- a/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestFilesetCatalog.java +++ b/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestFilesetCatalog.java @@ -11,6 +11,7 @@ import com.datastrato.gravitino.Catalog; import com.datastrato.gravitino.NameIdentifier; +import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.dto.AuditDTO; import com.datastrato.gravitino.dto.CatalogDTO; import com.datastrato.gravitino.dto.file.FilesetDTO; @@ -87,11 +88,20 @@ public static void setUp() throws Exception { @Test public void testListFileset() throws JsonProcessingException { - NameIdentifier fileset1 = NameIdentifier.of(metalakeName, catalogName, "schema1", "fileset1"); - NameIdentifier fileset2 = NameIdentifier.of(metalakeName, catalogName, "schema1", "fileset2"); - String filesetPath = withSlash(FilesetCatalog.formatFilesetRequestPath(fileset1.namespace())); + NameIdentifier fileset1 = NameIdentifier.of("schema1", "fileset1"); + NameIdentifier fileset2 = NameIdentifier.of("schema1", "fileset2"); + NameIdentifier expectedResultFileset1 = + NameIdentifier.of(metalakeName, catalogName, "schema1", "fileset1"); + NameIdentifier expectedResultFileset2 = + NameIdentifier.of(metalakeName, catalogName, "schema1", "fileset2"); + String filesetPath = + withSlash( + FilesetCatalog.formatFilesetRequestPath( + Namespace.of(metalakeName, catalogName, "schema1"))); - EntityListResponse resp = new EntityListResponse(new NameIdentifier[] {fileset1, fileset2}); + EntityListResponse resp = + new EntityListResponse( + new NameIdentifier[] {expectedResultFileset1, expectedResultFileset2}); buildMockResource(Method.GET, filesetPath, null, resp, SC_OK); NameIdentifier[] filesets = catalog.asFilesetCatalog().listFilesets(fileset1.namespace()); @@ -128,9 +138,12 @@ public void testListFileset() throws JsonProcessingException { @Test public void testLoadFileset() throws JsonProcessingException { - NameIdentifier fileset = NameIdentifier.of(metalakeName, catalogName, "schema1", "fileset1"); + NameIdentifier fileset = NameIdentifier.of("schema1", "fileset1"); String filesetPath = - withSlash(FilesetCatalog.formatFilesetRequestPath(fileset.namespace()) + "/fileset1"); + withSlash( + FilesetCatalog.formatFilesetRequestPath( + Namespace.of(metalakeName, catalogName, "schema1")) + + "/fileset1"); FilesetDTO mockFileset = mockFilesetDTO( @@ -172,8 +185,11 @@ public void testLoadFileset() throws JsonProcessingException { @Test public void testCreateFileset() throws JsonProcessingException { - NameIdentifier fileset = NameIdentifier.of(metalakeName, catalogName, "schema1", "fileset1"); - String filesetPath = withSlash(FilesetCatalog.formatFilesetRequestPath(fileset.namespace())); + NameIdentifier fileset = NameIdentifier.of("schema1", "fileset1"); + String filesetPath = + withSlash( + FilesetCatalog.formatFilesetRequestPath( + Namespace.of(metalakeName, catalogName, "schema1"))); FilesetDTO mockFileset = mockFilesetDTO( @@ -241,9 +257,12 @@ public void testCreateFileset() throws JsonProcessingException { @Test public void testDropFileset() throws JsonProcessingException { - NameIdentifier fileset = NameIdentifier.of(metalakeName, catalogName, "schema1", "fileset1"); + NameIdentifier fileset = NameIdentifier.of("schema1", "fileset1"); String filesetPath = - withSlash(FilesetCatalog.formatFilesetRequestPath(fileset.namespace()) + "/fileset1"); + withSlash( + FilesetCatalog.formatFilesetRequestPath( + Namespace.of(metalakeName, catalogName, "schema1")) + + "/fileset1"); DropResponse resp = new DropResponse(true); buildMockResource(Method.DELETE, filesetPath, null, resp, SC_OK); @@ -266,9 +285,12 @@ public void testDropFileset() throws JsonProcessingException { @Test public void testAlterFileset() throws JsonProcessingException { - NameIdentifier fileset = NameIdentifier.of(metalakeName, catalogName, "schema1", "fileset1"); + NameIdentifier fileset = NameIdentifier.of("schema1", "fileset1"); String filesetPath = - withSlash(FilesetCatalog.formatFilesetRequestPath(fileset.namespace()) + "/fileset1"); + withSlash( + FilesetCatalog.formatFilesetRequestPath( + Namespace.of(metalakeName, catalogName, "schema1")) + + "/fileset1"); // Test alter fileset name FilesetUpdateRequest req = new FilesetUpdateRequest.RenameFilesetRequest("new name"); diff --git a/clients/filesystem-hadoop3/src/main/java/com/datastrato/gravitino/filesystem/hadoop/GravitinoVirtualFileSystem.java b/clients/filesystem-hadoop3/src/main/java/com/datastrato/gravitino/filesystem/hadoop/GravitinoVirtualFileSystem.java index 32ec1b97179..733a1126f9b 100644 --- a/clients/filesystem-hadoop3/src/main/java/com/datastrato/gravitino/filesystem/hadoop/GravitinoVirtualFileSystem.java +++ b/clients/filesystem-hadoop3/src/main/java/com/datastrato/gravitino/filesystem/hadoop/GravitinoVirtualFileSystem.java @@ -378,7 +378,9 @@ private Pair constructNewFilesetPair(NameIdentifier identif private Fileset loadFileset(NameIdentifier identifier) { Catalog catalog = client.loadCatalog(identifier.namespace().level(1)); - return catalog.asFilesetCatalog().loadFileset(identifier); + return catalog + .asFilesetCatalog() + .loadFileset(NameIdentifier.of(identifier.namespace().level(2), identifier.name())); } @Override diff --git a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/client/filesystem/hadoop/GravitinoVirtualFileSystemIT.java b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/client/filesystem/hadoop/GravitinoVirtualFileSystemIT.java index 9e1ef2ad149..16f7eda0457 100644 --- a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/client/filesystem/hadoop/GravitinoVirtualFileSystemIT.java +++ b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/client/filesystem/hadoop/GravitinoVirtualFileSystemIT.java @@ -103,8 +103,7 @@ public static void tearDown() throws IOException { public void testCreate() throws IOException { // create fileset String filesetName = "test_fileset_create"; - NameIdentifier filesetIdent = - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName); + NameIdentifier filesetIdent = NameIdentifier.of(schemaName, filesetName); Catalog catalog = metalake.loadCatalog(catalogName); String storageLocation = genStorageLocation(filesetName); catalog @@ -138,8 +137,7 @@ public void testCreate() throws IOException { public void testAppend() throws IOException { // create fileset String filesetName = "test_fileset_append"; - NameIdentifier filesetIdent = - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName); + NameIdentifier filesetIdent = NameIdentifier.of(schemaName, filesetName); Catalog catalog = metalake.loadCatalog(catalogName); String storageLocation = genStorageLocation(filesetName); catalog @@ -198,8 +196,7 @@ public void testAppend() throws IOException { public void testDelete() throws IOException { // create fileset String filesetName = "test_fileset_delete"; - NameIdentifier filesetIdent = - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName); + NameIdentifier filesetIdent = NameIdentifier.of(schemaName, filesetName); Catalog catalog = metalake.loadCatalog(catalogName); String storageLocation = genStorageLocation(filesetName); catalog @@ -238,8 +235,7 @@ public void testDelete() throws IOException { public void testGetStatus() throws IOException { // create fileset String filesetName = "test_fileset_get_status"; - NameIdentifier filesetIdent = - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName); + NameIdentifier filesetIdent = NameIdentifier.of(schemaName, filesetName); Catalog catalog = metalake.loadCatalog(catalogName); String storageLocation = genStorageLocation(filesetName); catalog @@ -281,8 +277,7 @@ public void testGetStatus() throws IOException { public void testListStatus() throws IOException { // create fileset String filesetName = "test_fileset_list_status"; - NameIdentifier filesetIdent = - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName); + NameIdentifier filesetIdent = NameIdentifier.of(schemaName, filesetName); Catalog catalog = metalake.loadCatalog(catalogName); String storageLocation = genStorageLocation(filesetName); catalog @@ -338,8 +333,7 @@ public void testListStatus() throws IOException { public void testMkdirs() throws IOException { // create fileset String filesetName = "test_fileset_mkdirs"; - NameIdentifier filesetIdent = - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName); + NameIdentifier filesetIdent = NameIdentifier.of(schemaName, filesetName); Catalog catalog = metalake.loadCatalog(catalogName); String storageLocation = genStorageLocation(filesetName); catalog @@ -373,8 +367,7 @@ public void testMkdirs() throws IOException { public void testRename() throws IOException { // create fileset String filesetName = "test_fileset_rename"; - NameIdentifier filesetIdent = - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName); + NameIdentifier filesetIdent = NameIdentifier.of(schemaName, filesetName); Catalog catalog = metalake.loadCatalog(catalogName); String storageLocation = genStorageLocation(filesetName); catalog diff --git a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/CatalogsPageTest.java b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/CatalogsPageTest.java index 30565589d7b..05d4fc77e44 100644 --- a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/CatalogsPageTest.java +++ b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/CatalogsPageTest.java @@ -219,7 +219,7 @@ void createFileset( catalog_fileset .asFilesetCatalog() .createFileset( - NameIdentifier.of(metalakeName, catalogName, schemaName, filesetName), + NameIdentifier.of(schemaName, filesetName), "comment", Fileset.Type.MANAGED, storageLocation,