This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 3f330959da [#8335] fix(client): Fix double encoding issue and add
missing URL encoding (#8336)
3f330959da is described below
commit 3f330959da7d3c713d1d85896f87c2674cc40108
Author: Jerry Shao <[email protected]>
AuthorDate: Wed Sep 3 15:03:11 2025 +0800
[#8335] fix(client): Fix double encoding issue and add missing URL encoding
(#8336)
### What changes were proposed in this pull request?
This PR mainly fixes server things:
1. Fix the double encoding issue. Because Java and Python client will
automatically encode the query parameters. Doing it manually will encode
the string twice, which will lead to unexpected issues.
2. Add missing encoding in the URL.
3. Add a client version number in the header to let the server check and
do the action based on the version.
### Why are the changes needed?
Fix the double encoding problem.
Fix: #8335
### Does this PR introduce _any_ user-facing change?
Yes, the change will break the backward compatibility for fileset
related interfaces.
### How was this patch tested?
Local test.
---
.../apache/gravitino/client/FilesetCatalog.java | 4 +--
.../apache/gravitino/client/GravitinoMetalake.java | 13 +++++---
.../apache/gravitino/client/GravitinoVersion.java | 17 ++--------
.../org/apache/gravitino/client/HTTPClient.java | 9 ++++++
.../client/MetadataObjectCredentialOperations.java | 5 +--
.../client/MetadataObjectRoleOperations.java | 5 +--
.../client/MetadataObjectTagOperations.java | 4 +--
.../gravitino/client/TestFilesetCatalog.java | 5 ++-
.../gravitino/client/TestGravitinoVersion.java | 6 ++++
.../gravitino/client/fileset_catalog.py | 6 ++--
.../gravitino/client/generic_model_catalog.py | 6 ++--
.../gravitino/client/gravitino_client_base.py | 8 ++++-
.../gravitino/client/gravitino_metalake.py | 10 ++----
.../metadata_object_credential_operations.py | 5 +--
.../gravitino/filesystem/gvfs_base_operations.py | 10 ++++--
.../tests/unittests/test_gravitino_client.py | 5 +--
.../gravitino/filesystem/hadoop/TestGvfsBase.java | 26 ++++++++--------
.../main/java/org/apache/gravitino/Version.java | 36 ++++++++++++++++++++--
.../java/org/apache/gravitino/rest/RESTUtils.java | 15 ++-------
.../org/apache/gravitino/server/web/Utils.java | 11 +++++++
.../server/web/rest/FilesetOperations.java | 35 ++++++++++-----------
.../server/web/rest/TestFilesetOperations.java | 2 ++
22 files changed, 145 insertions(+), 98 deletions(-)
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/FilesetCatalog.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/FilesetCatalog.java
index 2bef734d06..33c9b8d131 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/FilesetCatalog.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/FilesetCatalog.java
@@ -249,9 +249,9 @@ class FilesetCatalog extends BaseSchemaCatalog
CallerContext callerContext = CallerContext.CallerContextHolder.get();
Map<String, String> params = new HashMap<>();
- params.put("sub_path", RESTUtils.encodeString(subPath));
+ params.put("sub_path", subPath);
if (locationName != null) {
- params.put("location_name", RESTUtils.encodeString(locationName));
+ params.put("location_name", locationName);
}
FileLocationResponse resp =
restClient.get(
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
index fe2ade5c4c..ae01555de0 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
@@ -175,7 +175,7 @@ public class GravitinoMetalake extends MetalakeDTO
EntityListResponse resp =
restClient.get(
- String.format("api/metalakes/%s/catalogs", this.name()),
+ String.format("api/metalakes/%s/catalogs",
RESTUtils.encodeString(this.name())),
EntityListResponse.class,
Collections.emptyMap(),
ErrorHandlers.catalogErrorHandler());
@@ -197,7 +197,7 @@ public class GravitinoMetalake extends MetalakeDTO
params.put("details", "true");
CatalogListResponse resp =
restClient.get(
- String.format("api/metalakes/%s/catalogs", this.name()),
+ String.format("api/metalakes/%s/catalogs",
RESTUtils.encodeString(this.name())),
params,
CatalogListResponse.class,
Collections.emptyMap(),
@@ -220,7 +220,10 @@ public class GravitinoMetalake extends MetalakeDTO
CatalogResponse resp =
restClient.get(
- String.format(API_METALAKES_CATALOGS_PATH, this.name(),
catalogName),
+ String.format(
+ API_METALAKES_CATALOGS_PATH,
+ RESTUtils.encodeString(this.name()),
+ RESTUtils.encodeString(catalogName)),
CatalogResponse.class,
Collections.emptyMap(),
ErrorHandlers.catalogErrorHandler());
@@ -255,7 +258,7 @@ public class GravitinoMetalake extends MetalakeDTO
CatalogResponse resp =
restClient.post(
- String.format("api/metalakes/%s/catalogs", this.name()),
+ String.format("api/metalakes/%s/catalogs",
RESTUtils.encodeString(this.name())),
req,
CatalogResponse.class,
Collections.emptyMap(),
@@ -1472,7 +1475,7 @@ public class GravitinoMetalake extends MetalakeDTO
JobListResponse resp =
restClient.get(
String.format(API_METALAKES_JOB_PATH,
RESTUtils.encodeString(this.name())),
- ImmutableMap.of("jobTemplateName",
RESTUtils.encodeString(jobTemplateName)),
+ ImmutableMap.of("jobTemplateName", jobTemplateName),
JobListResponse.class,
Collections.emptyMap(),
ErrorHandlers.jobErrorHandler());
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoVersion.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoVersion.java
index 83e09b3429..3fe2aef5a5 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoVersion.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoVersion.java
@@ -19,16 +19,12 @@
package org.apache.gravitino.client;
import com.google.common.annotations.VisibleForTesting;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
+import org.apache.gravitino.Version;
import org.apache.gravitino.dto.VersionDTO;
-import org.apache.gravitino.exceptions.GravitinoRuntimeException;
/** Apache Gravitino version information. */
public class GravitinoVersion extends VersionDTO {
- private static final int VERSION_PART_NUMBER = 3;
-
@VisibleForTesting
GravitinoVersion(String version, String compileDate, String gitCommit) {
super(version, compileDate, gitCommit);
@@ -41,16 +37,7 @@ public class GravitinoVersion extends VersionDTO {
@VisibleForTesting
/** @return parse the version number for a version string */
int[] getVersionNumber() {
- Pattern pattern = Pattern.compile("^(\\d+)\\.(\\d+)\\.(\\d+)(?:[-].+)?$");
- Matcher matcher = pattern.matcher(version());
- if (matcher.matches()) {
- int[] versionNumbers = new int[VERSION_PART_NUMBER];
- for (int i = 0; i < VERSION_PART_NUMBER; i++) {
- versionNumbers[i] = Integer.parseInt(matcher.group(i + 1));
- }
- return versionNumbers;
- }
- throw new GravitinoRuntimeException("Invalid version string " + version());
+ return Version.parseVersionNumber(version());
}
/**
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/HTTPClient.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/HTTPClient.java
index 915e9c07a9..c1ae1a7f32 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/HTTPClient.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/HTTPClient.java
@@ -33,6 +33,8 @@ import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.stream.Collectors;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Version;
import org.apache.gravitino.auth.AuthConstants;
import org.apache.gravitino.dto.responses.ErrorResponse;
import org.apache.gravitino.exceptions.RESTException;
@@ -680,7 +682,14 @@ public class HTTPClient implements RESTClient {
// Some systems require the Content-Type header to be set even for
empty-bodied requests to
// avoid failures.
request.setHeader(HttpHeaders.CONTENT_TYPE, bodyMimeType);
+ // Set the API version header
request.setHeader(HttpHeaders.ACCEPT, VERSION_HEADER);
+
+ // Set the client version header
+ if (StringUtils.isNotBlank(Version.getCurrentVersion().version)) {
+ request.setHeader(Version.CLIENT_VERSION_HEADER,
Version.getCurrentVersion().version);
+ }
+
if (requestHeaders != null) {
requestHeaders.forEach(request::setHeader);
}
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectCredentialOperations.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectCredentialOperations.java
index 58819dd94a..f80c932b31 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectCredentialOperations.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectCredentialOperations.java
@@ -26,6 +26,7 @@ import org.apache.gravitino.credential.Credential;
import org.apache.gravitino.credential.SupportsCredentials;
import org.apache.gravitino.dto.responses.CredentialResponse;
import org.apache.gravitino.dto.util.DTOConverters;
+import org.apache.gravitino.rest.RESTUtils;
/**
* The implementation of {@link SupportsCredentials}. This interface will be
composited into
@@ -43,9 +44,9 @@ class MetadataObjectCredentialOperations implements
SupportsCredentials {
this.credentialRequestPath =
String.format(
"api/metalakes/%s/objects/%s/%s/credentials",
- metalakeName,
+ RESTUtils.encodeString(metalakeName),
metadataObject.type().name().toLowerCase(Locale.ROOT),
- metadataObject.fullName());
+ RESTUtils.encodeString(metadataObject.fullName()));
}
@Override
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectRoleOperations.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectRoleOperations.java
index 54a6634355..0b6b31f006 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectRoleOperations.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectRoleOperations.java
@@ -23,6 +23,7 @@ import java.util.Locale;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.authorization.SupportsRoles;
import org.apache.gravitino.dto.responses.NameListResponse;
+import org.apache.gravitino.rest.RESTUtils;
class MetadataObjectRoleOperations implements SupportsRoles {
@@ -36,9 +37,9 @@ class MetadataObjectRoleOperations implements SupportsRoles {
this.roleRequestPath =
String.format(
"api/metalakes/%s/objects/%s/%s/roles",
- metalakeName,
+ RESTUtils.encodeString(metalakeName),
metadataObject.type().name().toLowerCase(Locale.ROOT),
- metadataObject.fullName());
+ RESTUtils.encodeString(metadataObject.fullName()));
}
@Override
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectTagOperations.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectTagOperations.java
index 2ac84bdbf5..29ec00512b 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectTagOperations.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/MetadataObjectTagOperations.java
@@ -53,9 +53,9 @@ class MetadataObjectTagOperations implements SupportsTags {
this.tagRequestPath =
String.format(
"api/metalakes/%s/objects/%s/%s/tags",
- metalakeName,
+ RESTUtils.encodeString(metalakeName),
metadataObject.type().name().toLowerCase(Locale.ROOT),
- metadataObject.fullName());
+ RESTUtils.encodeString(metadataObject.fullName()));
}
@Override
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 d878c0d684..f71bdce0e4 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
@@ -62,7 +62,6 @@ import
org.apache.gravitino.exceptions.NoSuchLocationNameException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NotFoundException;
import org.apache.gravitino.file.Fileset;
-import org.apache.gravitino.rest.RESTUtils;
import org.apache.hc.core5.http.Method;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
@@ -439,7 +438,7 @@ public class TestFilesetCatalog extends TestBase {
FilesetCatalog.formatFileLocationRequestPath(
Namespace.of(metalakeName, catalogName, "schema1"),
fileset.name()));
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString(mockSubPath));
+ queryParams.put("sub_path", mockSubPath);
String mockFileLocation =
String.format("file:/fileset/%s/%s/%s/%s", catalogName, "schema1",
"fileset1", mockSubPath);
@@ -531,7 +530,7 @@ public class TestFilesetCatalog extends TestBase {
FilesetCatalog.formatFileLocationRequestPath(
Namespace.of(metalakeName, catalogName, "schema1"),
fileset.name()));
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString(mockSubPath));
+ queryParams.put("sub_path", mockSubPath);
String mockFileLocation =
String.format("file:/fileset/%s/%s/%s/%s", catalogName, "schema1",
"fileset1", mockSubPath);
FileLocationResponse resp = new FileLocationResponse(mockFileLocation);
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestGravitinoVersion.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestGravitinoVersion.java
index 041bb30544..9da527a97a 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestGravitinoVersion.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestGravitinoVersion.java
@@ -108,6 +108,12 @@ public class TestGravitinoVersion {
assertEquals(9, versionNumber[1]);
assertEquals(0, versionNumber[2]);
+ version = new GravitinoVersion("1.0.0.dev0", "2023-01-01", "1234567");
+ versionNumber = version.getVersionNumber();
+ assertEquals(1, versionNumber[0]);
+ assertEquals(0, versionNumber[1]);
+ assertEquals(0, versionNumber[2]);
+
// Test an invalid the version string with 2 part
version = new GravitinoVersion("2.5", "2023-01-01", "1234567");
assertThrows(GravitinoRuntimeException.class, version::getVersionNumber);
diff --git a/clients/client-python/gravitino/client/fileset_catalog.py
b/clients/client-python/gravitino/client/fileset_catalog.py
index 27931f7d70..4ef8e0ecca 100644
--- a/clients/client-python/gravitino/client/fileset_catalog.py
+++ b/clients/client-python/gravitino/client/fileset_catalog.py
@@ -196,7 +196,7 @@ class FilesetCatalog(BaseSchemaCatalog,
SupportsCredentials):
full_namespace = self._get_fileset_full_namespace(ident.namespace())
req = FilesetCreateRequest(
- name=encode_string(ident.name()),
+ name=ident.name(),
comment=comment,
fileset_type=fileset_type,
storage_locations=storage_locations,
@@ -300,10 +300,10 @@ class FilesetCatalog(BaseSchemaCatalog,
SupportsCredentials):
try:
caller_context: CallerContext = CallerContextHolder.get()
params = {
- "sub_path": encode_string(sub_path),
+ "sub_path": sub_path,
}
if location_name is not None:
- params["location_name"] = encode_string(location_name)
+ params["location_name"] = location_name
resp = self.rest_client.get(
self.format_file_location_request_path(full_namespace,
ident.name()),
diff --git a/clients/client-python/gravitino/client/generic_model_catalog.py
b/clients/client-python/gravitino/client/generic_model_catalog.py
index e71287a1fc..bf8065addc 100644
--- a/clients/client-python/gravitino/client/generic_model_catalog.py
+++ b/clients/client-python/gravitino/client/generic_model_catalog.py
@@ -160,7 +160,7 @@ class GenericModelCatalog(BaseSchemaCatalog):
model_full_ns = self._model_full_namespace(ident.namespace())
model_req = ModelRegisterRequest(
- name=encode_string(ident.name()), comment=comment,
properties=properties
+ name=ident.name(), comment=comment, properties=properties
)
model_req.validate()
@@ -520,7 +520,7 @@ class GenericModelCatalog(BaseSchemaCatalog):
model_full_ident = self._model_full_identifier(model_ident)
params = {}
if uri_name is not None:
- params["uriName"] = encode_string(uri_name)
+ params["uriName"] = uri_name
resp = self.rest_client.get(
f"{self._format_model_version_request_path(model_full_ident)}/versions/{version}/uri",
@@ -557,7 +557,7 @@ class GenericModelCatalog(BaseSchemaCatalog):
model_full_ident = self._model_full_identifier(model_ident)
params = {}
if uri_name is not None:
- params["uriName"] = encode_string(uri_name)
+ params["uriName"] = uri_name
resp = self.rest_client.get(
f"{self._format_model_version_request_path(model_full_ident)}/aliases/{encode_string(alias)}/uri",
diff --git a/clients/client-python/gravitino/client/gravitino_client_base.py
b/clients/client-python/gravitino/client/gravitino_client_base.py
index 5c0068e2f5..cc4078aab8 100644
--- a/clients/client-python/gravitino/client/gravitino_client_base.py
+++ b/clients/client-python/gravitino/client/gravitino_client_base.py
@@ -51,6 +51,8 @@ class GravitinoClientBase:
API_METALAKES_IDENTIFIER_PATH = f"{API_METALAKES_LIST_PATH}/"
"""The REST API path prefix for load a specific metalake"""
+ CLIENT_VERSION_HEADER = "X-Client-Version"
+
def __init__(
self,
uri: str,
@@ -59,10 +61,14 @@ class GravitinoClientBase:
request_headers: dict = None,
client_config: dict = None,
):
+ new_headers = request_headers.copy() if request_headers else {}
+ client_version = self.get_client_version().version()
+ if client_version is not None:
+ new_headers[self.CLIENT_VERSION_HEADER] = client_version.strip()
self._rest_client = HTTPClient(
uri,
auth_data_provider=auth_data_provider,
- request_headers=request_headers,
+ request_headers=new_headers,
client_config=client_config,
)
if check_version:
diff --git a/clients/client-python/gravitino/client/gravitino_metalake.py
b/clients/client-python/gravitino/client/gravitino_metalake.py
index 3d7d7f4cfa..00c393537b 100644
--- a/clients/client-python/gravitino/client/gravitino_metalake.py
+++ b/clients/client-python/gravitino/client/gravitino_metalake.py
@@ -81,7 +81,7 @@ class GravitinoMetalake(MetalakeDTO, SupportsJobs):
Returns:
A list of the catalog names under this metalake.
"""
- url = f"api/metalakes/{self.name()}/catalogs"
+ url = f"api/metalakes/{encode_string(self.name())}/catalogs"
response = self.rest_client.get(url,
error_handler=CATALOG_ERROR_HANDLER)
entity_list = EntityListResponse.from_json(response.body,
infer_missing=True)
entity_list.validate()
@@ -97,7 +97,7 @@ class GravitinoMetalake(MetalakeDTO, SupportsJobs):
A list of Catalog under the specified namespace.
"""
params = {"details": "true"}
- url = f"api/metalakes/{self.name()}/catalogs"
+ url = f"api/metalakes/{encode_string(self.name())}/catalogs"
response = self.rest_client.get(
url, params=params, error_handler=CATALOG_ERROR_HANDLER
)
@@ -374,11 +374,7 @@ class GravitinoMetalake(MetalakeDTO, SupportsJobs):
Returns:
A list of JobHandle objects representing the jobs.
"""
- params = (
- {"jobTemplateName": encode_string(job_template_name)}
- if job_template_name
- else {}
- )
+ params = {"jobTemplateName": job_template_name} if job_template_name
else {}
url =
self.API_METALAKES_JOB_RUNS_PATH.format(encode_string(self.name()))
response = self.rest_client.get(
url, params=params, error_handler=JOB_ERROR_HANDLER
diff --git
a/clients/client-python/gravitino/client/metadata_object_credential_operations.py
b/clients/client-python/gravitino/client/metadata_object_credential_operations.py
index 8ae83b0ec1..12295b6223 100644
---
a/clients/client-python/gravitino/client/metadata_object_credential_operations.py
+++
b/clients/client-python/gravitino/client/metadata_object_credential_operations.py
@@ -26,6 +26,7 @@ from gravitino.dto.responses.credential_response import
CredentialResponse
from gravitino.exceptions.handlers.credential_error_handler import (
CREDENTIAL_ERROR_HANDLER,
)
+from gravitino.rest.rest_utils import encode_string
from gravitino.utils import HTTPClient
from gravitino.utils.credential_factory import CredentialFactory
@@ -49,8 +50,8 @@ class MetadataObjectCredentialOperations(SupportsCredentials):
metadata_object_type = metadata_object.type().value
metadata_object_name = metadata_object.name()
self._request_path = (
- f"api/metalakes/{metalake_name}/objects/{metadata_object_type}/"
- f"{metadata_object_name}/credentials"
+
f"api/metalakes/{encode_string(metalake_name)}/objects/{metadata_object_type}/"
+ f"{encode_string(metadata_object_name)}/credentials"
)
def get_credentials(self) -> List[Credential]:
diff --git a/clients/client-python/gravitino/filesystem/gvfs_base_operations.py
b/clients/client-python/gravitino/filesystem/gvfs_base_operations.py
index ce57a7a781..86059b986e 100644
--- a/clients/client-python/gravitino/filesystem/gvfs_base_operations.py
+++ b/clients/client-python/gravitino/filesystem/gvfs_base_operations.py
@@ -142,7 +142,7 @@ class BaseGVFSOperations(ABC):
if options is None
else options.get(
GVFSConfig.GVFS_FILESYSTEM_ENABLE_FILESET_METADATA_CACHE,
- self.ENABLE_FILESET_METADATA_CACHE_DEFAULT
+ self.ENABLE_FILESET_METADATA_CACHE_DEFAULT,
)
)
if self._enable_fileset_metadata_cache:
@@ -572,7 +572,9 @@ class BaseGVFSOperations(ABC):
)
catalog: FilesetCatalog = self._get_fileset_catalog(catalog_ident)
return catalog.as_fileset_catalog().load_fileset(
- NameIdentifier.of(fileset_ident.namespace().level(2),
fileset_ident.name())
+ NameIdentifier.of(
+ fileset_ident.namespace().level(2), fileset_ident.name()
+ )
)
read_lock = self._fileset_cache_lock.gen_rlock()
@@ -596,7 +598,9 @@ class BaseGVFSOperations(ABC):
)
catalog: FilesetCatalog = self._get_fileset_catalog(catalog_ident)
fileset = catalog.as_fileset_catalog().load_fileset(
- NameIdentifier.of(fileset_ident.namespace().level(2),
fileset_ident.name())
+ NameIdentifier.of(
+ fileset_ident.namespace().level(2), fileset_ident.name()
+ )
)
self._fileset_cache[fileset_ident] = fileset
return fileset
diff --git a/clients/client-python/tests/unittests/test_gravitino_client.py
b/clients/client-python/tests/unittests/test_gravitino_client.py
index 103b8cd396..80ebda8c16 100644
--- a/clients/client-python/tests/unittests/test_gravitino_client.py
+++ b/clients/client-python/tests/unittests/test_gravitino_client.py
@@ -34,7 +34,8 @@ class TestMetalake(unittest.TestCase):
request_headers=expected_headers,
)
self.assertEqual(
- expected_headers,
gravitino_admin_client._rest_client.request_headers
+ expected_headers["k1"],
+ gravitino_admin_client._rest_client.request_headers["k1"],
)
gravitino_client = GravitinoClient(
@@ -43,7 +44,7 @@ class TestMetalake(unittest.TestCase):
request_headers=expected_headers,
)
self.assertEqual(
- expected_headers, gravitino_client._rest_client.request_headers
+ expected_headers["k1"],
gravitino_client._rest_client.request_headers["k1"]
)
def test_gravitino_client_timeout(self, *mock_methods):
diff --git
a/clients/filesystem-hadoop3/src/test/java/org/apache/gravitino/filesystem/hadoop/TestGvfsBase.java
b/clients/filesystem-hadoop3/src/test/java/org/apache/gravitino/filesystem/hadoop/TestGvfsBase.java
index 7d266341bc..fa6b79161d 100644
---
a/clients/filesystem-hadoop3/src/test/java/org/apache/gravitino/filesystem/hadoop/TestGvfsBase.java
+++
b/clients/filesystem-hadoop3/src/test/java/org/apache/gravitino/filesystem/hadoop/TestGvfsBase.java
@@ -331,7 +331,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
// test proxied local fs, should not get the same fs
FileLocationResponse fileLocationResponse = new
FileLocationResponse(localPath.toString());
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString(""));
+ queryParams.put("sub_path", "");
buildMockResource(Method.GET, locationPath, queryParams, null,
fileLocationResponse, SC_OK);
buildMockResourceForCredential(filesetName, localPath.toString());
@@ -375,7 +375,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
try (FileSystem fs = filesetPath1.getFileSystem(configuration1)) {
FileLocationResponse fileLocationResponse = new
FileLocationResponse(localPath1.toString());
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString(""));
+ queryParams.put("sub_path", "");
buildMockResource(Method.GET, locationPath1, queryParams, null,
fileLocationResponse, SC_OK);
buildMockResourceForCredential("fileset1", localPath1.toString());
FileSystemTestUtils.mkdirs(filesetPath1, fs);
@@ -432,7 +432,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
ImmutableMap.of(PROPERTY_DEFAULT_LOCATION_NAME, "location1"));
FileLocationResponse fileLocationResponse = new
FileLocationResponse(localPath + "/test.txt");
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString("/test.txt"));
+ queryParams.put("sub_path", "/test.txt");
buildMockResource(Method.GET, locationPath, queryParams, null,
fileLocationResponse, SC_OK);
buildMockResourceForCredential(filesetName, localPath + "/test.txt");
@@ -488,7 +488,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
// test managed fileset append
FileLocationResponse fileLocationResponse = new
FileLocationResponse(localPath + "/test.txt");
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString("/test.txt"));
+ queryParams.put("sub_path", "/test.txt");
buildMockResource(Method.GET, locationPath, queryParams, null,
fileLocationResponse, SC_OK);
buildMockResourceForCredential(filesetName, localPath + "/test.txt");
@@ -550,13 +550,13 @@ public class TestGvfsBase extends GravitinoMockServerBase
{
FileLocationResponse fileLocationResponse =
new FileLocationResponse(localPath + "/rename_src");
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString("/rename_src"));
+ queryParams.put("sub_path", "/rename_src");
buildMockResource(Method.GET, locationPath, queryParams, null,
fileLocationResponse, SC_OK);
FileLocationResponse fileLocationResponse1 =
new FileLocationResponse(localPath + "/rename_dst2");
Map<String, String> queryParams1 = new HashMap<>();
- queryParams1.put("sub_path", RESTUtils.encodeString("/rename_dst2"));
+ queryParams1.put("sub_path", "/rename_dst2");
buildMockResource(Method.GET, locationPath, queryParams1, null,
fileLocationResponse1, SC_OK);
buildMockResourceForCredential(filesetName, localPath + "/rename_dst2");
@@ -627,7 +627,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
FileLocationResponse fileLocationResponse =
new FileLocationResponse(localPath + "/test_delete");
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString("/test_delete"));
+ queryParams.put("sub_path", "/test_delete");
buildMockResource(Method.GET, locationPath, queryParams, null,
fileLocationResponse, SC_OK);
buildMockResourceForCredential(filesetName, localPath + "/test_delete");
@@ -674,7 +674,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
FileLocationResponse fileLocationResponse = new
FileLocationResponse(localPath.toString());
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString(""));
+ queryParams.put("sub_path", "");
buildMockResource(Method.GET, locationPath, queryParams, null,
fileLocationResponse, SC_OK);
buildMockResourceForCredential(filesetName, localPath.toString());
@@ -719,7 +719,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
FileLocationResponse fileLocationResponse = new
FileLocationResponse(localPath.toString());
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString(""));
+ queryParams.put("sub_path", "");
buildMockResource(Method.GET, locationPath, queryParams, null,
fileLocationResponse, SC_OK);
buildMockResourceForCredential(filesetName, localPath.toString());
@@ -770,7 +770,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
FileLocationResponse fileLocationResponse =
new FileLocationResponse(localPath + "/test_mkdirs");
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString("/test_mkdirs"));
+ queryParams.put("sub_path", "/test_mkdirs");
buildMockResource(Method.GET, locationPath, queryParams, null,
fileLocationResponse, SC_OK);
buildMockResourceForCredential(filesetName, localPath + "/test_mkdirs");
@@ -896,7 +896,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
FileLocationResponse fileLocationResponse = new
FileLocationResponse(localPath.toString());
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString(""));
+ queryParams.put("sub_path", "");
buildMockResource(Method.GET, locationPath, queryParams, null,
fileLocationResponse, SC_OK);
buildMockResourceForCredential(filesetName, localPath.toString());
@@ -921,7 +921,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
FileLocationResponse fileLocationResponse = new
FileLocationResponse(localPath.toString());
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString(""));
+ queryParams.put("sub_path", "");
buildMockResource(Method.GET, locationPath, queryParams, null,
fileLocationResponse, SC_OK);
buildMockResourceForCredential(filesetName, localPath.toString());
@@ -965,7 +965,7 @@ public class TestGvfsBase extends GravitinoMockServerBase {
(GravitinoVirtualFileSystem) managedFilesetPath.getFileSystem(conf)) {
Map<String, String> queryParams = new HashMap<>();
- queryParams.put("sub_path", RESTUtils.encodeString(""));
+ queryParams.put("sub_path", "");
ErrorResponse errResp =
ErrorResponse.notFound(NoSuchFilesetException.class.getSimpleName(),
"fileset not found");
buildMockResource(Method.GET, locationPath, queryParams, null, errResp,
SC_NOT_FOUND);
diff --git a/common/src/main/java/org/apache/gravitino/Version.java
b/common/src/main/java/org/apache/gravitino/Version.java
index d327656b4a..5dcf78a4fe 100644
--- a/common/src/main/java/org/apache/gravitino/Version.java
+++ b/common/src/main/java/org/apache/gravitino/Version.java
@@ -18,19 +18,30 @@
*/
package org.apache.gravitino;
+import com.google.common.base.Preconditions;
import java.io.IOException;
import java.io.InputStream;
import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.dto.VersionDTO;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
/** Retrieve the version and build information from the building process */
public class Version {
+ /** The HTTP header to send the client version */
+ public static final String CLIENT_VERSION_HEADER = "X-Client-Version";
+
+ private static final int VERSION_PART_NUMBER = 3;
+ private static final Pattern PATTERN =
+ Pattern.compile("^(\\d+)\\.(\\d+)\\.(\\d+)(?:-.*|\\.([a-zA-Z].*))?$");
+
private static final Version INSTANCE = new Version();
- private VersionInfo versionInfo;
- private VersionDTO versionDTO;
+ private final VersionInfo versionInfo;
+ private final VersionDTO versionDTO;
private Version() {
Properties projectProperties = new Properties();
@@ -69,6 +80,27 @@ public class Version {
return INSTANCE.versionDTO;
}
+ /**
+ * Parse the version number from a version string
+ *
+ * @param versionString the version string to parse
+ * @return an array of integers representing the version number
+ */
+ public static int[] parseVersionNumber(String versionString) {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(versionString), "Version string is null or
empty");
+
+ Matcher matcher = PATTERN.matcher(versionString);
+ if (matcher.matches()) {
+ int[] versionNumbers = new int[VERSION_PART_NUMBER];
+ for (int i = 0; i < VERSION_PART_NUMBER; i++) {
+ versionNumbers[i] = Integer.parseInt(matcher.group(i + 1));
+ }
+ return versionNumbers;
+ }
+ throw new GravitinoRuntimeException("Invalid version string " +
versionString);
+ }
+
/** Store version information */
public static class VersionInfo {
/** build version */
diff --git a/common/src/main/java/org/apache/gravitino/rest/RESTUtils.java
b/common/src/main/java/org/apache/gravitino/rest/RESTUtils.java
index 98ef6a0dc0..1a268de82f 100644
--- a/common/src/main/java/org/apache/gravitino/rest/RESTUtils.java
+++ b/common/src/main/java/org/apache/gravitino/rest/RESTUtils.java
@@ -24,7 +24,6 @@ import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.io.UncheckedIOException;
-import java.io.UnsupportedEncodingException;
import java.net.ServerSocket;
import java.net.URLDecoder;
import java.net.URLEncoder;
@@ -101,12 +100,7 @@ public class RESTUtils {
*/
public static String encodeString(String toEncode) {
Preconditions.checkArgument(toEncode != null, "Invalid string to encode:
null");
- try {
- return URLEncoder.encode(toEncode, StandardCharsets.UTF_8.name());
- } catch (UnsupportedEncodingException e) {
- throw new UncheckedIOException(
- String.format("Failed to URL encode '%s': UTF-8 encoding is not
supported", toEncode), e);
- }
+ return URLEncoder.encode(toEncode, StandardCharsets.UTF_8);
}
/**
@@ -119,12 +113,7 @@ public class RESTUtils {
*/
public static String decodeString(String encoded) {
Preconditions.checkArgument(encoded != null, "Invalid string to decode:
null");
- try {
- return URLDecoder.decode(encoded, StandardCharsets.UTF_8.name());
- } catch (UnsupportedEncodingException e) {
- throw new UncheckedIOException(
- String.format("Failed to URL decode '%s': UTF-8 encoding is not
supported", encoded), e);
- }
+ return URLDecoder.decode(encoded, StandardCharsets.UTF_8);
}
/**
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/web/Utils.java
b/server-common/src/main/java/org/apache/gravitino/server/web/Utils.java
index 175d70b8e6..b11db99c7c 100644
--- a/server-common/src/main/java/org/apache/gravitino/server/web/Utils.java
+++ b/server-common/src/main/java/org/apache/gravitino/server/web/Utils.java
@@ -28,6 +28,7 @@ import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.Version;
import org.apache.gravitino.audit.FilesetAuditConstants;
import org.apache.gravitino.audit.FilesetDataOperation;
import org.apache.gravitino.audit.InternalClientType;
@@ -234,4 +235,14 @@ public class Utils {
}
return filteredHeaders;
}
+
+ public static int[] getClientVersion(HttpServletRequest request) {
+ String clientVersion = request.getHeader(Version.CLIENT_VERSION_HEADER);
+ if (StringUtils.isBlank(clientVersion)) {
+ // If the client does not send the version, we assume it is the current
version.
+ clientVersion = Version.getCurrentVersion().version;
+ }
+
+ return StringUtils.isBlank(clientVersion) ? null :
Version.parseVersionNumber(clientVersion);
+ }
}
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
index 55a152dc70..cc73414602 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java
@@ -23,9 +23,6 @@ import static
org.apache.gravitino.file.Fileset.LOCATION_NAME_UNKNOWN;
import com.codahale.metrics.annotation.ResponseMetered;
import com.codahale.metrics.annotation.Timed;
import com.google.common.collect.ImmutableMap;
-import java.io.UnsupportedEncodingException;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
@@ -44,7 +41,6 @@ import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.Response;
-import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Entity;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
@@ -234,8 +230,7 @@ public class FilesetOperations {
@PathParam("schema") @AuthorizationMetadata(type =
Entity.EntityType.SCHEMA) String schema,
@PathParam("fileset") @AuthorizationMetadata(type =
Entity.EntityType.FILESET) String fileset,
@QueryParam("sub_path") @DefaultValue("/") String subPath,
- @QueryParam("location_name") String locationName)
- throws UnsupportedEncodingException {
+ @QueryParam("location_name") String locationName) {
LOG.info(
"Received list files request: {}.{}.{}.{}, subPath: {},
locationName:{}",
metalake,
@@ -245,15 +240,14 @@ public class FilesetOperations {
subPath,
locationName);
- final String decodedSubPath =
- StringUtils.isNotBlank(subPath)
- ? URLDecoder.decode(subPath, StandardCharsets.UTF_8.name())
- : subPath;
-
try {
return Utils.doAs(
httpRequest,
() -> {
+ int[] clientVersion = Utils.getClientVersion(httpRequest);
+ boolean isV1PlusClient = clientVersion == null || clientVersion[0]
>= 1;
+ String decodedSubPath = isV1PlusClient ? subPath :
RESTUtils.decodeString(subPath);
+
NameIdentifier filesetIdent =
NameIdentifierUtil.ofFileset(metalake, catalog, schema,
fileset);
FileInfo[] files = dispatcher.listFiles(filesetIdent,
locationName, decodedSubPath);
@@ -264,7 +258,7 @@ public class FilesetOperations {
catalog,
schema,
fileset,
- subPath,
+ decodedSubPath,
locationName);
return response;
});
@@ -374,12 +368,20 @@ public class FilesetOperations {
catalog,
schema,
fileset,
- RESTUtils.decodeString(subPath),
-
Optional.ofNullable(locationName).map(RESTUtils::decodeString).orElse(null));
+ subPath,
+ locationName);
try {
return Utils.doAs(
httpRequest,
() -> {
+ int[] clientVersion = Utils.getClientVersion(httpRequest);
+ boolean isV1PlusClient = clientVersion == null || clientVersion[0]
>= 1;
+ String decodedSubPath = isV1PlusClient ? subPath :
RESTUtils.decodeString(subPath);
+ String decodedLocationName =
+ isV1PlusClient
+ ? locationName
+ :
Optional.ofNullable(locationName).map(RESTUtils::decodeString).orElse(null);
+
NameIdentifier ident = NameIdentifierUtil.ofFileset(metalake,
catalog, schema, fileset);
Map<String, String> filteredAuditHeaders =
Utils.filterFilesetAuditHeaders(httpRequest);
// set the audit info into the thread local context
@@ -389,10 +391,7 @@ public class FilesetOperations {
CallerContext.CallerContextHolder.set(context);
}
String actualFileLocation =
- dispatcher.getFileLocation(
- ident,
- RESTUtils.decodeString(subPath),
-
Optional.ofNullable(locationName).map(RESTUtils::decodeString).orElse(null));
+ dispatcher.getFileLocation(ident, decodedSubPath,
decodedLocationName);
return Utils.ok(new FileLocationResponse(actualFileLocation));
});
} catch (Exception e) {
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 2878e93ccd..1a263be311 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
@@ -43,6 +43,7 @@ import org.apache.gravitino.Audit;
import org.apache.gravitino.Config;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Version;
import org.apache.gravitino.audit.CallerContext;
import org.apache.gravitino.audit.FilesetAuditConstants;
import org.apache.gravitino.audit.FilesetDataOperation;
@@ -82,6 +83,7 @@ public class TestFilesetOperations extends BaseOperationsTest
{
public HttpServletRequest get() {
HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getRemoteUser()).thenReturn(null);
+ when(request.getHeader(Version.CLIENT_VERSION_HEADER)).thenReturn(null);
return request;
}
}
