Skip to content

Commit c5c9bef

Browse files
authored
[#3702]refactor(API): Refactoring SupportsCatalogs.listCatalogs() method to return String[] (#3741)
### What changes were proposed in this pull request? Currently, the SupportsCatalogs.listCatalogs() method returns an array of NameIdentifier to represents the catalogs. Actually a String[] will be more clear and easier to use. ### Why are the changes needed? To make the API more clear to use. Fix: #3702 ### Does this PR introduce _any_ user-facing change? Almost not; Just return type changed, will be more simple. ### How was this patch tested? Yes, many test cases cover this API.
1 parent 1952737 commit c5c9bef

File tree

10 files changed

+30
-37
lines changed

10 files changed

+30
-37
lines changed

api/src/main/java/com/datastrato/gravitino/SupportsCatalogs.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@
1818
public interface SupportsCatalogs {
1919

2020
/**
21-
* List all catalogs in the metalake.
21+
* List the name of all catalogs in the metalake.
2222
*
23-
* @return The list of catalog's name identifiers.
24-
* @throws NoSuchMetalakeException If the metalake with namespace does not exist.
23+
* @return The list of catalog's names.
24+
* @throws NoSuchMetalakeException If the metalake does not exist.
2525
*/
26-
NameIdentifier[] listCatalogs() throws NoSuchMetalakeException;
26+
String[] listCatalogs() throws NoSuchMetalakeException;
2727

2828
/**
2929
* List all catalogs with their information in the metalake.

catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/integration/test/CatalogHiveIT.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,8 @@ public static void stop() throws IOException {
207207
}));
208208
Arrays.stream(metalake.listCatalogs())
209209
.forEach(
210-
(ident -> {
211-
metalake.dropCatalog(ident.name());
210+
(catalogName -> {
211+
metalake.dropCatalog(catalogName);
212212
}));
213213
if (client != null) {
214214
client.dropMetalake(metalakeName);

catalogs/catalog-kafka/src/test/java/com/datastrato/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ public static void shutdown() {
108108
}));
109109
Arrays.stream(metalake.listCatalogs())
110110
.forEach(
111-
(ident -> {
112-
metalake.dropCatalog(ident.name());
111+
(catalogName -> {
112+
metalake.dropCatalog(catalogName);
113113
}));
114114
client.dropMetalake(METALAKE_NAME);
115115
if (adminClient != null) {

clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoClient.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import com.datastrato.gravitino.Catalog;
99
import com.datastrato.gravitino.CatalogChange;
10-
import com.datastrato.gravitino.NameIdentifier;
1110
import com.datastrato.gravitino.SupportsCatalogs;
1211
import com.datastrato.gravitino.exceptions.CatalogAlreadyExistsException;
1312
import com.datastrato.gravitino.exceptions.NoSuchCatalogException;
@@ -58,7 +57,7 @@ private GravitinoMetalake getMetalake() {
5857
}
5958

6059
@Override
61-
public NameIdentifier[] listCatalogs() throws NoSuchMetalakeException {
60+
public String[] listCatalogs() throws NoSuchMetalakeException {
6261
return getMetalake().listCatalogs();
6362
}
6463

clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoMetalake.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,11 @@ public class GravitinoMetalake extends MetalakeDTO implements SupportsCatalogs {
5252
/**
5353
* List all the catalogs under this metalake.
5454
*
55-
* @return A list of {@link NameIdentifier} of the catalogs under the specified namespace.
56-
* @throws NoSuchMetalakeException if the metalake with specified namespace does not exist.
55+
* @return A list of the catalog names under the current metalake.
56+
* @throws NoSuchMetalakeException If the metalake does not exist.
5757
*/
5858
@Override
59-
public NameIdentifier[] listCatalogs() throws NoSuchMetalakeException {
59+
public String[] listCatalogs() throws NoSuchMetalakeException {
6060

6161
EntityListResponse resp =
6262
restClient.get(
@@ -66,7 +66,7 @@ public NameIdentifier[] listCatalogs() throws NoSuchMetalakeException {
6666
ErrorHandlers.catalogErrorHandler());
6767
resp.validate();
6868

69-
return resp.identifiers();
69+
return Arrays.stream(resp.identifiers()).map(NameIdentifier::name).toArray(String[]::new);
7070
}
7171

7272
/**

clients/client-java/src/test/java/com/datastrato/gravitino/client/TestGravitinoMetalake.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,16 @@ public void testListCatalogs() throws JsonProcessingException {
7676

7777
EntityListResponse resp = new EntityListResponse(new NameIdentifier[] {ident1, ident2});
7878
buildMockResource(Method.GET, path, null, resp, HttpStatus.SC_OK);
79-
NameIdentifier[] catalogs = gravitinoClient.listCatalogs();
79+
String[] catalogs = gravitinoClient.listCatalogs();
8080

8181
Assertions.assertEquals(2, catalogs.length);
82-
Assertions.assertEquals(ident1, catalogs[0]);
83-
Assertions.assertEquals(ident2, catalogs[1]);
82+
Assertions.assertEquals(ident1.name(), catalogs[0]);
83+
Assertions.assertEquals(ident2.name(), catalogs[1]);
8484

8585
// Test return empty catalog list
8686
EntityListResponse resp1 = new EntityListResponse(new NameIdentifier[] {});
8787
buildMockResource(Method.GET, path, null, resp1, HttpStatus.SC_OK);
88-
NameIdentifier[] catalogs1 = gravitinoClient.listCatalogs();
88+
String[] catalogs1 = gravitinoClient.listCatalogs();
8989
Assertions.assertEquals(0, catalogs1.length);
9090

9191
// Test return internal error

flink-connector/src/main/java/com/datastrato/gravitino/flink/connector/catalog/GravitinoCatalogManager.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,13 @@
55
package com.datastrato.gravitino.flink.connector.catalog;
66

77
import com.datastrato.gravitino.Catalog;
8-
import com.datastrato.gravitino.NameIdentifier;
98
import com.datastrato.gravitino.client.GravitinoAdminClient;
109
import com.datastrato.gravitino.client.GravitinoMetalake;
1110
import com.google.common.base.Preconditions;
11+
import com.google.common.collect.Sets;
1212
import java.util.Arrays;
1313
import java.util.Map;
1414
import java.util.Set;
15-
import java.util.stream.Collectors;
1615
import org.slf4j.Logger;
1716
import org.slf4j.LoggerFactory;
1817

@@ -128,12 +127,12 @@ public boolean dropCatalog(String catalogName) {
128127
* @return Set of catalog names
129128
*/
130129
public Set<String> listCatalogs() {
131-
NameIdentifier[] catalogNames = metalake.listCatalogs();
130+
String[] catalogNames = metalake.listCatalogs();
132131
LOG.info(
133132
"Load metalake {}'s catalogs. catalogs: {}.",
134133
metalake.name(),
135134
Arrays.toString(catalogNames));
136-
return Arrays.stream(catalogNames).map(NameIdentifier::name).collect(Collectors.toSet());
135+
return Sets.newHashSet(catalogNames);
137136
}
138137

139138
/**

integration-test/src/test/java/com/datastrato/gravitino/integration/test/trino/TrinoQueryIT.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ public static void setup() throws Exception {
7979
private static void cleanupTestEnv() throws Exception {
8080
try {
8181
Arrays.stream(TrinoQueryITBase.metalake.listCatalogs())
82-
.filter(catalog -> catalog.name().startsWith("gt_"))
83-
.forEach(catalog -> TrinoQueryITBase.dropCatalog(catalog.name()));
82+
.filter(catalog -> catalog.startsWith("gt_"))
83+
.forEach(TrinoQueryITBase::dropCatalog);
8484

8585
await()
8686
.atMost(30, TimeUnit.SECONDS)

trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/catalog/CatalogConnectorManager.java

+5-8
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ private GravitinoMetalake retrieveMetalake(String metalakeName) {
142142
}
143143

144144
private void loadCatalogs(GravitinoMetalake metalake) {
145-
NameIdentifier[] catalogNames;
145+
String[] catalogNames;
146146
try {
147147
catalogNames = metalake.listCatalogs();
148148
} catch (Exception e) {
@@ -159,10 +159,7 @@ private void loadCatalogs(GravitinoMetalake metalake) {
159159
Set<String> catalogNameStrings =
160160
Arrays.stream(catalogNames)
161161
.map(
162-
id ->
163-
config.simplifyCatalogNames()
164-
? id.name()
165-
: getTrinoCatalogName(metalake.name(), id.name()))
162+
id -> config.simplifyCatalogNames() ? id : getTrinoCatalogName(metalake.name(), id))
166163
.collect(Collectors.toSet());
167164

168165
for (Map.Entry<String, CatalogConnectorContext> entry : catalogConnectors.entrySet()) {
@@ -181,9 +178,9 @@ private void loadCatalogs(GravitinoMetalake metalake) {
181178
// Load new catalogs belows to the metalake.
182179
Arrays.stream(catalogNames)
183180
.forEach(
184-
(NameIdentifier nameIdentifier) -> {
181+
(String catalogName) -> {
185182
try {
186-
Catalog catalog = metalake.loadCatalog(nameIdentifier.name());
183+
Catalog catalog = metalake.loadCatalog(catalogName);
187184
GravitinoCatalog gravitinoCatalog = new GravitinoCatalog(metalake.name(), catalog);
188185
if (catalogConnectors.containsKey(getTrinoCatalogName(gravitinoCatalog))) {
189186
// Reload catalogs that have been updated in Gravitino server.
@@ -195,7 +192,7 @@ private void loadCatalogs(GravitinoMetalake metalake) {
195192
}
196193
} catch (Exception e) {
197194
LOG.error(
198-
"Failed to load metalake {}'s catalog {}.", metalake.name(), nameIdentifier, e);
195+
"Failed to load metalake {}'s catalog {}.", metalake.name(), catalogName, e);
199196
}
200197
});
201198
}

trino-connector/src/test/java/com/datastrato/gravitino/trino/connector/GravitinoMockServer.java

+3-5
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,10 @@ private GravitinoMetalake createMetalake(String metalakeName) {
129129
when(metaLake.name()).thenReturn(metalakeName);
130130
when(metaLake.listCatalogs())
131131
.thenAnswer(
132-
new Answer<NameIdentifier[]>() {
132+
new Answer<String[]>() {
133133
@Override
134-
public NameIdentifier[] answer(InvocationOnMock invocation) throws Throwable {
135-
return metalakes.get(metalakeName).catalogs.keySet().stream()
136-
.map(catalogName -> NameIdentifier.of(metalakeName, catalogName))
137-
.toArray(NameIdentifier[]::new);
134+
public String[] answer(InvocationOnMock invocation) throws Throwable {
135+
return metalakes.get(metalakeName).catalogs.keySet().toArray(String[]::new);
138136
};
139137
});
140138

0 commit comments

Comments
 (0)