From 6769508d0678f3a239a584a084736cc4b732cbbd Mon Sep 17 00:00:00 2001 From: Clement BALAY Date: Wed, 20 Sep 2023 15:29:10 +0200 Subject: [PATCH 1/2] Fix config sources ordering --- .../runtime/ConsulConfigSourceFactory.java | 8 +++-- .../ConsulConfigSourceFactoryTest.java | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/consul/runtime/src/main/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactory.java b/consul/runtime/src/main/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactory.java index 0d9d3d2..b775022 100644 --- a/consul/runtime/src/main/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactory.java +++ b/consul/runtime/src/main/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactory.java @@ -45,22 +45,24 @@ Iterable getConfigSources(ConsulConfig config, ConsulConfigGateway return Collections.emptyList(); } - List result = new ArrayList<>(keys.size()); + Map results = new LinkedHashMap<>(keys.size()); List> allUnis = new ArrayList<>(); for (Map.Entry entry : keys.entrySet()) { String fullKey = config.prefix().isPresent() ? config.prefix().get() + "/" + entry.getKey() : entry.getKey(); + results.put(fullKey, null); allUnis.add(consulConfigGateway.getValue(fullKey).invoke(new Consumer() { @Override public void accept(Response response) { if (response != null) { - result.add(ResponseConfigSourceUtil.toConfigSource(response, entry.getValue(), config.prefix())); + results.put(response.getKey(), ResponseConfigSourceUtil.toConfigSource(response, entry.getValue(), config.prefix())); } else { String message = "Key '" + fullKey + "' not found in Consul."; if (config.failOnMissingKey()) { throw new RuntimeException(message); } else { + results.remove(fullKey); log.info(message); } } @@ -81,6 +83,6 @@ public void accept(Response response) { consulConfigGateway.close(); } - return result; + return results.values(); } } diff --git a/consul/runtime/src/test/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactoryTest.java b/consul/runtime/src/test/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactoryTest.java index 0bb050d..dca9e38 100644 --- a/consul/runtime/src/test/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactoryTest.java +++ b/consul/runtime/src/test/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactoryTest.java @@ -14,6 +14,7 @@ import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.Optional; import org.eclipse.microprofile.config.spi.ConfigSource; @@ -207,6 +208,36 @@ void testPropertiesKeysWithPrefix() { verify(mockGateway, times(1)).getValue("config/second"); } + @Test + void testConfigSourcesOrdered() { + ConsulConfig config = mock(ConsulConfig.class); + // We want this order: first > second > third (first overrides second which overrides third) + when(config.rawValueKeys()).thenReturn(keyValues("some/first", "some/second", "some/third")); + AgentConfig agentConfig = mock(AgentConfig.class); + when(config.agent()).thenReturn(agentConfig); + + ConsulConfigGateway mockGateway = mock(ConsulConfigGateway.class); + // make sure the third property is received first + when(mockGateway.getValue("some/first")).thenReturn(validResponse("some/third", "third")); + // make sure the first property is received in second + when(mockGateway.getValue("some/second")).thenReturn(validResponse("some/first", "first")); + // make sure the second property is received in third + when(mockGateway.getValue("some/third")).thenReturn(validResponse("some/second", "second")); + + Iterable configSources = new ConsulConfigSourceFactory().getConfigSources(config, mockGateway); + assertThat(configSources).hasSize(3); + assertThat(configSources).extracting(ConfigSource::getProperties).containsExactly( + Map.of("some.first", "first"), + Map.of("some.second", "second"), + Map.of("some.third", "third") + ); + + //all keys should have been resolved because we resolve keys in the order they were given by the user + verify(mockGateway, times(1)).getValue("some/first"); + verify(mockGateway, times(1)).getValue("some/second"); + verify(mockGateway, times(1)).getValue("some/third"); + } + private Optional> keyValues(String... keys) { return Optional.of(Arrays.asList(keys)); } From eca7f1c72edbb2ad65ca2f28d0870225f8cd9f27 Mon Sep 17 00:00:00 2001 From: Roberto Cortez Date: Thu, 21 Sep 2023 12:27:46 +0100 Subject: [PATCH 2/2] Codestyle --- .../runtime/ConsulConfigSourceFactory.java | 3 +- .../ConsulConfigSourceFactoryTest.java | 57 +++++++++---------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/consul/runtime/src/main/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactory.java b/consul/runtime/src/main/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactory.java index b775022..6ded0c2 100644 --- a/consul/runtime/src/main/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactory.java +++ b/consul/runtime/src/main/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactory.java @@ -56,7 +56,8 @@ Iterable getConfigSources(ConsulConfig config, ConsulConfigGateway @Override public void accept(Response response) { if (response != null) { - results.put(response.getKey(), ResponseConfigSourceUtil.toConfigSource(response, entry.getValue(), config.prefix())); + results.put(response.getKey(), + ResponseConfigSourceUtil.toConfigSource(response, entry.getValue(), config.prefix())); } else { String message = "Key '" + fullKey + "' not found in Consul."; if (config.failOnMissingKey()) { diff --git a/consul/runtime/src/test/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactoryTest.java b/consul/runtime/src/test/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactoryTest.java index dca9e38..3109f17 100644 --- a/consul/runtime/src/test/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactoryTest.java +++ b/consul/runtime/src/test/java/io/quarkus/consul/config/runtime/ConsulConfigSourceFactoryTest.java @@ -208,35 +208,34 @@ void testPropertiesKeysWithPrefix() { verify(mockGateway, times(1)).getValue("config/second"); } - @Test - void testConfigSourcesOrdered() { - ConsulConfig config = mock(ConsulConfig.class); - // We want this order: first > second > third (first overrides second which overrides third) - when(config.rawValueKeys()).thenReturn(keyValues("some/first", "some/second", "some/third")); - AgentConfig agentConfig = mock(AgentConfig.class); - when(config.agent()).thenReturn(agentConfig); - - ConsulConfigGateway mockGateway = mock(ConsulConfigGateway.class); - // make sure the third property is received first - when(mockGateway.getValue("some/first")).thenReturn(validResponse("some/third", "third")); - // make sure the first property is received in second - when(mockGateway.getValue("some/second")).thenReturn(validResponse("some/first", "first")); - // make sure the second property is received in third - when(mockGateway.getValue("some/third")).thenReturn(validResponse("some/second", "second")); - - Iterable configSources = new ConsulConfigSourceFactory().getConfigSources(config, mockGateway); - assertThat(configSources).hasSize(3); - assertThat(configSources).extracting(ConfigSource::getProperties).containsExactly( - Map.of("some.first", "first"), - Map.of("some.second", "second"), - Map.of("some.third", "third") - ); - - //all keys should have been resolved because we resolve keys in the order they were given by the user - verify(mockGateway, times(1)).getValue("some/first"); - verify(mockGateway, times(1)).getValue("some/second"); - verify(mockGateway, times(1)).getValue("some/third"); - } + @Test + void testConfigSourcesOrdered() { + ConsulConfig config = mock(ConsulConfig.class); + // We want this order: first > second > third (first overrides second which overrides third) + when(config.rawValueKeys()).thenReturn(keyValues("some/first", "some/second", "some/third")); + AgentConfig agentConfig = mock(AgentConfig.class); + when(config.agent()).thenReturn(agentConfig); + + ConsulConfigGateway mockGateway = mock(ConsulConfigGateway.class); + // make sure the third property is received first + when(mockGateway.getValue("some/first")).thenReturn(validResponse("some/third", "third")); + // make sure the first property is received in second + when(mockGateway.getValue("some/second")).thenReturn(validResponse("some/first", "first")); + // make sure the second property is received in third + when(mockGateway.getValue("some/third")).thenReturn(validResponse("some/second", "second")); + + Iterable configSources = new ConsulConfigSourceFactory().getConfigSources(config, mockGateway); + assertThat(configSources).hasSize(3); + assertThat(configSources).extracting(ConfigSource::getProperties).containsExactly( + Map.of("some.first", "first"), + Map.of("some.second", "second"), + Map.of("some.third", "third")); + + //all keys should have been resolved because we resolve keys in the order they were given by the user + verify(mockGateway, times(1)).getValue("some/first"); + verify(mockGateway, times(1)).getValue("some/second"); + verify(mockGateway, times(1)).getValue("some/third"); + } private Optional> keyValues(String... keys) { return Optional.of(Arrays.asList(keys));