FANNG1 commented on code in PR #5737:
URL: https://github.com/apache/gravitino/pull/5737#discussion_r1886429657


##########
bundles/azure-bundle/src/main/java/org/apache/gravitino/abs/credential/ADLSTokenProvider.java:
##########
@@ -0,0 +1,136 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package org.apache.gravitino.abs.credential;
+
+import com.azure.core.util.Context;
+import com.azure.identity.ClientSecretCredential;
+import com.azure.identity.ClientSecretCredentialBuilder;
+import com.azure.storage.file.datalake.DataLakeServiceClient;
+import com.azure.storage.file.datalake.DataLakeServiceClientBuilder;
+import com.azure.storage.file.datalake.implementation.util.DataLakeSasImplUtil;
+import com.azure.storage.file.datalake.models.UserDelegationKey;
+import com.azure.storage.file.datalake.sas.DataLakeServiceSasSignatureValues;
+import com.azure.storage.file.datalake.sas.PathSasPermission;
+import java.time.OffsetDateTime;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import org.apache.gravitino.credential.ADLSTokenCredential;
+import org.apache.gravitino.credential.Credential;
+import org.apache.gravitino.credential.CredentialConstants;
+import org.apache.gravitino.credential.CredentialContext;
+import org.apache.gravitino.credential.CredentialProvider;
+import org.apache.gravitino.credential.PathBasedCredentialContext;
+import org.apache.gravitino.credential.config.ADLSCredentialConfig;
+
+/** Generates ADLS token to access ADLS data. */
+public class ADLSTokenProvider implements CredentialProvider {
+  private String storageAccountName;
+  private String tenantId;
+  private String clientId;
+  private String clientSecret;
+  private String endpoint;
+  private Integer tokenExpireSecs;
+
+  @Override
+  public void initialize(Map<String, String> properties) {
+    ADLSCredentialConfig adlsCredentialConfig = new 
ADLSCredentialConfig(properties);
+    this.storageAccountName = adlsCredentialConfig.storageAccountName();
+    this.tenantId = adlsCredentialConfig.tenantId();
+    this.clientId = adlsCredentialConfig.clientId();
+    this.clientSecret = adlsCredentialConfig.clientSecret();
+    this.endpoint =
+        String.format("https://%s.%s";, storageAccountName, 
ADLSTokenCredential.ADLS_DOMAIN);
+    this.tokenExpireSecs = adlsCredentialConfig.tokenExpireInSecs();
+  }
+
+  @Override
+  public void close() {}
+
+  @Override
+  public String credentialType() {
+    return CredentialConstants.ADLS_TOKEN_CREDENTIAL_PROVIDER_TYPE;
+  }
+
+  @Override
+  public Credential getCredential(CredentialContext context) {
+    if (!(context instanceof PathBasedCredentialContext)) {
+      return null;
+    }
+    PathBasedCredentialContext pathBasedCredentialContext = 
(PathBasedCredentialContext) context;
+
+    Set<String> writePaths = pathBasedCredentialContext.getWritePaths();
+    Set<String> readPaths = pathBasedCredentialContext.getReadPaths();
+
+    Set<String> combinedPaths = new HashSet<>(writePaths);
+    combinedPaths.addAll(readPaths);
+
+    if (combinedPaths.size() != 1) {
+      throw new IllegalArgumentException(
+          "ADLS should contain exactly one unique path, but found: "
+              + combinedPaths.size()
+              + " paths: "
+              + combinedPaths);
+    }
+    String uniquePath = combinedPaths.iterator().next();
+
+    ClientSecretCredential clientSecretCredential =
+        new ClientSecretCredentialBuilder()
+            .tenantId(tenantId)
+            .clientId(clientId)
+            .clientSecret(clientSecret)
+            .build();
+
+    DataLakeServiceClient dataLakeServiceClient =
+        new DataLakeServiceClientBuilder()
+            .endpoint(endpoint)
+            .credential(clientSecretCredential)
+            .buildClient();
+
+    OffsetDateTime start = OffsetDateTime.now();
+    OffsetDateTime expiry = start.plusSeconds(tokenExpireSecs);
+    UserDelegationKey userDelegationKey = 
dataLakeServiceClient.getUserDelegationKey(start, expiry);
+
+    PathSasPermission pathSasPermission =

Review Comment:
   if there is only read paths, we should not request write related permissions



##########
api/src/main/java/org/apache/gravitino/credential/ADLSTokenCredential.java:
##########
@@ -0,0 +1,110 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package org.apache.gravitino.credential;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+
+/** ADLS SAS token credential. */
+public class ADLSTokenCredential implements Credential {
+
+  /** ADLS SAS token credential type. */
+  private static final String ADLS_SAS_TOKEN_CREDENTIAL_TYPE = 
"adls-sas-token";
+  /** ADLS base domain */
+  public static final String ADLS_DOMAIN = "dfs.core.windows.net";
+  /** ADLS storage account name */
+  public static final String GRAVITINO_ADLS_STORAGE_ACCOUNT_NAME = 
"adls-storage-account-name";
+  /** ADLS SAS token used to access ADLS data. */
+  public static final String GRAVITINO_ADLS_SAS_TOKEN = "adls-sas-token";
+
+  private String accountName;
+  private String sasToken;
+  private long expireTimeInMS;
+
+  /**
+   * Constructs an instance of {@link ADLSTokenCredential} with SAS token.
+   *
+   * @param accountName The ADLS account name.
+   * @param sasToken The ADLS SAS token.
+   * @param expireTimeInMS The SAS token expire time in ms.
+   */
+  public ADLSTokenCredential(String accountName, String sasToken, long 
expireTimeInMS) {
+    validate(accountName, sasToken, expireTimeInMS);
+    this.accountName = accountName;
+    this.sasToken = sasToken;
+    this.expireTimeInMS = expireTimeInMS;
+  }

Review Comment:
   please create a default ADLSTokenCredential constructor, add tests in 
`TestCredentialFactory`



##########
common/src/main/java/org/apache/gravitino/credential/CredentialPropertyUtils.java:
##########
@@ -75,6 +78,17 @@ public static Map<String, String> 
toIcebergProperties(Credential credential) {
     if (credential instanceof OSSTokenCredential || credential instanceof 
OSSSecretKeyCredential) {
       return transformProperties(credential.credentialInfo(), 
icebergCredentialPropertyMap);
     }
+    if (credential instanceof ADLSTokenCredential) {

Review Comment:
   could you add a test in TestCredentialPropertiesUtils?



##########
bundles/azure-bundle/src/main/java/org/apache/gravitino/abs/credential/ADLSTokenProvider.java:
##########
@@ -0,0 +1,136 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package org.apache.gravitino.abs.credential;
+
+import com.azure.core.util.Context;
+import com.azure.identity.ClientSecretCredential;
+import com.azure.identity.ClientSecretCredentialBuilder;
+import com.azure.storage.file.datalake.DataLakeServiceClient;
+import com.azure.storage.file.datalake.DataLakeServiceClientBuilder;
+import com.azure.storage.file.datalake.implementation.util.DataLakeSasImplUtil;
+import com.azure.storage.file.datalake.models.UserDelegationKey;
+import com.azure.storage.file.datalake.sas.DataLakeServiceSasSignatureValues;
+import com.azure.storage.file.datalake.sas.PathSasPermission;
+import java.time.OffsetDateTime;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import org.apache.gravitino.credential.ADLSTokenCredential;
+import org.apache.gravitino.credential.Credential;
+import org.apache.gravitino.credential.CredentialConstants;
+import org.apache.gravitino.credential.CredentialContext;
+import org.apache.gravitino.credential.CredentialProvider;
+import org.apache.gravitino.credential.PathBasedCredentialContext;
+import org.apache.gravitino.credential.config.ADLSCredentialConfig;
+
+/** Generates ADLS token to access ADLS data. */
+public class ADLSTokenProvider implements CredentialProvider {
+  private String storageAccountName;
+  private String tenantId;
+  private String clientId;
+  private String clientSecret;
+  private String endpoint;
+  private Integer tokenExpireSecs;
+
+  @Override
+  public void initialize(Map<String, String> properties) {
+    ADLSCredentialConfig adlsCredentialConfig = new 
ADLSCredentialConfig(properties);
+    this.storageAccountName = adlsCredentialConfig.storageAccountName();
+    this.tenantId = adlsCredentialConfig.tenantId();
+    this.clientId = adlsCredentialConfig.clientId();
+    this.clientSecret = adlsCredentialConfig.clientSecret();
+    this.endpoint =
+        String.format("https://%s.%s";, storageAccountName, 
ADLSTokenCredential.ADLS_DOMAIN);
+    this.tokenExpireSecs = adlsCredentialConfig.tokenExpireInSecs();
+  }
+
+  @Override
+  public void close() {}
+
+  @Override
+  public String credentialType() {
+    return CredentialConstants.ADLS_TOKEN_CREDENTIAL_PROVIDER_TYPE;
+  }
+
+  @Override
+  public Credential getCredential(CredentialContext context) {
+    if (!(context instanceof PathBasedCredentialContext)) {
+      return null;
+    }
+    PathBasedCredentialContext pathBasedCredentialContext = 
(PathBasedCredentialContext) context;
+
+    Set<String> writePaths = pathBasedCredentialContext.getWritePaths();
+    Set<String> readPaths = pathBasedCredentialContext.getReadPaths();
+
+    Set<String> combinedPaths = new HashSet<>(writePaths);
+    combinedPaths.addAll(readPaths);
+
+    if (combinedPaths.size() != 1) {
+      throw new IllegalArgumentException(
+          "ADLS should contain exactly one unique path, but found: "
+              + combinedPaths.size()
+              + " paths: "
+              + combinedPaths);
+    }
+    String uniquePath = combinedPaths.iterator().next();
+
+    ClientSecretCredential clientSecretCredential =
+        new ClientSecretCredentialBuilder()
+            .tenantId(tenantId)
+            .clientId(clientId)
+            .clientSecret(clientSecret)
+            .build();
+
+    DataLakeServiceClient dataLakeServiceClient =
+        new DataLakeServiceClientBuilder()
+            .endpoint(endpoint)
+            .credential(clientSecretCredential)
+            .buildClient();
+
+    OffsetDateTime start = OffsetDateTime.now();
+    OffsetDateTime expiry = start.plusSeconds(tokenExpireSecs);
+    UserDelegationKey userDelegationKey = 
dataLakeServiceClient.getUserDelegationKey(start, expiry);
+
+    PathSasPermission pathSasPermission =
+        new PathSasPermission()
+            .setReadPermission(true)
+            .setWritePermission(true)
+            .setDeletePermission(true)
+            .setListPermission(true)
+            .setCreatePermission(true)
+            .setAddPermission(true);
+
+    DataLakeServiceSasSignatureValues signatureValues =
+        new DataLakeServiceSasSignatureValues(expiry, pathSasPermission);
+
+    ADLSLocationUtils.ADLSLocationParts locationParts = 
ADLSLocationUtils.parseLocation(uniquePath);
+    String sasToken =
+        new DataLakeSasImplUtil(
+                signatureValues,
+                locationParts.getContainer(),
+                locationParts.getPath().replaceAll("^/+|/*$", ""),
+                true)
+            .generateUserDelegationSas(
+                userDelegationKey, locationParts.getAccountName(), 
Context.NONE);
+
+    return new ADLSTokenCredential(
+        locationParts.getAccountName(), sasToken, tokenExpireSecs * 1000);

Review Comment:
   should return `now + tokenExpireSecs * 1000` here



##########
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/ADLSProperties.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.storage;
+
+// Defines unified properties for Azure Data Lake Storage (ADLS) 
configurations.
+public class ADLSProperties {
+
+  // Configuration key for specifying the name of the ADLS storage account.
+  public static final String GRAVITINO_ADLS_STORAGE_ACCOUNT_NAME = 
"adls-storage-account-name";

Review Comment:
   adls-storage-account-name -> azure-storage-account-name?
   adls-storage-account-key -> azure-storage-account-key?



##########
iceberg/iceberg-rest-server/build.gradle.kts:
##########
@@ -79,6 +80,13 @@ dependencies {
 
   testImplementation(libs.iceberg.aws.bundle)
   testImplementation(libs.iceberg.gcp.bundle)
+  // Prevent netty conflict
+  testImplementation(libs.reactor.netty.http)

Review Comment:
   Do you know why add these two package could prevent netty conflict?



##########
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/ADLSProperties.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.storage;
+
+// Defines unified properties for Azure Data Lake Storage (ADLS) 
configurations.
+public class ADLSProperties {

Review Comment:
   Could you rename `ADLSProperties` to `AzureProperties`? 



##########
common/src/main/java/org/apache/gravitino/credential/CredentialPropertyUtils.java:
##########
@@ -53,7 +54,9 @@ public class CredentialPropertyUtils {
           OSSTokenCredential.GRAVITINO_OSS_SESSION_ACCESS_KEY_ID,
           ICEBERG_OSS_ACCESS_KEY_ID,
           OSSTokenCredential.GRAVITINO_OSS_SESSION_SECRET_ACCESS_KEY,
-          ICEBERG_OSS_ACCESS_KEY_SECRET);
+          ICEBERG_OSS_ACCESS_KEY_SECRET,

Review Comment:
   There is no need to add this, because ADLS token is added by credential 
vending.



##########
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/ADLSProperties.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.storage;
+
+// Defines unified properties for Azure Data Lake Storage (ADLS) 
configurations.
+public class ADLSProperties {
+
+  // Configuration key for specifying the name of the ADLS storage account.
+  public static final String GRAVITINO_ADLS_STORAGE_ACCOUNT_NAME = 
"adls-storage-account-name";
+  // Configuration key for specifying the key of the ADLS storage account.
+  public static final String GRAVITINO_ADLS_STORAGE_ACCOUNT_KEY = 
"adls-storage-account-key";
+
+  // Configuration key for specifying the Azure Active Directory (AAD) tenant 
ID.
+  public static final String GRAVITINO_ADLS_TENANT_ID = "adls-tenant-id";

Review Comment:
   adls-tenant-id -> azure-tenant-id
   adls-client-id -> azure-tenant-id
   adls-client-secret -> azure-client-secret
   
   since these properties are not related to adls



##########
bundles/azure-bundle/src/main/java/org/apache/gravitino/abs/credential/ADLSTokenProvider.java:
##########
@@ -0,0 +1,136 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package org.apache.gravitino.abs.credential;
+
+import com.azure.core.util.Context;
+import com.azure.identity.ClientSecretCredential;
+import com.azure.identity.ClientSecretCredentialBuilder;
+import com.azure.storage.file.datalake.DataLakeServiceClient;
+import com.azure.storage.file.datalake.DataLakeServiceClientBuilder;
+import com.azure.storage.file.datalake.implementation.util.DataLakeSasImplUtil;
+import com.azure.storage.file.datalake.models.UserDelegationKey;
+import com.azure.storage.file.datalake.sas.DataLakeServiceSasSignatureValues;
+import com.azure.storage.file.datalake.sas.PathSasPermission;
+import java.time.OffsetDateTime;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import org.apache.gravitino.credential.ADLSTokenCredential;
+import org.apache.gravitino.credential.Credential;
+import org.apache.gravitino.credential.CredentialConstants;
+import org.apache.gravitino.credential.CredentialContext;
+import org.apache.gravitino.credential.CredentialProvider;
+import org.apache.gravitino.credential.PathBasedCredentialContext;
+import org.apache.gravitino.credential.config.ADLSCredentialConfig;
+
+/** Generates ADLS token to access ADLS data. */
+public class ADLSTokenProvider implements CredentialProvider {
+  private String storageAccountName;
+  private String tenantId;
+  private String clientId;
+  private String clientSecret;
+  private String endpoint;
+  private Integer tokenExpireSecs;
+
+  @Override
+  public void initialize(Map<String, String> properties) {
+    ADLSCredentialConfig adlsCredentialConfig = new 
ADLSCredentialConfig(properties);
+    this.storageAccountName = adlsCredentialConfig.storageAccountName();
+    this.tenantId = adlsCredentialConfig.tenantId();
+    this.clientId = adlsCredentialConfig.clientId();
+    this.clientSecret = adlsCredentialConfig.clientSecret();
+    this.endpoint =
+        String.format("https://%s.%s";, storageAccountName, 
ADLSTokenCredential.ADLS_DOMAIN);
+    this.tokenExpireSecs = adlsCredentialConfig.tokenExpireInSecs();
+  }
+
+  @Override
+  public void close() {}
+
+  @Override
+  public String credentialType() {
+    return CredentialConstants.ADLS_TOKEN_CREDENTIAL_PROVIDER_TYPE;
+  }
+
+  @Override
+  public Credential getCredential(CredentialContext context) {
+    if (!(context instanceof PathBasedCredentialContext)) {
+      return null;
+    }
+    PathBasedCredentialContext pathBasedCredentialContext = 
(PathBasedCredentialContext) context;
+
+    Set<String> writePaths = pathBasedCredentialContext.getWritePaths();
+    Set<String> readPaths = pathBasedCredentialContext.getReadPaths();
+
+    Set<String> combinedPaths = new HashSet<>(writePaths);
+    combinedPaths.addAll(readPaths);
+
+    if (combinedPaths.size() != 1) {
+      throw new IllegalArgumentException(
+          "ADLS should contain exactly one unique path, but found: "
+              + combinedPaths.size()
+              + " paths: "
+              + combinedPaths);
+    }
+    String uniquePath = combinedPaths.iterator().next();
+
+    ClientSecretCredential clientSecretCredential =
+        new ClientSecretCredentialBuilder()
+            .tenantId(tenantId)
+            .clientId(clientId)
+            .clientSecret(clientSecret)
+            .build();
+
+    DataLakeServiceClient dataLakeServiceClient =
+        new DataLakeServiceClientBuilder()
+            .endpoint(endpoint)
+            .credential(clientSecretCredential)
+            .buildClient();
+
+    OffsetDateTime start = OffsetDateTime.now();
+    OffsetDateTime expiry = start.plusSeconds(tokenExpireSecs);
+    UserDelegationKey userDelegationKey = 
dataLakeServiceClient.getUserDelegationKey(start, expiry);
+
+    PathSasPermission pathSasPermission =
+        new PathSasPermission()
+            .setReadPermission(true)
+            .setWritePermission(true)
+            .setDeletePermission(true)
+            .setListPermission(true)
+            .setCreatePermission(true)
+            .setAddPermission(true);
+
+    DataLakeServiceSasSignatureValues signatureValues =
+        new DataLakeServiceSasSignatureValues(expiry, pathSasPermission);
+
+    ADLSLocationUtils.ADLSLocationParts locationParts = 
ADLSLocationUtils.parseLocation(uniquePath);
+    String sasToken =
+        new DataLakeSasImplUtil(
+                signatureValues,
+                locationParts.getContainer(),
+                locationParts.getPath().replaceAll("^/+|/*$", ""),

Review Comment:
   please provide a separate function to do `replaceAll("^/+|/*$", "")`, add 
comments and test to it



##########
api/src/main/java/org/apache/gravitino/credential/ADLSTokenCredential.java:
##########
@@ -0,0 +1,110 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package org.apache.gravitino.credential;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+
+/** ADLS SAS token credential. */
+public class ADLSTokenCredential implements Credential {
+
+  /** ADLS SAS token credential type. */
+  private static final String ADLS_SAS_TOKEN_CREDENTIAL_TYPE = 
"adls-sas-token";
+  /** ADLS base domain */
+  public static final String ADLS_DOMAIN = "dfs.core.windows.net";
+  /** ADLS storage account name */
+  public static final String GRAVITINO_ADLS_STORAGE_ACCOUNT_NAME = 
"adls-storage-account-name";

Review Comment:
   adls-storage-account-name -> azure-storage-account-name ?



##########
api/src/main/java/org/apache/gravitino/credential/ADLSTokenCredential.java:
##########
@@ -0,0 +1,110 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package org.apache.gravitino.credential;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+
+/** ADLS SAS token credential. */
+public class ADLSTokenCredential implements Credential {
+
+  /** ADLS SAS token credential type. */
+  private static final String ADLS_SAS_TOKEN_CREDENTIAL_TYPE = 
"adls-sas-token";
+  /** ADLS base domain */
+  public static final String ADLS_DOMAIN = "dfs.core.windows.net";
+  /** ADLS storage account name */
+  public static final String GRAVITINO_ADLS_STORAGE_ACCOUNT_NAME = 
"adls-storage-account-name";
+  /** ADLS SAS token used to access ADLS data. */
+  public static final String GRAVITINO_ADLS_SAS_TOKEN = "adls-sas-token";
+
+  private String accountName;
+  private String sasToken;
+  private long expireTimeInMS;
+
+  /**
+   * Constructs an instance of {@link ADLSTokenCredential} with SAS token.
+   *
+   * @param accountName The ADLS account name.
+   * @param sasToken The ADLS SAS token.
+   * @param expireTimeInMS The SAS token expire time in ms.
+   */
+  public ADLSTokenCredential(String accountName, String sasToken, long 
expireTimeInMS) {
+    validate(accountName, sasToken, expireTimeInMS);
+    this.accountName = accountName;
+    this.sasToken = sasToken;
+    this.expireTimeInMS = expireTimeInMS;
+  }
+
+  @Override
+  public String credentialType() {
+    return ADLS_SAS_TOKEN_CREDENTIAL_TYPE;
+  }
+
+  @Override
+  public long expireTimeInMs() {
+    return expireTimeInMS;
+  }
+
+  @Override
+  public Map<String, String> credentialInfo() {
+    return (new ImmutableMap.Builder<String, String>())
+        .put(GRAVITINO_ADLS_SAS_TOKEN, sasToken)
+        .build();
+  }
+
+  @Override
+  public void initialize(Map<String, String> credentialInfo, long 
expireTimeInMS) {
+    String accountName = 
credentialInfo.get(GRAVITINO_ADLS_STORAGE_ACCOUNT_NAME);
+    String sasToken = credentialInfo.get(GRAVITINO_ADLS_SAS_TOKEN);
+    validate(accountName, sasToken, expireTimeInMS);
+    this.accountName = accountName;
+    this.sasToken = sasToken;
+    this.expireTimeInMS = expireTimeInMS;
+  }
+
+  private void validate(String accountName, String sasToken, long 
expireTimeInMS) {

Review Comment:
   suggest to move private method to the last position of the class.



##########
api/src/main/java/org/apache/gravitino/credential/ADLSTokenCredential.java:
##########
@@ -0,0 +1,110 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package org.apache.gravitino.credential;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+
+/** ADLS SAS token credential. */
+public class ADLSTokenCredential implements Credential {
+
+  /** ADLS SAS token credential type. */
+  private static final String ADLS_SAS_TOKEN_CREDENTIAL_TYPE = 
"adls-sas-token";

Review Comment:
   adls-sas-token -> azure-adls-token?



##########
bundles/azure-bundle/src/main/java/org/apache/gravitino/abs/credential/ADLSLocationUtils.java:
##########
@@ -0,0 +1,61 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package org.apache.gravitino.abs.credential;
+
+import java.net.URI;
+
+public class ADLSLocationUtils {
+  public static class ADLSLocationParts {
+    private final String container;
+    private final String accountName;
+    private final String path;
+
+    public ADLSLocationParts(String container, String accountName, String 
path) {
+      this.container = container;
+      this.accountName = accountName;
+      this.path = path;
+    }
+
+    public String getContainer() {
+      return container;
+    }
+
+    public String getAccountName() {
+      return accountName;
+    }
+
+    public String getPath() {
+      return path;
+    }
+  }
+
+  public static ADLSLocationParts parseLocation(String location) {

Review Comment:
   please add an test



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to