From af58199d27aeb5da3d0639a0070d5eae60fe371d Mon Sep 17 00:00:00 2001 From: scottf Date: Sat, 13 Jun 2026 16:10:04 -0400 Subject: [PATCH 1/4] Better host comparison --- .gitattributes | 2 +- .../nats/client/impl/ApPassiveServerPool.java | 60 ++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/.gitattributes b/.gitattributes index 00a51af..e30ff2a 100644 --- a/.gitattributes +++ b/.gitattributes @@ -3,4 +3,4 @@ # # These are explicitly windows files and should use crlf *.bat text eol=crlf - +gradlew text eol=lf diff --git a/src/main/java/io/nats/client/impl/ApPassiveServerPool.java b/src/main/java/io/nats/client/impl/ApPassiveServerPool.java index 49c8f11..714b80c 100644 --- a/src/main/java/io/nats/client/impl/ApPassiveServerPool.java +++ b/src/main/java/io/nats/client/impl/ApPassiveServerPool.java @@ -6,30 +6,67 @@ import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; +import java.net.URISyntaxException; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.atomic.AtomicReference; public class ApPassiveServerPool implements ServerPool { final ServerPool pool; final AtomicReference activeServerRef; + final Map> resolvedMap; + Options.HostnameResolveMode resolveMode; public ApPassiveServerPool(ServerPool pool) { this.pool = pool; activeServerRef = new AtomicReference<>(); + resolvedMap = new HashMap<>(); } public void setActiveServer(NatsUri activeNuri) { activeServerRef.set(activeNuri); + resolve(activeNuri); + } + + private void resolve(NatsUri nuri) { + if (!nuri.hostIsIpAddress() && !nuri.isWebsocket() && resolveMode.resolve) { + String host = nuri.getHost(); + List resolved = resolvedMap.get(host); + if (resolved == null) { + resolved = pool.resolveHostToIps(host, resolveMode.maxOneResult, resolveMode.includeIPV6); + if (resolved != null && !resolved.isEmpty()) { + resolvedMap.put(host, resolved); + } + } + } + } + + private void resolve(@NonNull List serverList) { + for (String server : serverList) { + try { + resolve(new NatsUri(server)); + } + catch (URISyntaxException e) { + // ignore, sorry nothing we can do + } + } } @Override public void initialize(@NonNull Options opts) { + resolveMode = opts.hostnameResolveMode(); pool.initialize(opts); + resolve(pool.getServerList()); } @Override public boolean acceptDiscoveredUrls(@NonNull List<@NonNull String> discoveredServers) { - return pool.acceptDiscoveredUrls(discoveredServers); + boolean accepted = pool.acceptDiscoveredUrls(discoveredServers); + if (accepted) { + resolve(pool.getServerList()); + } + return accepted; } @Override @@ -41,7 +78,7 @@ public boolean acceptDiscoveredUrls(@NonNull List<@NonNull String> discoveredSer NatsUri firstPeek = pool.peekNextServer(); NatsUri peek = firstPeek; - while (peek != null && peek.equivalent(active)) { + while (peek != null && isEquivalent(peek, active)) { pool.nextServer(); // advance and peek again peek = pool.peekNextServer(); if (peek == firstPeek) { // if we've looped around, nothing else we can do @@ -51,6 +88,23 @@ public boolean acceptDiscoveredUrls(@NonNull List<@NonNull String> discoveredSer return peek; } + private boolean isEquivalent(NatsUri test, NatsUri active) { + String activeHost = active.getHost(); + if (test.getHost().equals(activeHost)) { + return true; + } + if (resolveMode.resolve) { + List testResolved = resolvedMap.get(test.getHost()); + List activeResolved = resolvedMap.get(active.getHost()); + for (String resolved : testResolved) { + if (activeResolved.contains(resolved) || resolved.equals(activeHost)) { + return true; + } + } + } + return false; + } + @Override public @Nullable NatsUri nextServer() { NatsUri active = activeServerRef.get(); @@ -59,7 +113,7 @@ public boolean acceptDiscoveredUrls(@NonNull List<@NonNull String> discoveredSer } NatsUri firstServer = pool.nextServer(); NatsUri server = firstServer; - while (server != null && server.equivalent(active)) { + while (server != null && isEquivalent(server, active)) { server = pool.nextServer(); // get the next nextServer if (server == firstServer) { // if we've looped around, nothing else we can do break; From 08d1b113bcab64f282411a694b0f3fd3b7376067 Mon Sep 17 00:00:00 2001 From: scottf Date: Mon, 15 Jun 2026 12:47:16 -0400 Subject: [PATCH 2/4] address review --- .../io/nats/client/impl/ApPassiveServerPool.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/nats/client/impl/ApPassiveServerPool.java b/src/main/java/io/nats/client/impl/ApPassiveServerPool.java index 714b80c..22f952e 100644 --- a/src/main/java/io/nats/client/impl/ApPassiveServerPool.java +++ b/src/main/java/io/nats/client/impl/ApPassiveServerPool.java @@ -95,10 +95,14 @@ private boolean isEquivalent(NatsUri test, NatsUri active) { } if (resolveMode.resolve) { List testResolved = resolvedMap.get(test.getHost()); - List activeResolved = resolvedMap.get(active.getHost()); - for (String resolved : testResolved) { - if (activeResolved.contains(resolved) || resolved.equals(activeHost)) { - return true; + if (testResolved != null) { + List activeResolved = resolvedMap.get(active.getHost()); + if (activeResolved != null) { + for (String resolved : testResolved) { + if (activeResolved.contains(resolved) || resolved.equals(activeHost)) { + return true; + } + } } } } From 37d23bd1299da7843be45ee06cf18b09f564bb94 Mon Sep 17 00:00:00 2001 From: scottf Date: Mon, 15 Jun 2026 13:12:19 -0400 Subject: [PATCH 3/4] fixed tests --- build.gradle | 4 +-- downloader/build.gradle | 2 +- downloader/pom.xml | 2 +- .../java/io/nats/client/impl/ApTests.java | 35 ++++++++++--------- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/build.gradle b/build.gradle index 0fee1e8..5c1db3c 100644 --- a/build.gradle +++ b/build.gradle @@ -10,7 +10,7 @@ plugins { id("signing") } -def jarVersion = "0.0.0" +def jarVersion = "0.0.1" group = 'io.synadia' def isRelease = System.getenv("BUILD_EVENT") == "release" @@ -40,7 +40,7 @@ repositories { } dependencies { - implementation 'io.nats:jnats:2.25.2' + implementation 'io.nats:jnats:2.25.3' implementation 'org.jspecify:jspecify:1.0.0' testImplementation 'io.nats:jnats-server-runner:3.1.0' diff --git a/downloader/build.gradle b/downloader/build.gradle index c8e9658..ef122c6 100644 --- a/downloader/build.gradle +++ b/downloader/build.gradle @@ -17,7 +17,7 @@ repositories { } dependencies { - implementation 'io.nats:jnats:2.25.2' + implementation 'io.nats:jnats:2.25.3' implementation 'io.synadia:active-passive:0.0.0-SNAPSHOT' implementation 'io.synadia:active-passive-jdk17:0.0.0-SNAPSHOT' implementation 'io.synadia:active-passive-jdk21:0.0.0-SNAPSHOT' diff --git a/downloader/pom.xml b/downloader/pom.xml index a97ef1e..feb3bc9 100644 --- a/downloader/pom.xml +++ b/downloader/pom.xml @@ -33,7 +33,7 @@ io.nats jnats - 2.25.2 + 2.25.3 io.synadia diff --git a/src/test/java/io/nats/client/impl/ApTests.java b/src/test/java/io/nats/client/impl/ApTests.java index 1643f2f..fb9c135 100644 --- a/src/test/java/io/nats/client/impl/ApTests.java +++ b/src/test/java/io/nats/client/impl/ApTests.java @@ -102,8 +102,7 @@ public void testSomeBadServers() throws Exception { helper.passiveListener.reset(); helper.passiveListener.queueConnectionEvent(ConnectionListener.Events.DISCONNECTED); - helper.passiveListener.queueConnectionEvent(ConnectionListener.Events.RECONNECTED); - helper.passiveListener.queueConnectionEvent(ConnectionListener.Events.RESUBSCRIBED); + helper.passiveListener.queueConnectionEvent(ConnectionListener.Events.CONNECTED); helper.passiveListener.queueConnectionEvent(ConnectionListener.Events.CLOSED); try (ApConnection apc = ApConnection.connect(helper.apOptions)) { @@ -151,21 +150,23 @@ public void testServerPoolBehavior() throws Exception { } try (NatsServerRunner server2 = new NatsServerRunner()) { - // make sure passive never is the same as active - helper = getHelper(server1, server2); - try (ApConnection apc = ApConnection.connect(helper.apOptions)) { - helper.validateConnected(); - assertNotEquals( - apc.getServerInfo().getServerId(), - apc.getPassiveServerInfo().getServerId()); - - apc.passiveForceReconnect(); - assertNotEquals( - apc.getServerInfo().getServerId(), - apc.getPassiveServerInfo().getServerId()); - } - catch (InterruptedException | IOException e) { - fail(); + try (NatsServerRunner server3 = new NatsServerRunner()) { + // make sure passive never is the same as active + helper = getHelper(server1, server2, server3); + try (ApConnection apc = ApConnection.connect(helper.apOptions)) { + helper.validateConnected(); + assertNotEquals( + apc.getServerInfo().getServerId(), + apc.getPassiveServerInfo().getServerId()); + + apc.passiveForceReconnect(); + assertNotEquals( + apc.getServerInfo().getServerId(), + apc.getPassiveServerInfo().getServerId()); + } + catch (InterruptedException | IOException e) { + fail(); + } } } } From e9f9782778e5422deb6960dc2c217c4d4cf1cff0 Mon Sep 17 00:00:00 2001 From: scottf Date: Mon, 15 Jun 2026 15:58:09 -0400 Subject: [PATCH 4/4] improved behavior --- .gitignore | 5 +- .../nats/client/impl/ApPassiveServerPool.java | 78 +-- .../client/impl/ApPassiveServerPoolTests.java | 456 ++++++++++++++++++ 3 files changed, 501 insertions(+), 38 deletions(-) create mode 100644 src/test/java/io/nats/client/impl/ApPassiveServerPoolTests.java diff --git a/.gitignore b/.gitignore index b3e2ca5..e0f0fa3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,8 @@ - -# NATS stuff # +# NATS / custom # ############## gnatsd.log +.claude/** +**/Debug*.java *.csv # Compiled source # diff --git a/src/main/java/io/nats/client/impl/ApPassiveServerPool.java b/src/main/java/io/nats/client/impl/ApPassiveServerPool.java index 22f952e..6936fff 100644 --- a/src/main/java/io/nats/client/impl/ApPassiveServerPool.java +++ b/src/main/java/io/nats/client/impl/ApPassiveServerPool.java @@ -7,6 +7,7 @@ import org.jspecify.annotations.Nullable; import java.net.URISyntaxException; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -29,30 +30,6 @@ public void setActiveServer(NatsUri activeNuri) { resolve(activeNuri); } - private void resolve(NatsUri nuri) { - if (!nuri.hostIsIpAddress() && !nuri.isWebsocket() && resolveMode.resolve) { - String host = nuri.getHost(); - List resolved = resolvedMap.get(host); - if (resolved == null) { - resolved = pool.resolveHostToIps(host, resolveMode.maxOneResult, resolveMode.includeIPV6); - if (resolved != null && !resolved.isEmpty()) { - resolvedMap.put(host, resolved); - } - } - } - } - - private void resolve(@NonNull List serverList) { - for (String server : serverList) { - try { - resolve(new NatsUri(server)); - } - catch (URISyntaxException e) { - // ignore, sorry nothing we can do - } - } - } - @Override public void initialize(@NonNull Options opts) { resolveMode = opts.hostnameResolveMode(); @@ -89,22 +66,21 @@ public boolean acceptDiscoveredUrls(@NonNull List<@NonNull String> discoveredSer } private boolean isEquivalent(NatsUri test, NatsUri active) { + String testHost = test.getHost(); String activeHost = active.getHost(); - if (test.getHost().equals(activeHost)) { + if (testHost.equals(activeHost)) { return true; } if (resolveMode.resolve) { - List testResolved = resolvedMap.get(test.getHost()); - if (testResolved != null) { - List activeResolved = resolvedMap.get(active.getHost()); - if (activeResolved != null) { - for (String resolved : testResolved) { - if (activeResolved.contains(resolved) || resolved.equals(activeHost)) { - return true; - } - } - } + // The NATS server reports discovered servers as IP addresses, while users typically + // supply hostnames - so a comparison is always host-vs-IP, never hostname-vs-hostname + // by resolution (two distinct hostnames only match by the direct equality check above). + if (active.hostIsIpAddress()) { + // active is an IP: equivalent only to a test hostname that resolves to that IP. + return !test.hostIsIpAddress() && _resolveHostToIps(testHost).contains(activeHost); } + // active is a hostname: equivalent only to a test IP that is one of its resolved IPs. + return test.hostIsIpAddress() && _resolveHostToIps(activeHost).contains(testHost); } return false; } @@ -128,7 +104,37 @@ private boolean isEquivalent(NatsUri test, NatsUri active) { @Override public @Nullable List resolveHostToIps(@NonNull String host) { - return pool.resolveHostToIps(host); + return _resolveHostToIps(host); + } + + private @NonNull List _resolveHostToIps(@NonNull String host) { + List resolved = resolvedMap.get(host); + if (resolved == null) { + resolved = pool.resolveHostToIps(host, resolveMode.maxOneResult, resolveMode.includeIPV6); + if (resolved == null || resolved.isEmpty()) { + // placeholder so we don't keep re-resolving a host that resolves to nothing + resolved = Collections.emptyList(); + } + resolvedMap.put(host, resolved); + } + return resolved; + } + + private void resolve(NatsUri nuri) { + if (!nuri.hostIsIpAddress() && !nuri.isWebsocket() && resolveMode.resolve) { + _resolveHostToIps(nuri.getHost()); + } + } + + private void resolve(@NonNull List serverList) { + for (String server : serverList) { + try { + resolve(new NatsUri(server)); + } + catch (URISyntaxException e) { + throw new RuntimeException(e); // this should never happen, if it does it's a user error + } + } } @Override diff --git a/src/test/java/io/nats/client/impl/ApPassiveServerPoolTests.java b/src/test/java/io/nats/client/impl/ApPassiveServerPoolTests.java new file mode 100644 index 0000000..acabe74 --- /dev/null +++ b/src/test/java/io/nats/client/impl/ApPassiveServerPoolTests.java @@ -0,0 +1,456 @@ +package io.nats.client.impl; + +import io.nats.client.Options; +import io.nats.client.ServerPool; +import io.nats.client.support.NatsUri; +import org.junit.jupiter.api.Test; + +import java.net.URISyntaxException; +import java.util.*; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Unit tests for the wrapper/equivalence logic added in PR #2 ("Better host comparison"). + *

