Skip to content

Commit

Permalink
Handles non-existent keys in consul properly.
Browse files Browse the repository at this point in the history
Previously a `ConfigData` with a list of PropertySources with one entry of `null` was returned in ConsulConfigDataLoader when the api call to consul did not find anything.

Now it returns null which is a signal to skip.

Optionality is all handled by ConfigData.

Fixes spring-cloudgh-693
  • Loading branch information
spencergibb committed Jan 22, 2021
1 parent 2be3632 commit 695a8b0
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,11 @@ public ConfigData load(ConfigDataLoaderContext context, ConsulConfigDataResource
ConsulClient consul = getBean(context, ConsulClient.class);
ConsulConfigIndexes indexes = getBean(context, ConsulConfigIndexes.class);

ConsulPropertySource propertySource = resource.getConsulPropertySources().createPropertySource(
resource.getContext(), resource.isOptional(), consul, indexes.getIndexes()::put);
ConsulPropertySource propertySource = resource.getConsulPropertySources()
.createPropertySource(resource.getContext(), consul, indexes.getIndexes()::put);
if (propertySource == null) {
return null;
}
return new ConfigData(Collections.singletonList(propertySource));
}
catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public List<ConsulConfigDataResource> resolveProfileSpecific(ConfigDataLocationR
InstanceSupplier.from(ConsulConfigDataIndexes::new));

return contexts.stream().map(propertySourceContext -> new ConsulConfigDataResource(propertySourceContext,
location.isOptional(), properties, consulPropertySources)).collect(Collectors.toList());
properties, consulPropertySources)).collect(Collectors.toList());
}

