From 922065913033647014a799ff23395a5978d7c9b6 Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Wed, 10 Sep 2025 15:22:08 +0800 Subject: [PATCH 1/4] [core] RESTCatalog: add DLF OSS endpoint support and improve configuration merge --- .../paimon/rest/RESTCatalogOptions.java | 6 + .../java/org/apache/paimon/rest/RESTUtil.java | 27 ++- .../org/apache/paimon/utils/RESTUtilTest.java | 182 ++++++++++++++++++ .../org/apache/paimon/rest/RESTUtilTest.java | 90 --------- 4 files changed, 208 insertions(+), 97 deletions(-) create mode 100644 paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java delete mode 100644 paimon-core/src/test/java/org/apache/paimon/rest/RESTUtilTest.java diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/RESTCatalogOptions.java b/paimon-api/src/main/java/org/apache/paimon/rest/RESTCatalogOptions.java index deb3be5d3a64..d1a88ef4f839 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/RESTCatalogOptions.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/RESTCatalogOptions.java @@ -97,4 +97,10 @@ public class RESTCatalogOptions { .noDefaultValue() .withDescription( "The user agent of http client connecting to REST Catalog server."); + + public static final ConfigOption DLF_OSS_ENDPOINT = + ConfigOptions.key("dlf.oss-endpoint") + .stringType() + .noDefaultValue() + .withDescription("REST Catalog DLF OSS endpoint."); } diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java index 52a633b62a61..76ec2c99c1b1 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java @@ -42,6 +42,8 @@ import java.nio.charset.StandardCharsets; import java.util.Map; +import static org.apache.paimon.rest.RESTCatalogOptions.DLF_OSS_ENDPOINT; + /** Util for REST. */ public class RESTUtil { @@ -72,19 +74,30 @@ public static Map merge( if (updates == null) { updates = Maps.newHashMap(); } - ImmutableMap.Builder builder = ImmutableMap.builder(); - for (Map.Entry entry : targets.entrySet()) { - if (!updates.containsKey(entry.getKey()) && entry.getValue() != null) { - builder.put(entry.getKey(), entry.getValue()); + + Map result = Maps.newLinkedHashMap(); + + // First, add all non-null entries from updates + for (Map.Entry entry : updates.entrySet()) { + if (entry.getValue() != null) { + result.put(entry.getKey(), entry.getValue()); } } - for (Map.Entry entry : updates.entrySet()) { + + // Then, add entries from targets, which take precedence (override updates) + for (Map.Entry entry : targets.entrySet()) { if (entry.getValue() != null) { - builder.put(entry.getKey(), entry.getValue()); + result.put(entry.getKey(), entry.getValue()); } } - return builder.build(); + // Handle special case: dlf.oss-endpoint should override fs.oss.endpoint + String dlfOssEndpoint = result.get(DLF_OSS_ENDPOINT.key()); + if (dlfOssEndpoint != null && !dlfOssEndpoint.isEmpty()) { + result.put("fs.oss.endpoint", dlfOssEndpoint); + } + + return ImmutableMap.copyOf(result); } public static String encodeString(String toEncode) { diff --git a/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java b/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java new file mode 100644 index 000000000000..9cd2cff30880 --- /dev/null +++ b/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java @@ -0,0 +1,182 @@ +/* + * 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.paimon.utils; + +import org.apache.paimon.rest.RESTUtil; + +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.Map; + +import static org.apache.paimon.rest.RESTCatalogOptions.DLF_OSS_ENDPOINT; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** Test for {@link RESTUtil}. */ +public class RESTUtilTest { + @Test + public void testMerge() { + // Test case 1: targets has precedence over updates for existing keys + { + Map targets = new HashMap<>(); + targets.put("key1", "default1"); + targets.put("key2", "default2"); + Map updates = new HashMap<>(); + updates.put("key2", "update2"); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.get("key1"), "default1"); + // key2 should keep targets value, not be overridden by updates + assertEquals(result.get("key2"), "default2"); + } + + // Test case 2: targets has precedence even when updates has same value + { + Map targets = new HashMap<>(); + targets.put("key1", "default1"); + targets.put("key2", "default2"); + Map updates = new HashMap<>(); + updates.put("key1", "default1"); + updates.put("key2", "update2"); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.get("key1"), "default1"); + // key2 should keep targets value + assertEquals(result.get("key2"), "default2"); + } + + // Test case 3: empty updates, targets unchanged + { + Map targets = new HashMap<>(); + targets.put("key1", "default1"); + targets.put("key2", "default2"); + Map updates = new HashMap<>(); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.get("key1"), "default1"); + assertEquals(result.get("key2"), "default2"); + } + + // Test case 4: empty targets, updates are added + { + Map targets = new HashMap<>(); + Map updates = new HashMap<>(); + updates.put("key2", "update2"); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.get("key2"), "update2"); + } + + // Test case 5: both empty + { + Map targets = new HashMap<>(); + Map updates = new HashMap<>(); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.size(), 0); + } + + // Test case 6: both null + { + Map targets = null; + Map updates = null; + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.size(), 0); + } + + // Test case 7: null values are ignored + { + Map targets = new HashMap<>(); + targets.put("key3", null); + Map updates = new HashMap<>(); + updates.put("key2", null); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.size(), 0); + } + + // Test case 8: updates adds new keys that don't exist in targets + { + Map targets = new HashMap<>(); + targets.put("key1", "default1"); + Map updates = new HashMap<>(); + updates.put("key2", "update2"); + updates.put("key3", "update3"); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.get("key1"), "default1"); + assertEquals(result.get("key2"), "update2"); + assertEquals(result.get("key3"), "update3"); + assertEquals(result.size(), 3); + } + } + + @Test + public void testMergeDlfOssEndpointSpecialCase() { + // Test case 1: dlf.oss-endpoint overrides fs.oss.endpoint when present and non-empty + { + Map targets = new HashMap<>(); + targets.put("fs.oss.endpoint", "original-endpoint"); + targets.put("other.config", "value1"); + Map updates = new HashMap<>(); + updates.put(DLF_OSS_ENDPOINT.key(), "new-oss-endpoint"); + updates.put("other.config", "value2"); // This should not override + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.get("fs.oss.endpoint"), "new-oss-endpoint"); + assertEquals(result.get("other.config"), "value1"); // targets takes precedence + assertEquals(result.size(), 3); + } + + // Test case 2: dlf.oss-endpoint adds fs.oss.endpoint when not present in targets + { + Map targets = new HashMap<>(); + targets.put("other.config", "value1"); + Map updates = new HashMap<>(); + updates.put(DLF_OSS_ENDPOINT.key(), "new-oss-endpoint"); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.get("fs.oss.endpoint"), "new-oss-endpoint"); + assertEquals(result.get("other.config"), "value1"); + assertEquals(result.size(), 3); + } + + // Test case 3: Empty dlf.oss-endpoint should not override + { + Map targets = new HashMap<>(); + targets.put("fs.oss.endpoint", "original-endpoint"); + Map updates = new HashMap<>(); + updates.put(DLF_OSS_ENDPOINT.key(), ""); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.get("fs.oss.endpoint"), "original-endpoint"); + } + + // Test case 4: Null dlf.oss-endpoint should not override + { + Map targets = new HashMap<>(); + targets.put("fs.oss.endpoint", "original-endpoint"); + Map updates = new HashMap<>(); + updates.put(DLF_OSS_ENDPOINT.key(), null); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.get("fs.oss.endpoint"), "original-endpoint"); + } + + // Test case 5: No dlf.oss-endpoint in updates, fs.oss.endpoint unchanged + { + Map targets = new HashMap<>(); + targets.put("fs.oss.endpoint", "original-endpoint"); + Map updates = new HashMap<>(); + updates.put("other.config", "value1"); + Map result = RESTUtil.merge(targets, updates); + assertEquals(result.get("fs.oss.endpoint"), "original-endpoint"); + assertEquals(result.get("other.config"), "value1"); + } + } +} diff --git a/paimon-core/src/test/java/org/apache/paimon/rest/RESTUtilTest.java b/paimon-core/src/test/java/org/apache/paimon/rest/RESTUtilTest.java deleted file mode 100644 index 025610ac5008..000000000000 --- a/paimon-core/src/test/java/org/apache/paimon/rest/RESTUtilTest.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * 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.paimon.rest; - -import org.junit.jupiter.api.Test; - -import java.util.HashMap; -import java.util.Map; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -/** Test for {@link RESTUtil}. */ -public class RESTUtilTest { - @Test - public void testMerge() { - { - Map targets = new HashMap<>(); - targets.put("key1", "default1"); - targets.put("key2", "default2"); - Map updates = new HashMap<>(); - updates.put("key2", "update2"); - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.get("key1"), "default1"); - assertEquals(result.get("key2"), "update2"); - } - { - Map targets = new HashMap<>(); - targets.put("key1", "default1"); - targets.put("key2", "default2"); - Map updates = new HashMap<>(); - updates.put("key1", "default1"); - updates.put("key2", "update2"); - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.get("key1"), "default1"); - assertEquals(result.get("key2"), "update2"); - } - { - Map targets = new HashMap<>(); - targets.put("key1", "default1"); - targets.put("key2", "default2"); - Map updates = new HashMap<>(); - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.get("key1"), "default1"); - assertEquals(result.get("key2"), "default2"); - } - { - Map targets = new HashMap<>(); - Map updates = new HashMap<>(); - updates.put("key2", "update2"); - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.get("key2"), "update2"); - } - { - Map targets = new HashMap<>(); - Map updates = new HashMap<>(); - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.size(), 0); - } - { - Map targets = null; - Map updates = null; - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.size(), 0); - } - { - Map targets = new HashMap<>(); - targets.put("key3", null); - Map updates = new HashMap<>(); - updates.put("key2", null); - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.size(), 0); - } - } -} From 8dfbbc7d6939e822477754bf627b84c3f45bd299 Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Wed, 10 Sep 2025 17:41:39 +0800 Subject: [PATCH 2/4] improve RESTUtil.merge method parameter naming and logic --- .../java/org/apache/paimon/rest/RESTUtil.java | 43 ++++++------ .../org/apache/paimon/utils/RESTUtilTest.java | 70 ++++--------------- .../apache/paimon/rest/RESTTokenFileIO.java | 35 +++++++++- 3 files changed, 67 insertions(+), 81 deletions(-) diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java index 76ec2c99c1b1..06d55bedb85b 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java @@ -42,8 +42,6 @@ import java.nio.charset.StandardCharsets; import java.util.Map; -import static org.apache.paimon.rest.RESTCatalogOptions.DLF_OSS_ENDPOINT; - /** Util for REST. */ public class RESTUtil { @@ -66,38 +64,39 @@ public static Map extractPrefixMap( return result; } + /** + * Merges two string maps with override properties taking precedence over base properties. + * + *

This method combines two maps of string key-value pairs, where the override map's values + * will override any conflicting keys from the base map. Only non-null values are included in + * the final result. + */ public static Map merge( - Map targets, Map updates) { - if (targets == null) { - targets = Maps.newHashMap(); + Map overrideProperties, Map baseProperties) { + if (overrideProperties == null) { + overrideProperties = Maps.newHashMap(); } - if (updates == null) { - updates = Maps.newHashMap(); + if (baseProperties == null) { + baseProperties = Maps.newHashMap(); } - Map result = Maps.newLinkedHashMap(); + ImmutableMap.Builder builder = ImmutableMap.builder(); - // First, add all non-null entries from updates - for (Map.Entry entry : updates.entrySet()) { - if (entry.getValue() != null) { - result.put(entry.getKey(), entry.getValue()); + // First, add all non-null entries from baseProperties that are not in overrideProperties + for (Map.Entry entry : baseProperties.entrySet()) { + if (entry.getValue() != null && !overrideProperties.containsKey(entry.getKey())) { + builder.put(entry.getKey(), entry.getValue()); } } - // Then, add entries from targets, which take precedence (override updates) - for (Map.Entry entry : targets.entrySet()) { + // Then, add all non-null entries from overrideProperties (these take precedence) + for (Map.Entry entry : overrideProperties.entrySet()) { if (entry.getValue() != null) { - result.put(entry.getKey(), entry.getValue()); + builder.put(entry.getKey(), entry.getValue()); } } - // Handle special case: dlf.oss-endpoint should override fs.oss.endpoint - String dlfOssEndpoint = result.get(DLF_OSS_ENDPOINT.key()); - if (dlfOssEndpoint != null && !dlfOssEndpoint.isEmpty()) { - result.put("fs.oss.endpoint", dlfOssEndpoint); - } - - return ImmutableMap.copyOf(result); + return builder.build(); } public static String encodeString(String toEncode) { diff --git a/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java b/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java index 9cd2cff30880..a486bf055561 100644 --- a/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java +++ b/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java @@ -121,62 +121,18 @@ public void testMerge() { } @Test - public void testMergeDlfOssEndpointSpecialCase() { - // Test case 1: dlf.oss-endpoint overrides fs.oss.endpoint when present and non-empty - { - Map targets = new HashMap<>(); - targets.put("fs.oss.endpoint", "original-endpoint"); - targets.put("other.config", "value1"); - Map updates = new HashMap<>(); - updates.put(DLF_OSS_ENDPOINT.key(), "new-oss-endpoint"); - updates.put("other.config", "value2"); // This should not override - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.get("fs.oss.endpoint"), "new-oss-endpoint"); - assertEquals(result.get("other.config"), "value1"); // targets takes precedence - assertEquals(result.size(), 3); - } - - // Test case 2: dlf.oss-endpoint adds fs.oss.endpoint when not present in targets - { - Map targets = new HashMap<>(); - targets.put("other.config", "value1"); - Map updates = new HashMap<>(); - updates.put(DLF_OSS_ENDPOINT.key(), "new-oss-endpoint"); - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.get("fs.oss.endpoint"), "new-oss-endpoint"); - assertEquals(result.get("other.config"), "value1"); - assertEquals(result.size(), 3); - } - - // Test case 3: Empty dlf.oss-endpoint should not override - { - Map targets = new HashMap<>(); - targets.put("fs.oss.endpoint", "original-endpoint"); - Map updates = new HashMap<>(); - updates.put(DLF_OSS_ENDPOINT.key(), ""); - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.get("fs.oss.endpoint"), "original-endpoint"); - } - - // Test case 4: Null dlf.oss-endpoint should not override - { - Map targets = new HashMap<>(); - targets.put("fs.oss.endpoint", "original-endpoint"); - Map updates = new HashMap<>(); - updates.put(DLF_OSS_ENDPOINT.key(), null); - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.get("fs.oss.endpoint"), "original-endpoint"); - } - - // Test case 5: No dlf.oss-endpoint in updates, fs.oss.endpoint unchanged - { - Map targets = new HashMap<>(); - targets.put("fs.oss.endpoint", "original-endpoint"); - Map updates = new HashMap<>(); - updates.put("other.config", "value1"); - Map result = RESTUtil.merge(targets, updates); - assertEquals(result.get("fs.oss.endpoint"), "original-endpoint"); - assertEquals(result.get("other.config"), "value1"); - } + public void testMergeWithoutDlfOssEndpointHandling() { + Map targets = new HashMap<>(); + targets.put("fs.oss.endpoint", "original-endpoint"); + targets.put("other.config", "value1"); + Map updates = new HashMap<>(); + updates.put(DLF_OSS_ENDPOINT.key(), "new-oss-endpoint"); + updates.put("other.config", "value2"); // This should not override + Map result = RESTUtil.merge(targets, updates); + // fs.oss.endpoint should remain unchanged as RESTUtil.merge no longer handles this + assertEquals(result.get("fs.oss.endpoint"), "original-endpoint"); + assertEquals(result.get("other.config"), "value1"); // targets takes precedence + assertEquals(result.get(DLF_OSS_ENDPOINT.key()), "new-oss-endpoint"); + assertEquals(result.size(), 3); } } diff --git a/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java b/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java index fc4134c8b8fa..80d037e23bb6 100644 --- a/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java +++ b/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java @@ -35,17 +35,21 @@ import org.apache.paimon.shade.caffeine2.com.github.benmanes.caffeine.cache.Cache; import org.apache.paimon.shade.caffeine2.com.github.benmanes.caffeine.cache.Caffeine; import org.apache.paimon.shade.caffeine2.com.github.benmanes.caffeine.cache.Scheduler; +import org.apache.paimon.shade.guava30.com.google.common.collect.ImmutableMap; +import org.apache.paimon.shade.guava30.com.google.common.collect.Maps; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.UncheckedIOException; +import java.util.Map; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import static org.apache.paimon.options.CatalogOptions.FILE_IO_ALLOW_CACHE; import static org.apache.paimon.rest.RESTApi.TOKEN_EXPIRATION_SAFE_TIME_MILLIS; +import static org.apache.paimon.rest.RESTCatalogOptions.DLF_OSS_ENDPOINT; /** A {@link FileIO} to support getting token from REST Server. */ public class RESTTokenFileIO implements FileIO { @@ -162,8 +166,8 @@ public FileIO fileIO() throws IOException { } Options options = catalogContext.options(); - // the original options are not overwritten - options = new Options(RESTUtil.merge(token.token(), options.toMap())); + options = + new Options(mergeTokenWithDlfEndpointHandling(token.token(), options.toMap())); options.set(FILE_IO_ALLOW_CACHE, false); CatalogContext context = CatalogContext.create( @@ -211,6 +215,33 @@ private void refreshToken() { token = new RESTToken(response.getToken(), response.getExpiresAtMillis()); } + /** + * Merges token properties with catalog properties and handles DLF OSS endpoint configuration. + * + *

This method performs the same merge logic as {@link RESTUtil#merge(Map, Map)} but also + * handles the special case where the DLF OSS endpoint should override the standard OSS + * endpoint. When 'dlf.oss-endpoint' is present in the merged properties, it will be used to set + * 'fs.oss.endpoint' for OSS file system configuration. + * + * @param restTokenProperties the properties from the REST token + * @param catalogProperties the catalog properties to merge with + * @return merged properties with DLF OSS endpoint handling applied + */ + private Map mergeTokenWithDlfEndpointHandling( + Map restTokenProperties, Map catalogProperties) { + // Use RESTUtil.merge for the basic merge logic + Map result = + Maps.newLinkedHashMap(RESTUtil.merge(catalogProperties, restTokenProperties)); + + // Handle special case: dlf.oss-endpoint should override fs.oss.endpoint + String dlfOssEndpoint = result.get(DLF_OSS_ENDPOINT.key()); + if (dlfOssEndpoint != null && !dlfOssEndpoint.isEmpty()) { + result.put("fs.oss.endpoint", dlfOssEndpoint); + } + + return ImmutableMap.copyOf(result); + } + /** * Public interface to get valid token, this can be invoked by native engines to get the token * and use own File System. From 09009f1ada60f0234cb27c20d93bac7dfc40cb9d Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Wed, 10 Sep 2025 20:12:06 +0800 Subject: [PATCH 3/4] update. --- .../java/org/apache/paimon/rest/RESTUtil.java | 2 +- .../org/apache/paimon/utils/RESTUtilTest.java | 29 +--- .../apache/paimon/rest/RESTTokenFileIO.java | 6 +- .../paimon/rest/RESTTokenFileIOTest.java | 144 ++++++++++++++++++ 4 files changed, 155 insertions(+), 26 deletions(-) create mode 100644 paimon-common/src/test/java/org/apache/paimon/rest/RESTTokenFileIOTest.java diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java index 06d55bedb85b..5872af209154 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java @@ -72,7 +72,7 @@ public static Map extractPrefixMap( * the final result. */ public static Map merge( - Map overrideProperties, Map baseProperties) { + Map baseProperties, Map overrideProperties) { if (overrideProperties == null) { overrideProperties = Maps.newHashMap(); } diff --git a/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java b/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java index a486bf055561..86849f9f62dc 100644 --- a/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java +++ b/paimon-api/src/test/java/org/apache/paimon/utils/RESTUtilTest.java @@ -25,14 +25,13 @@ import java.util.HashMap; import java.util.Map; -import static org.apache.paimon.rest.RESTCatalogOptions.DLF_OSS_ENDPOINT; import static org.junit.jupiter.api.Assertions.assertEquals; /** Test for {@link RESTUtil}. */ public class RESTUtilTest { @Test public void testMerge() { - // Test case 1: targets has precedence over updates for existing keys + // Test case 1: updates has precedence over targets for existing keys { Map targets = new HashMap<>(); targets.put("key1", "default1"); @@ -41,11 +40,11 @@ public void testMerge() { updates.put("key2", "update2"); Map result = RESTUtil.merge(targets, updates); assertEquals(result.get("key1"), "default1"); - // key2 should keep targets value, not be overridden by updates - assertEquals(result.get("key2"), "default2"); + // key2 should be overridden by updates value + assertEquals(result.get("key2"), "update2"); } - // Test case 2: targets has precedence even when updates has same value + // Test case 2: updates has precedence even when targets has same value { Map targets = new HashMap<>(); targets.put("key1", "default1"); @@ -55,8 +54,8 @@ public void testMerge() { updates.put("key2", "update2"); Map result = RESTUtil.merge(targets, updates); assertEquals(result.get("key1"), "default1"); - // key2 should keep targets value - assertEquals(result.get("key2"), "default2"); + // key2 should be overridden by updates value + assertEquals(result.get("key2"), "update2"); } // Test case 3: empty updates, targets unchanged @@ -119,20 +118,4 @@ public void testMerge() { assertEquals(result.size(), 3); } } - - @Test - public void testMergeWithoutDlfOssEndpointHandling() { - Map targets = new HashMap<>(); - targets.put("fs.oss.endpoint", "original-endpoint"); - targets.put("other.config", "value1"); - Map updates = new HashMap<>(); - updates.put(DLF_OSS_ENDPOINT.key(), "new-oss-endpoint"); - updates.put("other.config", "value2"); // This should not override - Map result = RESTUtil.merge(targets, updates); - // fs.oss.endpoint should remain unchanged as RESTUtil.merge no longer handles this - assertEquals(result.get("fs.oss.endpoint"), "original-endpoint"); - assertEquals(result.get("other.config"), "value1"); // targets takes precedence - assertEquals(result.get(DLF_OSS_ENDPOINT.key()), "new-oss-endpoint"); - assertEquals(result.size(), 3); - } } diff --git a/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java b/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java index 80d037e23bb6..ad084d23e2e7 100644 --- a/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java +++ b/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java @@ -167,7 +167,9 @@ public FileIO fileIO() throws IOException { Options options = catalogContext.options(); options = - new Options(mergeTokenWithDlfEndpointHandling(token.token(), options.toMap())); + new Options( + RESTTokenFileIO.mergeTokenWithDlfEndpointHandling( + token.token(), options.toMap())); options.set(FILE_IO_ALLOW_CACHE, false); CatalogContext context = CatalogContext.create( @@ -227,7 +229,7 @@ private void refreshToken() { * @param catalogProperties the catalog properties to merge with * @return merged properties with DLF OSS endpoint handling applied */ - private Map mergeTokenWithDlfEndpointHandling( + public static Map mergeTokenWithDlfEndpointHandling( Map restTokenProperties, Map catalogProperties) { // Use RESTUtil.merge for the basic merge logic Map result = diff --git a/paimon-common/src/test/java/org/apache/paimon/rest/RESTTokenFileIOTest.java b/paimon-common/src/test/java/org/apache/paimon/rest/RESTTokenFileIOTest.java new file mode 100644 index 000000000000..a5133546e22a --- /dev/null +++ b/paimon-common/src/test/java/org/apache/paimon/rest/RESTTokenFileIOTest.java @@ -0,0 +1,144 @@ +/* + * 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.paimon.rest; + +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.Map; + +import static org.apache.paimon.rest.RESTCatalogOptions.DLF_OSS_ENDPOINT; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** Test for {@link RESTTokenFileIO}. */ +public class RESTTokenFileIOTest { + @Test + public void testMergeTokenWithDlfEndpointHandling() { + // Test case 1: dlf.oss-endpoint overrides fs.oss.endpoint when present and non-empty + { + Map catalogProperties = new HashMap<>(); + catalogProperties.put("fs.oss.endpoint", "original-endpoint"); + catalogProperties.put("other.config", "value1"); + Map restTokenProperties = new HashMap<>(); + restTokenProperties.put(DLF_OSS_ENDPOINT.key(), "new-oss-endpoint"); + restTokenProperties.put("other.config", "value2"); // This should override + Map result = + RESTTokenFileIO.mergeTokenWithDlfEndpointHandling( + catalogProperties, restTokenProperties); + assertEquals("new-oss-endpoint", result.get("fs.oss.endpoint")); + assertEquals( + "value2", result.get("other.config")); // restTokenProperties takes precedence + assertEquals("new-oss-endpoint", result.get(DLF_OSS_ENDPOINT.key())); + assertEquals(3, result.size()); + } + + // Test case 2: dlf.oss-endpoint adds fs.oss.endpoint when not present in catalogProperties + { + Map catalogProperties = new HashMap<>(); + catalogProperties.put("other.config", "value1"); + Map restTokenProperties = new HashMap<>(); + restTokenProperties.put(DLF_OSS_ENDPOINT.key(), "new-oss-endpoint"); + Map result = + RESTTokenFileIO.mergeTokenWithDlfEndpointHandling( + catalogProperties, restTokenProperties); + assertEquals("new-oss-endpoint", result.get("fs.oss.endpoint")); + assertEquals("value1", result.get("other.config")); + assertEquals("new-oss-endpoint", result.get(DLF_OSS_ENDPOINT.key())); + assertEquals(3, result.size()); + } + + // Test case 3: Empty dlf.oss-endpoint should not override + { + Map catalogProperties = new HashMap<>(); + catalogProperties.put("fs.oss.endpoint", "original-endpoint"); + Map restTokenProperties = new HashMap<>(); + restTokenProperties.put(DLF_OSS_ENDPOINT.key(), ""); + Map result = + RESTTokenFileIO.mergeTokenWithDlfEndpointHandling( + catalogProperties, restTokenProperties); + assertEquals("original-endpoint", result.get("fs.oss.endpoint")); + assertEquals( + 2, result.size()); // fs.oss.endpoint and dlf.oss-endpoint (empty string is not + // null) + } + + // Test case 4: Null dlf.oss-endpoint should not override + { + Map catalogProperties = new HashMap<>(); + catalogProperties.put("fs.oss.endpoint", "original-endpoint"); + Map restTokenProperties = new HashMap<>(); + restTokenProperties.put(DLF_OSS_ENDPOINT.key(), null); + Map result = + RESTTokenFileIO.mergeTokenWithDlfEndpointHandling( + catalogProperties, restTokenProperties); + assertEquals("original-endpoint", result.get("fs.oss.endpoint")); + assertEquals(1, result.size()); // Only fs.oss.endpoint (null values are filtered out) + } + + // Test case 5: No dlf.oss-endpoint in restTokenProperties, fs.oss.endpoint unchanged + { + Map catalogProperties = new HashMap<>(); + catalogProperties.put("fs.oss.endpoint", "original-endpoint"); + Map restTokenProperties = new HashMap<>(); + restTokenProperties.put("other.config", "value1"); + Map result = + RESTTokenFileIO.mergeTokenWithDlfEndpointHandling( + catalogProperties, restTokenProperties); + assertEquals("original-endpoint", result.get("fs.oss.endpoint")); + assertEquals("value1", result.get("other.config")); + assertEquals(2, result.size()); + } + + // Test case 6: Both empty maps + { + Map catalogProperties = new HashMap<>(); + Map restTokenProperties = new HashMap<>(); + Map result = + RESTTokenFileIO.mergeTokenWithDlfEndpointHandling( + catalogProperties, restTokenProperties); + assertEquals(0, result.size()); + } + + // Test case 7: Both null maps + { + Map result = + RESTTokenFileIO.mergeTokenWithDlfEndpointHandling(null, null); + assertEquals(0, result.size()); + } + + // Test case 8: restTokenProperties adds new keys that don't exist in catalogProperties + { + Map catalogProperties = new HashMap<>(); + catalogProperties.put("key1", "catalog1"); + Map restTokenProperties = new HashMap<>(); + restTokenProperties.put("key2", "token2"); + restTokenProperties.put("key3", "token3"); + restTokenProperties.put(DLF_OSS_ENDPOINT.key(), "dlf-endpoint"); + Map result = + RESTTokenFileIO.mergeTokenWithDlfEndpointHandling( + catalogProperties, restTokenProperties); + assertEquals("catalog1", result.get("key1")); + assertEquals("token2", result.get("key2")); + assertEquals("token3", result.get("key3")); + assertEquals("dlf-endpoint", result.get(DLF_OSS_ENDPOINT.key())); + assertEquals("dlf-endpoint", result.get("fs.oss.endpoint")); // DLF endpoint handling + assertEquals(5, result.size()); + } + } +} From 9d2866771cf6107d26326993f722bee2ac0c9b65 Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Wed, 10 Sep 2025 21:13:33 +0800 Subject: [PATCH 4/4] fix test case. --- .../src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java b/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java index ad084d23e2e7..02fa24e1a642 100644 --- a/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java +++ b/paimon-common/src/main/java/org/apache/paimon/rest/RESTTokenFileIO.java @@ -169,7 +169,7 @@ public FileIO fileIO() throws IOException { options = new Options( RESTTokenFileIO.mergeTokenWithDlfEndpointHandling( - token.token(), options.toMap())); + options.toMap(), token.token())); options.set(FILE_IO_ALLOW_CACHE, false); CatalogContext context = CatalogContext.create( @@ -230,7 +230,7 @@ private void refreshToken() { * @return merged properties with DLF OSS endpoint handling applied */ public static Map mergeTokenWithDlfEndpointHandling( - Map restTokenProperties, Map catalogProperties) { + Map catalogProperties, Map restTokenProperties) { // Use RESTUtil.merge for the basic merge logic Map result = Maps.newLinkedHashMap(RESTUtil.merge(catalogProperties, restTokenProperties));