+ * These exercise {@link ApPassiveServerPool}'s new resolution-based equivalence + * ({@code isEquivalent}, {@code resolve}, {@code resolvedMap}, {@code resolveMode}) and the + * peek/next traversal in isolation, using a deterministic fake {@link ServerPool} so no real + * DNS or NATS servers are required. The existing {@code ApTests} cover only the + * boot-real-servers-on-localhost path, where hostnames are never resolved. + */ +public class ApPassiveServerPoolTests { + + // ---------------------------------------------------------------------------------------- + // Test doubles / helpers + // ---------------------------------------------------------------------------------------- + + /** Deterministic ServerPool for unit-testing ApPassiveServerPool's wrapper logic. */ + static class FakeServerPool implements ServerPool { + final Map> dns = new HashMap<>(); // host -> resolved IPs + final List peekQueue = new ArrayList<>(); // what peek/next hand back, in order + int pos = 0; + List serverList = new ArrayList<>(); + boolean acceptResult = true; + boolean secure = false; + final List resolveCalls = new ArrayList<>(); // records each resolve, for cache asserts + final List connectSucceededCalls = new ArrayList<>(); + final List connectFailedCalls = new ArrayList<>(); + Options initializedWith; + + @Override public void initialize(Options opts) { initializedWith = opts; } + + @Override public boolean acceptDiscoveredUrls(List discovered) { return acceptResult; } + + @Override public NatsUri peekNextServer() { + return peekQueue.isEmpty() ? null : peekQueue.get(pos % peekQueue.size()); + } + + @Override public NatsUri nextServer() { + if (peekQueue.isEmpty()) { + return null; + } + NatsUri n = peekQueue.get(pos % peekQueue.size()); + pos++; + return n; + } + + @Override public List resolveHostToIps(String host) { return dns.get(host); } + + @Override public List resolveHostToIps(String host, boolean maxOne, boolean ipv6) { + resolveCalls.add(host); + List ips = dns.get(host); + if (ips == null) { + return null; + } + return (maxOne && !ips.isEmpty()) ? ips.subList(0, 1) : ips; + } + + @Override public void connectSucceeded(NatsUri nuri) { connectSucceededCalls.add(nuri); } + @Override public void connectFailed(NatsUri nuri) { connectFailedCalls.add(nuri); } + @Override public List getServerList() { return serverList; } + @Override public boolean hasSecureServer() { return secure; } + } + + private static NatsUri uri(String s) { + try { + return new NatsUri(s); + } + catch (URISyntaxException e) { + throw new RuntimeException(e); + } + } + + private static Options optionsWith(Options.HostnameResolveMode mode) { + return Options.builder().hostnameResolveMode(mode).build(); + } + + // ---------------------------------------------------------------------------------------- + // A. isEquivalent - direct host-string match (resolution disabled) + // ---------------------------------------------------------------------------------------- + + @Test + public void isEquivalent_A1_sameHostString_isSkipped() { + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.Unresolved; + + sp.setActiveServer(uri("nats://alpha.example.com:4222")); + fake.peekQueue.add(uri("nats://alpha.example.com:4222")); // same host -> skipped + fake.peekQueue.add(uri("nats://beta.example.com:4222")); + + assertEquals("beta.example.com", sp.peekNextServer().getHost()); + } + + @Test + public void isEquivalent_A2_differentHost_isReturned() { + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.Unresolved; + + sp.setActiveServer(uri("nats://alpha.example.com:4222")); + fake.peekQueue.add(uri("nats://beta.example.com:4222")); + + assertEquals("beta.example.com", sp.peekNextServer().getHost()); + } + + // ---------------------------------------------------------------------------------------- + // B. isEquivalent - resolution branch (ResolveToAll) + // ---------------------------------------------------------------------------------------- + + @Test + public void isEquivalent_B1_twoDistinctHostnames_neverEquivalent() { + // Discovered servers always come back as IPs and users supply hostnames, so two distinct + // hostnames never get compared by resolution - only by direct equality. A shared resolved + // IP does NOT make them equivalent. + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.ResolveToAll; + sp.resolvedMap.put("a.example.com", Arrays.asList("10.0.0.1")); + sp.resolvedMap.put("b.example.com", Arrays.asList("10.0.0.1", "10.0.0.2")); // shares 10.0.0.1 + + sp.setActiveServer(uri("nats://a.example.com:4222")); + fake.peekQueue.add(uri("nats://b.example.com:4222")); + + assertEquals("b.example.com", sp.peekNextServer().getHost()); // not skipped despite shared IP + } + + @Test + public void isEquivalent_B2_activeIp_testHostnameResolvesToIt_equivalent() { + // active is a literal IP; a peer hostname that resolves to that IP is equivalent (skipped). + // The distinct server c.example.com proves the skip actually happened. + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.ResolveToAll; + sp.resolvedMap.put("b.example.com", Arrays.asList("10.0.0.1")); + + sp.setActiveServer(uri("nats://10.0.0.1:4222")); // literal IP, not cached + fake.peekQueue.add(uri("nats://b.example.com:4222")); // resolves to active IP -> skipped + fake.peekQueue.add(uri("nats://c.example.com:4222")); // distinct -> returned + + assertEquals("c.example.com", sp.peekNextServer().getHost()); + } + + @Test + public void isEquivalent_B3_activeIp_testHostnameDoesNotResolveToIt_notEquivalent() { + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.ResolveToAll; + sp.resolvedMap.put("b.example.com", Arrays.asList("10.0.0.9")); // resolves elsewhere + + sp.setActiveServer(uri("nats://10.0.0.1:4222")); + fake.peekQueue.add(uri("nats://b.example.com:4222")); + + assertEquals("b.example.com", sp.peekNextServer().getHost()); // not skipped + } + + @Test + public void isEquivalent_B4_activeHostname_testIpInResolution_equivalent() { + // Reverse of B2: active is a resolvable hostname; a peer literal IP it resolves to is + // equivalent (skipped). + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.ResolveToAll; + sp.resolvedMap.put("a.example.com", Arrays.asList("10.0.0.1")); + + sp.setActiveServer(uri("nats://a.example.com:4222")); + fake.peekQueue.add(uri("nats://10.0.0.1:4222")); // active resolves to this IP -> skipped + fake.peekQueue.add(uri("nats://c.example.com:4222")); // distinct -> returned + + assertEquals("c.example.com", sp.peekNextServer().getHost()); + + // comparing a literal-IP peer must not resolve it or pollute the cache + assertFalse(sp.resolvedMap.containsKey("10.0.0.1")); + assertFalse(fake.resolveCalls.contains("10.0.0.1")); + } + + @Test + public void isEquivalent_B5_activeHostname_testIpNotInResolution_notEquivalent() { + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.ResolveToAll; + sp.resolvedMap.put("a.example.com", Arrays.asList("10.0.0.1")); + + sp.setActiveServer(uri("nats://a.example.com:4222")); + fake.peekQueue.add(uri("nats://10.0.0.9:4222")); // not one of active's resolved IPs + + assertEquals("10.0.0.9", sp.peekNextServer().getHost()); // not skipped + } + + // ---------------------------------------------------------------------------------------- + // C. resolve(...) cache population (assert on resolvedMap after initialize) + // ---------------------------------------------------------------------------------------- + + @Test + public void resolve_C1_hostnamesAreCached() { + FakeServerPool fake = new FakeServerPool(); + fake.dns.put("a.example.com", Arrays.asList("10.0.0.1")); + fake.dns.put("b.example.com", Arrays.asList("10.0.0.2", "10.0.0.3")); + fake.serverList = Arrays.asList("nats://a.example.com:4222", "nats://b.example.com:4222"); + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.initialize(optionsWith(Options.HostnameResolveMode.ResolveToAll)); + + assertEquals(2, sp.resolvedMap.size()); + assertEquals(Arrays.asList("10.0.0.1"), sp.resolvedMap.get("a.example.com")); + assertEquals(Arrays.asList("10.0.0.2", "10.0.0.3"), sp.resolvedMap.get("b.example.com")); + } + + @Test + public void resolve_C2_literalIpIsNotCached() { + FakeServerPool fake = new FakeServerPool(); + fake.serverList = Arrays.asList("nats://10.0.0.1:4222"); + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.initialize(optionsWith(Options.HostnameResolveMode.ResolveToAll)); + + assertTrue(sp.resolvedMap.isEmpty()); + assertTrue(fake.resolveCalls.isEmpty()); // never even attempted + } + + @Test + public void resolve_C3_websocketIsNotCached() { + FakeServerPool fake = new FakeServerPool(); + fake.dns.put("a.example.com", Arrays.asList("10.0.0.1")); + fake.serverList = Arrays.asList("ws://a.example.com:4222"); + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.initialize(optionsWith(Options.HostnameResolveMode.ResolveToAll)); + + assertTrue(sp.resolvedMap.isEmpty()); + } + + @Test + public void resolve_C4_unresolvedModeCachesNothing() { + FakeServerPool fake = new FakeServerPool(); + fake.dns.put("a.example.com", Arrays.asList("10.0.0.1")); + fake.serverList = Arrays.asList("nats://a.example.com:4222"); + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.initialize(optionsWith(Options.HostnameResolveMode.Unresolved)); + + assertTrue(sp.resolvedMap.isEmpty()); + assertTrue(fake.resolveCalls.isEmpty()); + } + + @Test + public void resolve_C5_emptyResolutionCachesPlaceholderAndIsNotReResolved() { + FakeServerPool fake = new FakeServerPool(); + fake.dns.put("empty.example.com", new ArrayList<>()); // resolves to empty list + fake.serverList = Arrays.asList("nats://empty.example.com:4222"); + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.initialize(optionsWith(Options.HostnameResolveMode.ResolveToAll)); + + // "resolved to nothing" is recorded as an (empty) placeholder, not left absent + assertTrue(sp.resolvedMap.containsKey("empty.example.com")); + assertTrue(sp.resolvedMap.get("empty.example.com").isEmpty()); + + // so a second pass finds the placeholder and does not resolve again + sp.setActiveServer(uri("nats://empty.example.com:4222")); + assertEquals(1, Collections.frequency(fake.resolveCalls, "empty.example.com")); + } + + @Test + public void resolve_C6_resultsAreCachedNotReResolved() { + FakeServerPool fake = new FakeServerPool(); + fake.dns.put("a.example.com", Arrays.asList("10.0.0.1")); + fake.serverList = Arrays.asList("nats://a.example.com:4222"); + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.initialize(optionsWith(Options.HostnameResolveMode.ResolveToAll)); // resolves a once + sp.setActiveServer(uri("nats://a.example.com:4222")); // same host -> cache hit + + assertEquals(1, Collections.frequency(fake.resolveCalls, "a.example.com")); + } + + @Test + public void resolve_C7_malformedServerStringFailsFast() { + FakeServerPool fake = new FakeServerPool(); + fake.dns.put("good.example.com", Arrays.asList("10.0.0.1")); + fake.serverList = Arrays.asList("nats://bad host:4222", "nats://good.example.com:4222"); + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + // a malformed server string is treated as user error and surfaced (wrapped URISyntaxException) + assertThrows(RuntimeException.class, + () -> sp.initialize(optionsWith(Options.HostnameResolveMode.ResolveToAll))); + } + + // ---------------------------------------------------------------------------------------- + // D. acceptDiscoveredUrls + // ---------------------------------------------------------------------------------------- + + @Test + public void acceptDiscoveredUrls_D1_accepted_reResolves() { + FakeServerPool fake = new FakeServerPool(); + fake.acceptResult = true; + fake.dns.put("new.example.com", Arrays.asList("10.0.0.5")); + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.initialize(optionsWith(Options.HostnameResolveMode.ResolveToAll)); // empty server list + + fake.serverList = Arrays.asList("nats://new.example.com:4222"); // discovered + assertTrue(sp.acceptDiscoveredUrls(Arrays.asList("nats://new.example.com:4222"))); + assertTrue(sp.resolvedMap.containsKey("new.example.com")); + } + + @Test + public void acceptDiscoveredUrls_D2_rejected_doesNotReResolve() { + FakeServerPool fake = new FakeServerPool(); + fake.acceptResult = false; + fake.dns.put("new.example.com", Arrays.asList("10.0.0.5")); + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.initialize(optionsWith(Options.HostnameResolveMode.ResolveToAll)); + + fake.serverList = Arrays.asList("nats://new.example.com:4222"); + assertFalse(sp.acceptDiscoveredUrls(Arrays.asList("nats://new.example.com:4222"))); + assertTrue(sp.resolvedMap.isEmpty()); + } + + // ---------------------------------------------------------------------------------------- + // E. setActiveServer + // ---------------------------------------------------------------------------------------- + + @Test + public void setActiveServer_E1_cachesActiveHostname() { + FakeServerPool fake = new FakeServerPool(); + fake.dns.put("a.example.com", Arrays.asList("10.0.0.1")); + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.ResolveToAll; + + NatsUri active = uri("nats://a.example.com:4222"); + sp.setActiveServer(active); + + assertSame(active, sp.activeServerRef.get()); + assertEquals(Arrays.asList("10.0.0.1"), sp.resolvedMap.get("a.example.com")); + } + + @Test + public void setActiveServer_E2_ipActiveHostNotCached() { + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.ResolveToAll; + + sp.setActiveServer(uri("nats://10.0.0.1:4222")); + + assertTrue(sp.resolvedMap.isEmpty()); + } + + // ---------------------------------------------------------------------------------------- + // F. peek/next traversal & loop-around termination + // ---------------------------------------------------------------------------------------- + + @Test + public void traversal_F1_nullActive_peekDelegatesDirectly() { + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + + NatsUri only = uri("nats://a.example.com:4222"); + fake.peekQueue.add(only); + + assertSame(only, sp.peekNextServer()); // no active -> straight delegation, no skipping + } + + @Test + public void traversal_F2_nullActive_nextDelegatesDirectly() { + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + + NatsUri only = uri("nats://a.example.com:4222"); + fake.peekQueue.add(only); + + assertSame(only, sp.nextServer()); + } + + @Test + public void traversal_F3_nextSkipsEquivalentsReturnsFirstDistinct() { + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.Unresolved; + + sp.setActiveServer(uri("nats://h.example.com:4222")); + fake.peekQueue.add(uri("nats://h.example.com:4222")); // equivalent (same host) + fake.peekQueue.add(uri("nats://h.example.com:6222")); // equivalent (same host, diff port) + fake.peekQueue.add(uri("nats://other.example.com:4222")); + + assertEquals("other.example.com", sp.nextServer().getHost()); + } + + @Test + public void traversal_F4_peekLoopAroundTerminates() { + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.Unresolved; + + sp.setActiveServer(uri("nats://h.example.com:4222")); + NatsUri sameRef = uri("nats://h.example.com:4222"); // only candidate, always equivalent + fake.peekQueue.add(sameRef); + + // peek == firstPeek guard must break the loop and return the (equivalent) server + assertSame(sameRef, sp.peekNextServer()); + } + + @Test + public void traversal_F5_nextLoopAroundTerminates() { + FakeServerPool fake = new FakeServerPool(); + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + sp.resolveMode = Options.HostnameResolveMode.Unresolved; + + sp.setActiveServer(uri("nats://h.example.com:4222")); + NatsUri sameRef = uri("nats://h.example.com:4222"); + fake.peekQueue.add(sameRef); + + // server == firstServer guard must break the loop and return the (equivalent) server + assertSame(sameRef, sp.nextServer()); + } + + // ---------------------------------------------------------------------------------------- + // G. pass-through delegation + // ---------------------------------------------------------------------------------------- + + @Test + public void delegation_G_passesThroughToWrappedPool() { + FakeServerPool fake = new FakeServerPool(); + fake.dns.put("a.example.com", Arrays.asList("10.0.0.1")); + fake.serverList = Arrays.asList("nats://a.example.com:4222"); + fake.secure = true; + + ApPassiveServerPool sp = new ApPassiveServerPool(fake); + Options opts = optionsWith(Options.HostnameResolveMode.Unresolved); + sp.initialize(opts); + + assertSame(opts, fake.initializedWith); + assertEquals(Arrays.asList("10.0.0.1"), sp.resolveHostToIps("a.example.com")); + assertSame(fake.serverList, sp.getServerList()); + assertTrue(sp.hasSecureServer()); + + NatsUri ok = uri("nats://a.example.com:4222"); + NatsUri bad = uri("nats://b.example.com:4222"); + sp.connectSucceeded(ok); + sp.connectFailed(bad); + assertEquals(Arrays.asList(ok), fake.connectSucceededCalls); + assertEquals(Arrays.asList(bad), fake.connectFailedCalls); + } +}