private BindHandler getBindHandler(ConfigDataLocationResolverContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ public class ConsulConfigDataResource extends ConfigDataResource {

private final ConsulPropertySources consulPropertySources;

public ConsulConfigDataResource(String context, ConsulConfigProperties properties,
ConsulPropertySources consulPropertySources) {
this(context, true, properties, consulPropertySources);
}

@Deprecated
public ConsulConfigDataResource(String context, boolean optional, ConsulConfigProperties properties,
ConsulPropertySources consulPropertySources) {
this.properties = properties;
Expand All @@ -43,6 +49,7 @@ public String getContext() {
return this.context;
}

@Deprecated
public boolean isOptional() {
return this.optional;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public List<String> getAutomaticContexts(List<String> profiles) {
}

protected String getContext(String prefix, String context) {
if (StringUtils.isEmpty(prefix)) {
if (!StringUtils.hasText(prefix)) {
return context;
}
else {
Expand All @@ -95,8 +95,14 @@ private void addProfiles(List<String> contexts, String baseContext, List<String>
}
}

@Deprecated
public ConsulPropertySource createPropertySource(String propertySourceContext, boolean optional,
ConsulClient consul, BiConsumer<String, Long> indexConsumer) {
return createPropertySource(propertySourceContext, consul, indexConsumer);
}

public ConsulPropertySource createPropertySource(String propertySourceContext, ConsulClient consul,
BiConsumer<String, Long> indexConsumer) {
try {
ConsulPropertySource propertySource = null;

Expand All @@ -109,9 +115,6 @@ public ConsulPropertySource createPropertySource(String propertySourceContext, b
filesPropertySource.init(response.getValue());
propertySource = filesPropertySource;
}
else if (!optional) {
throw new PropertySourceNotFoundException(propertySourceContext);
}
}
else {
propertySource = create(propertySourceContext, consul, indexConsumer);
Expand All @@ -122,7 +125,7 @@ else if (!optional) {
throw e;
}
catch (Exception e) {
if (properties.isFailFast() || !optional) {
if (properties.isFailFast()) {
throw new PropertySourceNotFoundException(propertySourceContext, e);
}
else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
/*
* Copyright 2013-2019 the original author or authors.
*
* Licensed 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
*
* https://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.springframework.cloud.consul.config;

import java.util.UUID;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import com.ecwid.consul.v1.ConsulClient;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import org.springframework.boot.WebApplicationType;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.builder.SpringApplicationBuilder;
import org.springframework.cloud.consul.test.ConsulTestcontainers;
import org.springframework.cloud.context.environment.EnvironmentChangeEvent;
import org.springframework.context.ApplicationListener;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.test.annotation.DirtiesContext;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author Spencer Gibb
*/
@DirtiesContext
public class ConsulConfigDataFileIntegrationTests {

private static final String APP_NAME = "testConsulConfigDataFile";

private static final String PREFIX = "_configDataIntegrationFileTests_config__";

private static final String ROOT = PREFIX + UUID.randomUUID();

private static final String VALUE1 = "testPropVal";

private static final String TEST_PROP = "testProp";

private static final String TEST_PROP_CANONICAL = "test-prop";

private static final String KEY1 = ROOT + "/application.properties";

private static final String VALUE2 = "testPropVal2";

private static final String TEST_PROP2 = "testProp2";

private static final String TEST_PROP2_CANONICAL = "test-prop2";

private static final String TEST_PROP3 = "testProp3";

private static final String TEST_PROP3_CANONICAL = "test-prop3";

private static final String KEY3 = ROOT + "/" + APP_NAME + ".properties";

private static ConfigurableApplicationContext context;

private static ConfigurableEnvironment environment;

private static ConsulClient client;

@BeforeAll
public static void setup() {
ConsulTestcontainers.start();
client = ConsulTestcontainers.client();
client.deleteKVValues(PREFIX);
client.setKVValue(KEY1, TEST_PROP + "=" + VALUE1 + "\n" + TEST_PROP2 + "=" + VALUE2);

context = new SpringApplicationBuilder(Config.class).web(WebApplicationType.NONE).run(
"--spring.application.name=" + APP_NAME, "--spring.cloud.consul.config.format=files",
"--spring.config.import=optional:consul:" + ConsulTestcontainers.getHost() + ":"
+ ConsulTestcontainers.getPort(),
"--spring.cloud.consul.config.prefix=" + ROOT, "--spring.cloud.consul.config.watch.delay=10");

client = context.getBean(ConsulClient.class);
environment = context.getEnvironment();
}

@AfterAll
public static void teardown() {
client.deleteKVValues(PREFIX);
if (context != null) {
context.close();
}
}

@Test
public void propertyLoaded() {
String testProp = environment.getProperty(TEST_PROP_CANONICAL);
assertThat(testProp).as(TEST_PROP + " was wrong").isEqualTo(VALUE1);
String testProp2 = environment.getProperty(TEST_PROP2_CANONICAL);
assertThat(testProp2).as(TEST_PROP2 + " was wrong").isEqualTo(VALUE2);
}

@Test
public void propertyLoadedAndUpdated() throws Exception {
String testProp = environment.getProperty(TEST_PROP_CANONICAL);
assertThat(testProp).as("testProp was wrong").isEqualTo(VALUE1);

client.setKVValue(KEY1, TEST_PROP + "=testPropValUpdate\n" + TEST_PROP2 + "=" + VALUE2);

CountDownLatch latch = context.getBean("countDownLatch1", CountDownLatch.class);
boolean receivedEvent = latch.await(15, TimeUnit.SECONDS);
assertThat(receivedEvent).as("listener didn't receive event").isTrue();

testProp = environment.getProperty(TEST_PROP_CANONICAL);
assertThat(testProp).as("testProp was wrong after update").isEqualTo("testPropValUpdate");
}

@Test
public void contextDoesNotExistThenExists() throws Exception {
String testProp = environment.getProperty(TEST_PROP3_CANONICAL);
assertThat(testProp).as(TEST_PROP3 + " was wrong").isNull();

client.setKVValue(KEY3, TEST_PROP3 + "=testPropValInsert");

CountDownLatch latch = context.getBean("countDownLatch2", CountDownLatch.class);
boolean receivedEvent = latch.await(15, TimeUnit.SECONDS);
assertThat(receivedEvent).as("listener didn't receive event").isTrue();

testProp = environment.getProperty(TEST_PROP3_CANONICAL);
assertThat(testProp).as(TEST_PROP3 + " was wrong after update").isEqualTo("testPropValInsert");
}

@Configuration
@EnableAutoConfiguration
static class Config implements ApplicationListener<EnvironmentChangeEvent> {

@Bean
public CountDownLatch countDownLatch1() {
return new CountDownLatch(1);
}

@Bean
public CountDownLatch countDownLatch2() {
return new CountDownLatch(1);
}

@Override
public void onApplicationEvent(EnvironmentChangeEvent event) {
if (event.getKeys().contains(TEST_PROP)) {
countDownLatch1().countDown();
}
else if (event.getKeys().contains(TEST_PROP3)) {
countDownLatch2().countDown();
}
}

}

}

0 comments on commit 695a8b0

Please sign in to comment.