This is an automated email from the ASF dual-hosted git repository.
yuqi4733 pushed a commit to branch branch-0.9
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-0.9 by this push:
new 75a8f0e6f5 [#7686] improvement(client-java): Correct URL encode path
in alterModelVersion in GenericModelCatalog.java (#7694)
75a8f0e6f5 is described below
commit 75a8f0e6f5bd75ee3d3bb0c8adb93ca94ebada20
Author: Sua Bae <[email protected]>
AuthorDate: Tue Jul 15 10:30:10 2025 +0900
[#7686] improvement(client-java): Correct URL encode path in
alterModelVersion in GenericModelCatalog.java (#7694)
### What changes were proposed in this pull request?
The cherry-pick conflicts were successfully resolved by manually
### Why are the changes needed?
Close: #7686
### Does this PR introduce _any_ user-facing change?
None
### How was this patch tested?
using the client-java module's tests.
---
.../gravitino/client/GenericModelCatalog.java | 4 +-
.../gravitino/client/TestGenericModelCatalog.java | 207 +++++++++++++--------
2 files changed, 137 insertions(+), 74 deletions(-)
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/GenericModelCatalog.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/GenericModelCatalog.java
index c10b5bbc40..eeec6be5d8 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/GenericModelCatalog.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/GenericModelCatalog.java
@@ -329,7 +329,9 @@ class GenericModelCatalog extends BaseSchemaCatalog
implements ModelCatalog {
NameIdentifier modelFullIdent = modelFullNameIdentifier(ident);
ModelVersionResponse resp =
restClient.put(
- formatModelVersionRequestPath(modelFullIdent) + "/aliases/" +
alias,
+ formatModelVersionRequestPath(modelFullIdent)
+ + "/aliases/"
+ + RESTUtils.encodeString(alias),
req,
ModelVersionResponse.class,
Collections.emptyMap(),
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestGenericModelCatalog.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestGenericModelCatalog.java
index 1fdea36228..2c190288cc 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestGenericModelCatalog.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestGenericModelCatalog.java
@@ -53,11 +53,13 @@ import
org.apache.gravitino.exceptions.NoSuchModelVersionException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.model.Model;
import org.apache.gravitino.model.ModelVersion;
+import org.apache.gravitino.rest.RESTUtils;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.Method;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
public class TestGenericModelCatalog extends TestBase {
@@ -101,20 +103,24 @@ public class TestGenericModelCatalog extends TestBase {
metalake.createCatalog(CATALOG_NAME, Catalog.Type.MODEL, "comment",
Collections.emptyMap());
}
- @Test
- public void testListModels() throws JsonProcessingException {
- NameIdentifier modelId1 = NameIdentifier.of("schema1", "model1");
- NameIdentifier modelId2 = NameIdentifier.of("schema1", "model2");
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1/model2", "스키마1/모델1/모델2"})
+ public void testListModels(String input) throws JsonProcessingException {
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String[] modelNames = new String[] {split[1], split[2]};
+ NameIdentifier modelId1 = NameIdentifier.of(schemaName, modelNames[0]);
+ NameIdentifier modelId2 = NameIdentifier.of(schemaName, modelNames[1]);
NameIdentifier resultModelId1 =
- NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, "schema1", "model1");
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName,
modelNames[0]);
NameIdentifier resultModelId2 =
- NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, "schema1", "model2");
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName,
modelNames[1]);
String modelPath =
withSlash(
GenericModelCatalog.formatModelRequestPath(
- Namespace.of(METALAKE_NAME, CATALOG_NAME, "schema1")));
+ Namespace.of(METALAKE_NAME, CATALOG_NAME, schemaName)));
EntityListResponse resp =
new EntityListResponse(new NameIdentifier[] {resultModelId1,
resultModelId2});
@@ -143,15 +149,20 @@ public class TestGenericModelCatalog extends TestBase {
"internal error");
}
- @Test
- public void testGetModel() throws JsonProcessingException {
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1", "스키마1/모델1"})
+ public void testGetModel(String input) throws JsonProcessingException {
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
String modelPath =
withSlash(
GenericModelCatalog.formatModelRequestPath(
- Namespace.of(METALAKE_NAME, CATALOG_NAME, "schema1"))
+ Namespace.of(METALAKE_NAME, CATALOG_NAME, schemaName))
+ "/"
- + modelId.name());
+ + RESTUtils.encodeString(modelId.name()));
ModelDTO modelDTO = mockModelDTO("model1", 0, "model comment",
Collections.emptyMap());
ModelResponse resp = new ModelResponse(modelDTO);
@@ -178,10 +189,14 @@ public class TestGenericModelCatalog extends TestBase {
RuntimeException.class, () ->
catalog.asModelCatalog().getModel(modelId), "internal error");
}
- @Test
- public void testRegisterModel() throws JsonProcessingException {
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
- ModelDTO modelDTO = mockModelDTO("model1", 0, "model comment",
Collections.emptyMap());
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1", "스키마1/모델1"})
+ public void testRegisterModel(String input) throws JsonProcessingException {
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
+ ModelDTO modelDTO = mockModelDTO(schemaName, 0, "model comment",
Collections.emptyMap());
ModelResponse resp = new ModelResponse(modelDTO);
ModelRegisterRequest request =
new ModelRegisterRequest(modelId.name(), "model comment",
Collections.emptyMap());
@@ -189,7 +204,8 @@ public class TestGenericModelCatalog extends TestBase {
String modelPath =
withSlash(
GenericModelCatalog.formatModelRequestPath(
- Namespace.of(METALAKE_NAME, CATALOG_NAME, "schema1")));
+ Namespace.of(METALAKE_NAME, CATALOG_NAME, schemaName)));
+
buildMockResource(Method.POST, modelPath, request, resp, HttpStatus.SC_OK);
Model model =
@@ -237,15 +253,20 @@ public class TestGenericModelCatalog extends TestBase {
"internal error");
}
- @Test
- public void testDeleteModel() throws JsonProcessingException {
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1", "스키마1/모델1"})
+ public void testDeleteModel(String input) throws JsonProcessingException {
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
String modelPath =
withSlash(
GenericModelCatalog.formatModelRequestPath(
- Namespace.of(METALAKE_NAME, CATALOG_NAME, "schema1"))
+ Namespace.of(METALAKE_NAME, CATALOG_NAME, schemaName))
+ "/"
- + modelId.name());
+ + RESTUtils.encodeString(modelId.name()));
DropResponse resp = new DropResponse(true);
buildMockResource(Method.DELETE, modelPath, null, resp, HttpStatus.SC_OK);
@@ -262,13 +283,17 @@ public class TestGenericModelCatalog extends TestBase {
"internal error");
}
- @Test
- public void testListModelVersions() throws JsonProcessingException {
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1", "스키마1/모델1"})
+ public void testListModelVersions(String input) throws
JsonProcessingException {
+ String split[] = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
String modelVersionPath =
withSlash(
GenericModelCatalog.formatModelVersionRequestPath(
- NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, "schema1",
"model1"))
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName,
modelName))
+ "/versions");
int[] expectedVersions = new int[] {0, 1, 2};
@@ -299,13 +324,17 @@ public class TestGenericModelCatalog extends TestBase {
"internal error");
}
- @Test
- public void testGetModelVersion() throws JsonProcessingException {
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1", "스키마1/모델1"})
+ public void testGetModelVersion(String input) throws JsonProcessingException
{
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
String modelVersionPath =
withSlash(
GenericModelCatalog.formatModelVersionRequestPath(
- NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, "schema1",
"model1"))
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName,
modelName))
+ "/versions/0");
ModelVersionDTO mockModelVersion =
@@ -339,22 +368,27 @@ public class TestGenericModelCatalog extends TestBase {
"internal error");
}
- @Test
- public void testGetModelVersionByAlias() throws JsonProcessingException {
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1/alias1/alias2", "스키마1/모델1/별칭1/별칭2"})
+ public void testGetModelVersionByAlias(String input) throws
JsonProcessingException {
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+ String[] aliasNames = new String[] {split[2], split[3]};
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
String modelVersionPath =
withSlash(
GenericModelCatalog.formatModelVersionRequestPath(
- NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, "schema1",
"model1"))
- + "/aliases/alias1");
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName,
modelName))
+ + "/aliases/"
+ + RESTUtils.encodeString(aliasNames[0]));
ModelVersionDTO mockModelVersion =
- mockModelVersion(
- 0, "uri", new String[] {"alias1", "alias2"}, "comment",
Collections.emptyMap());
+ mockModelVersion(0, "uri", aliasNames, "comment",
Collections.emptyMap());
ModelVersionResponse resp = new ModelVersionResponse(mockModelVersion);
buildMockResource(Method.GET, modelVersionPath, null, resp,
HttpStatus.SC_OK);
- ModelVersion modelVersion =
catalog.asModelCatalog().getModelVersion(modelId, "alias1");
+ ModelVersion modelVersion =
catalog.asModelCatalog().getModelVersion(modelId, aliasNames[0]);
compareModelVersion(mockModelVersion, modelVersion);
// Throw model version not found exception
@@ -365,7 +399,7 @@ public class TestGenericModelCatalog extends TestBase {
Assertions.assertThrows(
NoSuchModelVersionException.class,
- () -> catalog.asModelCatalog().getModelVersion(modelId, "alias1"),
+ () -> catalog.asModelCatalog().getModelVersion(modelId, aliasNames[0]),
"model version not found");
// Throw RuntimeException
@@ -375,17 +409,21 @@ public class TestGenericModelCatalog extends TestBase {
Assertions.assertThrows(
RuntimeException.class,
- () -> catalog.asModelCatalog().getModelVersion(modelId, "alias1"),
+ () -> catalog.asModelCatalog().getModelVersion(modelId, aliasNames[0]),
"internal error");
}
- @Test
- public void testLinkModelVersion() throws JsonProcessingException {
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1", "스키마1/모델1"})
+ public void testLinkModelVersion(String input) throws
JsonProcessingException {
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
String modelVersionPath =
withSlash(
GenericModelCatalog.formatModelVersionRequestPath(
- NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, "schema1",
"model1"))
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName,
modelName))
+ "/versions");
ModelVersionLinkRequest request =
@@ -462,13 +500,17 @@ public class TestGenericModelCatalog extends TestBase {
"internal error");
}
- @Test
- public void testDeleteModelVersion() throws JsonProcessingException {
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1", "스키마1/모델1"})
+ public void testDeleteModelVersion(String input) throws
JsonProcessingException {
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
String modelVersionPath =
withSlash(
GenericModelCatalog.formatModelVersionRequestPath(
- NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, "schema1",
"model1"))
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName,
modelName))
+ "/versions/0");
DropResponse resp = new DropResponse(true);
@@ -487,19 +529,25 @@ public class TestGenericModelCatalog extends TestBase {
"internal error");
}
- @Test
- public void testDeleteModelVersionByAlias() throws JsonProcessingException {
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1/alias1", "스키마1/모델1/별칭1"})
+ public void testDeleteModelVersionByAlias(String input) throws
JsonProcessingException {
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+ String aliasName = split[2];
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
String modelVersionPath =
withSlash(
GenericModelCatalog.formatModelVersionRequestPath(
- NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, "schema1",
"model1"))
- + "/aliases/alias1");
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName,
modelName))
+ + "/aliases/"
+ + RESTUtils.encodeString(aliasName));
DropResponse resp = new DropResponse(true);
buildMockResource(Method.DELETE, modelVersionPath, null, resp,
HttpStatus.SC_OK);
- Assertions.assertTrue(catalog.asModelCatalog().deleteModelVersion(modelId,
"alias1"));
+ Assertions.assertTrue(catalog.asModelCatalog().deleteModelVersion(modelId,
aliasName));
// Test RuntimeException
ErrorResponse errResp = ErrorResponse.internalError("internal error");
@@ -508,23 +556,26 @@ public class TestGenericModelCatalog extends TestBase {
Assertions.assertThrows(
RuntimeException.class,
- () -> catalog.asModelCatalog().deleteModelVersion(modelId, "alias1"),
+ () -> catalog.asModelCatalog().deleteModelVersion(modelId, aliasName),
"internal error");
}
- @Test
- public void testAlterModel() throws JsonProcessingException {
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1/new_model1", "스키마1/모델1/새로운_모델1"})
+ public void testAlterModel(String input) throws JsonProcessingException {
+ String[] split = input.split("/");
+ String schema = split[0];
+ String oldName = split[1];
+ String newName = split[2];
String comment = "comment";
- String schema = "schema1";
- String oldName = "model1";
- String newName = "new_model";
+
NameIdentifier modelId = NameIdentifier.of(schema, oldName);
String modelPath =
withSlash(
GenericModelCatalog.formatModelRequestPath(
Namespace.of(METALAKE_NAME, CATALOG_NAME, schema))
+ "/"
- + modelId.name());
+ + RESTUtils.encodeString(modelId.name()));
// Test rename the model
ModelUpdateRequest.RenameModelRequest renameModelReq =
@@ -572,18 +623,22 @@ public class TestGenericModelCatalog extends TestBase {
"internal error");
}
- @Test
- void testUpdateModelVersionComment() throws JsonProcessingException {
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1/alias1/alias2", "스키마1/모델1/별칭1/별칭2"})
+ void testUpdateModelVersionComment(String input) throws
JsonProcessingException {
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+ String[] aliases = new String[] {split[2], split[3]};
String newComment = "new comment";
String uri = "uri";
int version = 0;
- String[] aliases = new String[] {"alias1", "alias2"};
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
String modelVersionPath =
withSlash(
GenericModelCatalog.formatModelVersionRequestPath(
- NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, "schema1",
"model1"))
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName,
modelName))
+ "/versions/0");
ModelVersionDTO mockModelVersion =
@@ -646,19 +701,25 @@ public class TestGenericModelCatalog extends TestBase {
"internal error");
}
- @Test
- void testUpdateModelVersionCommentByAlias() throws JsonProcessingException {
+ @ParameterizedTest
+ @ValueSource(strings = {"schema1/model1/alias1/alias2", "스키마1/모델1/별칭1/별칭2"})
+ void testUpdateModelVersionCommentByAlias(String input) throws
JsonProcessingException {
+ String[] split = input.split("/");
+ String schemaName = split[0];
+ String modelName = split[1];
+ String[] aliases = new String[] {split[2], split[3]};
+
String newComment = "new comment";
String uri = "uri";
int version = 0;
- String[] aliases = new String[] {"alias1", "alias2"};
- NameIdentifier modelId = NameIdentifier.of("schema1", "model1");
+ NameIdentifier modelId = NameIdentifier.of(schemaName, modelName);
String modelVersionPath =
withSlash(
GenericModelCatalog.formatModelVersionRequestPath(
- NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, "schema1",
"model1"))
- + "/aliases/alias1");
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaName,
modelName))
+ + "/aliases/"
+ + RESTUtils.encodeString(aliases[0]));
ModelVersionDTO mockModelVersion =
mockModelVersion(version, uri, aliases, newComment,
Collections.emptyMap());
