From ef9d177adcae6538b82aca5b6c11c2b57587d535 Mon Sep 17 00:00:00 2001 From: slhafzjw Date: Fri, 3 Apr 2026 15:35:27 +0800 Subject: [PATCH] fix(config): correct duplicate config checking --- .../api/agent/runtime/config/ConfigCenter.kt | 37 ++++++----- .../runtime/config/ConfigCenterTest.java | 65 +++++++++++++++---- 2 files changed, 72 insertions(+), 30 deletions(-) diff --git a/Partner-Framework/src/main/java/work/slhaf/partner/api/agent/runtime/config/ConfigCenter.kt b/Partner-Framework/src/main/java/work/slhaf/partner/api/agent/runtime/config/ConfigCenter.kt index c515572c..5947ba45 100644 --- a/Partner-Framework/src/main/java/work/slhaf/partner/api/agent/runtime/config/ConfigCenter.kt +++ b/Partner-Framework/src/main/java/work/slhaf/partner/api/agent/runtime/config/ConfigCenter.kt @@ -36,14 +36,14 @@ object ConfigCenter : AutoCloseable { declared.forEach { (path, registration) -> val normalizedPath = normalizeRelativePath(path) - check(normalized.putIfAbsent(normalizedPath, registration) != null) { + check(normalized.putIfAbsent(normalizedPath, registration) == null) { "Duplicated config path declared in the same configurable: $normalizedPath" } } normalized.forEach { (path, registration) -> - check(registrations.putIfAbsent(path, registration) != null) { + check(registrations.putIfAbsent(path, registration) == null) { "Config path already registered: $path" } } @@ -77,14 +77,16 @@ object ConfigCenter : AutoCloseable { @Suppress("UNCHECKED_CAST") fun initAll() { registrations.forEach { (path, registration) -> - try { - val config = loadConfig(path, registration) + val config = loadConfig(path, registration) + if (config != null) { (registration as ConfigRegistration).init(config) - } catch (e: Exception) { - if (registration.configRequired()) { - throw AgentLaunchFailedException("Failed to init config", e) - } + return } + val defaultConfig = registration.defaultConfig() + if (defaultConfig != null) { + (registration as ConfigRegistration).init(defaultConfig) + } + throw AgentLaunchFailedException("Failed to init config, related path: $path") } } @@ -125,18 +127,21 @@ object ConfigCenter : AutoCloseable { val registration = registrations[relativePath] ?: return try { val config = loadConfig(file, registration) - (registration as ConfigRegistration).init(config) + if (config != null) { + (registration as ConfigRegistration).onReload(config) + } } catch (e: Exception) { log.error("Config reload failed: {}", relativePath, e) } } - private fun loadConfig(file: Path, registration: ConfigRegistration): Config { - return JSON.parseObject(Files.readString(file, StandardCharsets.UTF_8), registration.type()) as Config - } - - private fun notifyReload(registration: ConfigRegistration, config: Config) { - registration.onReload(config) + private fun loadConfig(file: Path, registration: ConfigRegistration): Config? { + return try { + JSON.parseObject(Files.readString(file, StandardCharsets.UTF_8), registration.type()) as Config + } catch (e: Exception) { + log.error("Config reload failed: {}", file, e) + null + } } private fun toRelativeConfigPath(file: Path): Path? { @@ -186,5 +191,5 @@ interface ConfigRegistration { fun type(): Class fun init(config: T) fun onReload(config: T) {} - fun configRequired(): Boolean + fun defaultConfig(): T? } diff --git a/Partner-Framework/src/test/java/work/slhaf/partner/api/agent/runtime/config/ConfigCenterTest.java b/Partner-Framework/src/test/java/work/slhaf/partner/api/agent/runtime/config/ConfigCenterTest.java index 98033026..51ca9dad 100644 --- a/Partner-Framework/src/test/java/work/slhaf/partner/api/agent/runtime/config/ConfigCenterTest.java +++ b/Partner-Framework/src/test/java/work/slhaf/partner/api/agent/runtime/config/ConfigCenterTest.java @@ -1,5 +1,6 @@ package work.slhaf.partner.api.agent.runtime.config; +import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.*; import org.junit.jupiter.api.io.TempDir; @@ -16,14 +17,16 @@ import java.util.function.BooleanSupplier; @TestMethodOrder(MethodOrderer.OrderAnnotation.class) class ConfigCenterTest { - private static final Path INITIAL_PATH = Path.of("root-initial.json"); - private static final Path NESTED_PATH = Path.of("nested", "child.json"); - private static final Path DELETE_PATH = Path.of("delete", "target.json"); - private static final Path INVALID_PATH = Path.of("invalid", "target.json"); - private static final Path IDEMPOTENT_PATH = Path.of("idempotent", "target.json"); + private static final Path TEST_ROOT = Path.of("target", "config-center-test"); + private static final Path INITIAL_PATH = TEST_ROOT.resolve("root-initial.json"); + private static final Path NESTED_PATH = TEST_ROOT.resolve(Path.of("nested", "child.json")); + private static final Path DELETE_PATH = TEST_ROOT.resolve(Path.of("delete", "target.json")); + private static final Path INVALID_PATH = TEST_ROOT.resolve(Path.of("invalid", "target.json")); + private static final Path IDEMPOTENT_PATH = TEST_ROOT.resolve(Path.of("idempotent", "target.json")); private static String originalUserHome; private static Path configDir; + private static Path workingDir; private static TrackingRegistration initialRegistration; private static TrackingRegistration nestedRegistration; private static TrackingRegistration deleteRegistration; @@ -44,13 +47,19 @@ class ConfigCenterTest { invalidRegistration = new TrackingRegistration(); idempotentRegistration = new TrackingRegistration(); + workingDir = Path.of("").toAbsolutePath().normalize(); configDir = ConfigCenter.INSTANCE.getPaths().getConfigDir(); Files.createDirectories(configDir); Files.createDirectories(configDir.resolve(NESTED_PATH).getParent()); Files.createDirectories(configDir.resolve(DELETE_PATH).getParent()); Files.createDirectories(configDir.resolve(INVALID_PATH).getParent()); Files.createDirectories(configDir.resolve(IDEMPOTENT_PATH).getParent()); - writeJson(configDir.resolve(INITIAL_PATH), "initial", 1); + writeJson(workingDir.resolve(INITIAL_PATH), "initial-init", 1); + writeJson(workingDir.resolve(NESTED_PATH), "nested-init", 1); + writeJson(workingDir.resolve(DELETE_PATH), "delete-init", 1); + writeJson(workingDir.resolve(INVALID_PATH), "invalid-init", 1); + writeJson(workingDir.resolve(IDEMPOTENT_PATH), "idempotent-init", 1); + writeJson(configDir.resolve(INITIAL_PATH), "initial-config-dir", 1); ConfigCenter.INSTANCE.register(() -> { Map> declared = new LinkedHashMap<>(); @@ -67,6 +76,7 @@ class ConfigCenterTest { @AfterAll static void afterAll() { ConfigCenter.INSTANCE.close(); + deleteRecursivelyIfExists(workingDir.resolve(TEST_ROOT)); if (originalUserHome == null) { System.clearProperty("user.home"); } else { @@ -74,6 +84,14 @@ class ConfigCenterTest { } } + private static int totalInitCount() { + return initialRegistration.initCount() + + nestedRegistration.initCount() + + deleteRegistration.initCount() + + invalidRegistration.initCount() + + idempotentRegistration.initCount(); + } + private static int totalReloadCount() { return initialRegistration.reloadCount() + nestedRegistration.reloadCount() @@ -82,6 +100,22 @@ class ConfigCenterTest { + idempotentRegistration.reloadCount(); } + private static void deleteRecursivelyIfExists(Path root) { + if (!Files.exists(root)) { + return; + } + try (var stream = Files.walk(root)) { + stream.sorted((left, right) -> right.getNameCount() - left.getNameCount()) + .forEach(path -> { + try { + Files.deleteIfExists(path); + } catch (IOException ignored) { + } + }); + } catch (IOException ignored) { + } + } + private static void writeJson(Path file, String name, int version) throws IOException { Files.createDirectories(file.getParent()); Files.writeString(file, @@ -119,12 +153,9 @@ class ConfigCenterTest { @Test @Order(1) - void testInitialReconcileReloadsRegisteredJson() throws Exception { - waitForCount(initialRegistration, 1, 3000); - - Assertions.assertEquals(1, initialRegistration.reloadCount()); - Assertions.assertEquals("initial", initialRegistration.lastConfig().name); - Assertions.assertEquals(1, initialRegistration.lastConfig().version); + void testStartOnlyInitializesOneRegisteredConfigAndDoesNotTriggerReload() { + Assertions.assertEquals(1, totalInitCount()); + Assertions.assertEquals(0, totalReloadCount()); } @Test @@ -226,6 +257,7 @@ class ConfigCenterTest { } private static class TrackingRegistration implements ConfigRegistration { + private final AtomicInteger initCount = new AtomicInteger(); private final AtomicInteger reloadCount = new AtomicInteger(); private final AtomicReference lastConfig = new AtomicReference<>(); @@ -236,6 +268,7 @@ class ConfigCenterTest { @Override public void init(TestConfig config) { + initCount.incrementAndGet(); } @Override @@ -244,6 +277,10 @@ class ConfigCenterTest { reloadCount.incrementAndGet(); } + int initCount() { + return initCount.get(); + } + int reloadCount() { return reloadCount.get(); } @@ -253,8 +290,8 @@ class ConfigCenterTest { } @Override - public boolean configRequired() { - return true; + public @Nullable TestConfig defaultConfig() { + return null; } } }