Copilot commented on code in PR #3309:
URL: https://github.com/apache/hugegraph/pull/3309#discussion_r4246874333


##########
hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/rocksdb/ZipUtilsPathSafetyTest.java:
##########
@@ -0,0 +1,156 @@
+/*
+ * 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.hugegraph.store.rocksdb;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.zip.CRC32;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+
+import org.apache.hugegraph.rocksdb.access.util.CRC64;
+import org.apache.hugegraph.rocksdb.access.util.ZipUtils;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+/**
+ * Snapshot archives are produced and unpacked by two ZipUtils copies: the 
RocksDB one used for
+ * partition snapshots and the core one. Both must keep extracted files inside 
the target dir.
+ */
+public class ZipUtilsPathSafetyTest {
+
+    @Rule
+    public TemporaryFolder folder = new TemporaryFolder();
+
+    private static void write(File file, String content) throws IOException {
+        Files.createDirectories(file.getParentFile().toPath());
+        Files.write(file.toPath(), content.getBytes(StandardCharsets.UTF_8));
+    }
+
+    private static String read(File file) throws IOException {
+        return new String(Files.readAllBytes(file.toPath()), 
StandardCharsets.UTF_8);
+    }
+
+    private File maliciousZip(String... entryNames) throws IOException {
+        File zip = this.folder.newFile("malicious.zip");
+        try (ZipOutputStream zos = new ZipOutputStream(new 
FileOutputStream(zip))) {
+            zos.putNextEntry(new ZipEntry("snapshot/ok.sst"));
+            zos.write("ok".getBytes(StandardCharsets.UTF_8));
+            zos.closeEntry();
+            for (String name : entryNames) {
+                zos.putNextEntry(new ZipEntry(name));
+                zos.write("pwned".getBytes(StandardCharsets.UTF_8));
+                zos.closeEntry();
+            }
+        }
+        return zip;
+    }
+
+    @Test
+    public void testRoundTripKeepsNestedLayoutAndChecksumsAgree() throws 
IOException {
+        File root = this.folder.newFolder("root");
+        write(new File(root, "snapshot/CURRENT"), "MANIFEST-000001");
+        write(new File(root, "snapshot/sub/dir/000004.sst"), "sst-bytes");
+        Files.createDirectories(new File(root, "snapshot/empty").toPath());
+        File zip = new File(this.folder.getRoot(), "snapshot.zip");
+        File out = this.folder.newFolder("out");
+
+        CRC64 written = new CRC64();
+        ZipUtils.compress(root.getAbsolutePath(), "snapshot", 
zip.getAbsolutePath(), written);
+        CRC64 read = new CRC64();
+        ZipUtils.decompress(zip.getAbsolutePath(), out.getAbsolutePath(), 
read);
+
+        assertEquals("MANIFEST-000001", read(new File(out, 
"snapshot/CURRENT")));
+        assertEquals("sst-bytes", read(new File(out, 
"snapshot/sub/dir/000004.sst")));
+        // The checksum covers the whole archive on both sides, so it detects 
transfer damage
+        assertTrue(written.getValue() != 0L);
+        assertEquals(written.getValue(), read.getValue());
+
+        File coreOut = this.folder.newFolder("core-out");
+        CRC32 coreRead = new CRC32();
+        
org.apache.hugegraph.store.util.ZipUtils.decompress(zip.getAbsolutePath(),
+                                                            
coreOut.getAbsolutePath(), coreRead);
+        assertArrayEquals(Files.readAllBytes(new File(out, 
"snapshot/sub/dir/000004.sst").toPath()),
+                          Files.readAllBytes(new File(coreOut, 
"snapshot/sub/dir/000004.sst").toPath()));
+    }
+
+    @Test
+    public void testRocksDbDecompressSkipsEntriesEscapingTheTargetDir() throws 
IOException {
+        File out = this.folder.newFolder("out");
+        File zip = maliciousZip("../escaped.txt", 
"snapshot/../../escaped2.txt", "../out-evil/x.txt",
+                                new File(this.folder.getRoot(), 
"absolute.txt").getAbsolutePath());

Review Comment:
   The “absolute path” zip entry name is built using `File#getAbsolutePath()`, 
which is OS-dependent (e.g., Windows drive letters and backslashes). That can 
make this security test non-portable or fail to exercise the intended “absolute 
path zip-slip” branch consistently. Consider generating an absolute-style entry 
name in a platform-agnostic way (or branching the assertion per OS), and ensure 
the entry name uses zip-standard `/` separators.



##########
hugegraph-commons/hugegraph-common/src/test/java/org/apache/hugegraph/unit/config/HugeConfigValidationTest.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.hugegraph.unit.config;
+
+import static org.apache.hugegraph.config.OptionChecker.disallowEmpty;
+import static org.apache.hugegraph.config.OptionChecker.rangeInt;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.commons.configuration2.Configuration;
+import org.apache.hugegraph.config.ConfigException;
+import org.apache.hugegraph.config.ConfigListOption;
+import org.apache.hugegraph.config.ConfigOption;
+import org.apache.hugegraph.config.HugeConfig;
+import org.apache.hugegraph.config.OptionHolder;
+import org.apache.hugegraph.config.OptionSpace;
+import org.apache.hugegraph.testutil.Assert;
+import org.apache.hugegraph.unit.BaseUnitTest;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+
+public class HugeConfigValidationTest extends BaseUnitTest {
+
+    @BeforeClass
+    public static void init() {
+        OptionSpace.register("validation-test", 
ValidationOptions.class.getName());

Review Comment:
   `OptionSpace.register(...)` is called unconditionally. If other tests (or 
re-runs in the same JVM) register the same option space name, this can cause 
cross-test interference and failures that depend on execution order. Guard the 
registration (similar to how RocksDB options registration is guarded elsewhere) 
or use a unique namespace per test class run.



##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/rocksdb/RocksDBStdSessionsLifecycleTest.java:
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.hugegraph.unit.rocksdb;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+
+import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.hugegraph.backend.store.BackendEntry.BackendColumnIterator;
+import org.apache.hugegraph.backend.store.rocksdb.RocksDBOptions;
+import org.apache.hugegraph.backend.store.rocksdb.RocksDBSessions;
+import org.apache.hugegraph.backend.store.rocksdb.RocksDBSessions.Session;
+import org.apache.hugegraph.backend.store.rocksdb.RocksDBStdSessions;
+import org.apache.hugegraph.config.HugeConfig;
+import org.apache.hugegraph.config.OptionSpace;
+import org.apache.hugegraph.exception.BackendException;
+import org.apache.hugegraph.testutil.Assert;
+import org.apache.hugegraph.unit.BaseUnitTest;
+import org.apache.hugegraph.unit.FakeObjects;
+import org.junit.After;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.rocksdb.RocksDBException;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+/**
+ * Lifecycle and negative-path tests for {@link RocksDBStdSessions}. Every test
+ * uses its own temporary directory so it never shares the fixed path used by
+ * {@link BaseRocksDBUnitTest}.
+ */
+public class RocksDBStdSessionsLifecycleTest extends BaseUnitTest {
+
+    private static final String TABLE = "lifecycle-table";
+
+    @Rule
+    public TemporaryFolder temporary = new TemporaryFolder();
+
+    private final List<RocksDBSessions> opened = new ArrayList<>();
+
+    @BeforeClass
+    public static void registerRocksDBOptions() {
+        // Typed values in HugeConfig require the RocksDB options to be known
+        if (!OptionSpace.containKey(RocksDBOptions.NUM_LEVELS.name())) {
+            OptionSpace.register("rocksdb", RocksDBOptions.class.getName());
+        }
+    }
+
+    @After
+    public void closeOpenedSessions() {
+        for (RocksDBSessions sessions : this.opened) {
+            try {
+                // Closes this thread's session and, when it is the last one, 
the DB
+                sessions.close();
+            } finally {
+                if (sessions.databaseOpened()) {
+                    sessions.forceCloseRocksDB();
+                }
+            }
+        }
+        this.opened.clear();
+    }
+
+    @Test
+    public void testOperationsOnUnopenedTableAreRejected() throws Exception {
+        RocksDBSessions sessions = this.open(this.newPath("unopened"));
+        Session session = sessions.session();
+        byte[] key = getBytes("k");
+
+        Assert.assertThrows(BackendException.class, () -> {
+            session.put("missing", key, getBytes("v"));
+        }, e -> {
+            Assert.assertContains("Table 'missing' is not opened", 
e.getMessage());
+        });
+        Assert.assertFalse(session.hasChanges());
+
+        Assert.assertThrows(BackendException.class, () -> {
+            session.get("missing", key);
+        }, e -> {
+            Assert.assertContains("Table 'missing' is not opened", 
e.getMessage());
+        });
+        Assert.assertThrows(BackendException.class, () -> {
+            session.scan("missing");
+        }, e -> {
+            Assert.assertContains("Table 'missing' is not opened", 
e.getMessage());
+        });
+        Assert.assertThrows(BackendException.class, () -> {
+            session.delete("missing", key);
+        }, e -> {
+            Assert.assertContains("Table 'missing' is not opened", 
e.getMessage());
+        });
+        Assert.assertThrows(BackendException.class, () -> {
+            session.keyRange("missing");
+        }, e -> {
+            Assert.assertContains("Table 'missing' is not opened", 
e.getMessage());
+        });
+
+        // The opened table is still fully usable after the rejected calls
+        session.put(TABLE, key, getBytes("v"));
+        Assert.assertEquals(1, (int) session.commit());
+        Assert.assertEquals("v", getString(session.get(TABLE, key)));
+    }
+
+    @Test
+    public void testCreateTableTwiceKeepsDataAndDropMissingTableIsNoop() 
throws Exception {
+        RocksDBSessions sessions = this.open(this.newPath("idempotent"));
+        Session session = sessions.session();
+        session.put(TABLE, getBytes("k"), getBytes("v"));
+        session.commit();
+
+        // Re-creating an existing table must not truncate or replace it
+        sessions.createTable(TABLE);
+        Assert.assertEquals("v", getString(session.get(TABLE, getBytes("k"))));
+
+        // Dropping a table that never existed leaves the opened tables intact
+        sessions.dropTable("never-created");
+        Assert.assertTrue(sessions.existsTable(TABLE));
+        Assert.assertFalse(sessions.existsTable("never-created"));
+
+        sessions.dropTable(TABLE);
+        Assert.assertFalse(sessions.existsTable(TABLE));
+        // Dropping again after the table is gone is still a no-op
+        sessions.dropTable(TABLE);
+        Assert.assertFalse(sessions.openedTables().contains(TABLE));
+    }
+
+    @Test
+    public void testRollbackDiscardsPendingWrites() throws Exception {
+        RocksDBSessions sessions = this.open(this.newPath("rollback"));
+        Session session = sessions.session();
+
+        Assert.assertEquals(0, (int) session.commit());
+        session.put(TABLE, getBytes("k2"), getBytes("v2"));
+        Assert.assertEquals(1, (int) session.commit());
+
+        session.put(TABLE, getBytes("k1"), getBytes("v1"));
+        session.delete(TABLE, getBytes("k2"));
+        Assert.assertTrue(session.hasChanges());
+
+        session.rollback();
+        Assert.assertFalse(session.hasChanges());
+        Assert.assertEquals(0, (int) session.commit());
+        // Neither the pending put nor the pending delete reached the DB
+        Assert.assertNull(session.get(TABLE, getBytes("k1")));
+        Assert.assertEquals("v2", getString(session.get(TABLE, 
getBytes("k2"))));
+    }
+
+    @Test
+    public void testCommittedDataSurvivesReopenButPendingWritesDoNot() throws 
Exception {
+        String path = this.newPath("reopen");
+        RocksDBSessions sessions = this.open(path);
+        Session session = sessions.session();
+        session.put(TABLE, getBytes("committed"), getBytes("v1"));
+        session.commit();
+        session.put(TABLE, getBytes("pending"), getBytes("v2"));
+        Assert.assertTrue(session.hasChanges());
+
+        Assert.assertTrue(sessions.close());
+        Assert.assertFalse(sessions.databaseOpened());
+
+        // Reopen without createTable(): existing column families are reloaded
+        RocksDBSessions reopened = this.openWithoutTable(path);
+        Assert.assertTrue(reopened.existsTable(TABLE));
+        Session session2 = reopened.session();
+        Assert.assertFalse(session2.hasChanges());
+        Assert.assertEquals("v1", getString(session2.get(TABLE, 
getBytes("committed"))));
+        Assert.assertNull(session2.get(TABLE, getBytes("pending")));
+    }
+
+    @Test
+    public void testLiveDatabaseCannotBeOpenedTwiceUntilClosed() throws 
Exception {
+        String path = this.newPath("exclusive");
+        RocksDBSessions sessions = this.open(path);
+        sessions.session().put(TABLE, getBytes("k"), getBytes("v"));
+        sessions.session().commit();
+
+        Assert.assertThrows(RocksDBException.class, () -> {
+            this.openWithoutTable(path);
+        }, e -> {
+            Assert.assertContains("lock", e.getMessage());
+        });
+        // The failed open must not disturb the live instance
+        Assert.assertTrue(sessions.databaseOpened());
+        Assert.assertEquals("v", getString(sessions.session().get(TABLE, 
getBytes("k"))));
+
+        Assert.assertTrue(sessions.close());
+        RocksDBSessions reopened = this.openWithoutTable(path);
+        Assert.assertEquals("v", getString(reopened.session().get(TABLE, 
getBytes("k"))));
+    }
+
+    @Test
+    public void testClosedDatabaseRejectsTableAndSessionAccess() throws 
Exception {
+        String path = this.newPath("closed");
+        RocksDBSessions sessions = this.open(path);
+        sessions.forceCloseRocksDB();
+        Assert.assertFalse(sessions.databaseOpened());
+
+        Assert.assertThrows(IllegalStateException.class, () -> {
+            sessions.createTable("another-table");
+        }, e -> {
+            Assert.assertContains("RocksDB has been closed", e.getMessage());
+        });
+        Assert.assertThrows(IllegalStateException.class, () -> {
+            sessions.dropTable(TABLE);
+        }, e -> {
+            Assert.assertContains("RocksDB has been closed", e.getMessage());
+        });
+        Assert.assertThrows(IllegalStateException.class, 
sessions::compactRange, e -> {
+            Assert.assertContains("RocksDB has been closed", e.getMessage());
+        });
+        Assert.assertThrows(IllegalStateException.class, sessions::session, e 
-> {
+            Assert.assertContains("session pool is closed", e.getMessage());
+        });
+
+        // Force close must release the database lock for a later owner
+        RocksDBSessions reopened = this.openWithoutTable(path);
+        Assert.assertTrue(reopened.databaseOpened());
+        Assert.assertEquals(ImmutableSet.of("default", TABLE), 
reopened.openedTables());
+    }
+
+    @Test
+    public void testScanIteratorCloseIsIdempotentAndStopsIteration() throws 
Exception {
+        RocksDBSessions sessions = this.open(this.newPath("scan-close"));
+        Session session = sessions.session();
+        session.put(TABLE, getBytes("a1"), getBytes("1"));
+        session.put(TABLE, getBytes("a2"), getBytes("2"));
+        session.put(TABLE, getBytes("b1"), getBytes("3"));
+        session.commit();
+
+        BackendColumnIterator iter = session.scan(TABLE, getBytes("a"));
+        Assert.assertTrue(iter.hasNext());
+        Assert.assertEquals("a1", getString(iter.next().name));
+        Assert.assertEquals("a1", getString(iter.position()));
+
+        // Closing early releases the native iterator; further reads end 
cleanly
+        iter.close();
+        iter.close();
+        Assert.assertFalse(iter.hasNext());
+        Assert.assertThrows(NoSuchElementException.class, iter::next);
+
+        // A fully consumed iterator clears its paging position
+        BackendColumnIterator all = session.scan(TABLE, getBytes("a"));
+        Assert.assertEquals("a1", getString(all.next().name));
+        Assert.assertEquals("a2", getString(all.next().name));
+        Assert.assertFalse(all.hasNext());
+        Assert.assertNull(all.position());
+        all.close();
+    }
+
+    @Test
+    public void testMismatchedLevelCompressionsRejectedAndLockReleased() 
throws Exception {
+        String path = this.newPath("bad-options");
+        PropertiesConfiguration conf = new PropertiesConfiguration();
+        conf.setProperty(RocksDBOptions.NUM_LEVELS.name(), "3");
+        conf.setProperty(RocksDBOptions.LEVELS_COMPRESSIONS.name(), "[none, 
snappy]");
+        HugeConfig badConfig = new HugeConfig(conf);
+
+        Assert.assertThrows(IllegalArgumentException.class, () -> {
+            new RocksDBStdSessions(badConfig, "db", "store", path, path);
+        }, e -> {
+            Assert.assertContains(RocksDBOptions.LEVELS_COMPRESSIONS.name(), 
e.getMessage());
+            Assert.assertContains("but got 2 != 3", e.getMessage());
+        });
+
+        // The failed open must not leave the database lock behind
+        RocksDBSessions sessions = this.open(path);
+        Assert.assertTrue(sessions.databaseOpened());
+    }
+
+    @Test
+    public void testBuildSnapshotPathRequiresExistingSnapshot() throws 
Exception {
+        File parent = this.temporary.newFolder("graph");
+        File dataDir = new File(parent, "rocksdb-data/g");
+        Assert.assertTrue(dataDir.getParentFile().mkdirs());
+        String dataPath = dataDir.getAbsolutePath();
+        RocksDBSessions sessions = this.open(dataPath);
+
+        File expected = new File(parent, "snapshot_rocksdb-data/g");
+        Assert.assertThrows(IllegalArgumentException.class, () -> {
+            sessions.buildSnapshotPath("snapshot");
+        }, e -> {
+            Assert.assertContains("doesn't exist", e.getMessage());
+            Assert.assertContains(expected.getPath(), e.getMessage());
+        });
+
+        Assert.assertTrue(expected.mkdirs());
+        Assert.assertEquals(expected.getPath(), 
sessions.buildSnapshotPath("snapshot"));
+    }
+
+    private String newPath(String name) throws Exception {
+        return new File(this.temporary.newFolder(name), 
"db").getAbsolutePath();
+    }
+
+    private RocksDBSessions open(String path) throws RocksDBException {
+        RocksDBSessions sessions = this.openWithoutTable(path);
+        sessions.createTable(TABLE);
+        return sessions;
+    }
+
+    private RocksDBSessions openWithoutTable(String path) throws 
RocksDBException {
+        // Passing no CF names still reopens every column family found on disk
+        RocksDBSessions sessions = new 
RocksDBStdSessions(FakeObjects.newConfig(), "db",
+                                                          "store", path, path,
+                                                          ImmutableList.of());
+        this.opened.add(sessions);
+        return sessions;
+    }
+
+    private static byte[] getBytes(String str) {
+        return str.getBytes();
+    }
+
+    private static String getString(byte[] bytes) {
+        return bytes == null ? null : new String(bytes);
+    }

Review Comment:
   `String.getBytes()` / `new String(bytes)` use the platform default charset, 
which can make these tests fail on systems not using UTF-8 (or behave 
inconsistently across CI agents). Use an explicit charset (e.g., UTF_8) for 
both encoding and decoding to keep the test deterministic.



##########
hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/RpcConfigTest.java:
##########
@@ -0,0 +1,246 @@
+/*
+ * 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.hugegraph.unit;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.hugegraph.config.ConfigException;
+import org.apache.hugegraph.config.HugeConfig;
+import org.apache.hugegraph.config.RpcOptions;
+import org.apache.hugegraph.rpc.RpcClientProvider;
+import org.apache.hugegraph.rpc.RpcConsumerConfig;
+import org.apache.hugegraph.rpc.RpcProviderConfig;
+import org.apache.hugegraph.rpc.RpcServer;
+import org.apache.hugegraph.testutil.Assert;
+import org.apache.hugegraph.testutil.Whitebox;
+import org.apache.hugegraph.unit.ServerClientTest.HelloService;
+import org.apache.hugegraph.unit.ServerClientTest.HelloServiceImpl;
+import org.junit.Test;
+
+import com.alipay.sofa.rpc.config.ConsumerConfig;
+import com.alipay.sofa.rpc.config.ProviderConfig;
+
+public class RpcConfigTest extends BaseUnitTest {
+
+    private static final String HELLO = HelloService.class.getName();
+
+    @Test
+    public void testExcludeSelfUrl() {
+        Assert.assertEquals("127.0.0.1:8092,127.0.0.1:8093",
+                            
excludeSelfUrl("127.0.0.1:8091,127.0.0.1:8092;127.0.0.1:8093",
+                                           "127.0.0.1:8091"));
+        // Order is kept and duplicates are collapsed
+        Assert.assertEquals("b:2,a:1",
+                            excludeSelfUrl("b:2,a:1,b:2,self:0", "self:0"));
+        Assert.assertEquals("a:1", excludeSelfUrl("a:1", "self:0"));
+        Assert.assertEquals("", excludeSelfUrl("self:0,self:0", "self:0"));
+        Assert.assertEquals("", excludeSelfUrl("", "self:0"));
+    }
+
+    @Test
+    public void testClientDisabledWhenOnlySelfIsRemote() {
+        Map<String, Object> options = new HashMap<>();
+        options.put(RpcOptions.RPC_SERVER_HOST.name(), "127.0.0.1");
+        options.put(RpcOptions.RPC_SERVER_PORT.name(), "8091");
+        options.put(RpcOptions.RPC_REMOTE_URL.name(), "127.0.0.1:8091");
+        RpcClientProvider selfOnly = new RpcClientProvider(new 
HugeConfig(options));
+        Assert.assertFalse(selfOnly.enabled());
+        Assert.assertThrows(IllegalArgumentException.class, selfOnly::config, 
e -> {
+            Assert.assertContains("add an address other than self service", 
e.getMessage());
+        });
+        // Both are no-ops when the client is disabled
+        selfOnly.unreferAll();
+        selfOnly.destroy();
+
+        options.put(RpcOptions.RPC_REMOTE_URL.name(), 
"127.0.0.1:8091,127.0.0.1:8092");
+        RpcClientProvider withPeer = new RpcClientProvider(new 
HugeConfig(options));
+        try {
+            Assert.assertTrue(withPeer.enabled());
+            Assert.assertEquals("127.0.0.1:8092",
+                                Whitebox.getInternalState(withPeer.config(), 
"remoteUrls"));

Review Comment:
   This assertion relies on a private field name (`remoteUrls`) via reflection, 
which makes the test brittle to refactors that don’t change behavior. Prefer 
asserting through public behavior (e.g., the resulting consumer config’s direct 
URL, or a public accessor) so the test remains stable even if internal fields 
are renamed or reorganized.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to