From 913a5b997c6b777a1f54ab599afac92f730b61f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Anchieta=20Santiago?= Date: Tue, 29 Nov 2022 01:30:41 +0100 Subject: [PATCH] Fix getLocations that was ignoring failOnCompositeError flag (#2191) Fixes gh-2110 --- .../CompositeEnvironmentRepository.java | 2 +- ...rchPathCompositeEnvironmentRepository.java | 16 +++++-- .../CompositeEnvironmentRepositoryTests.java | 44 +++++++++++++++++++ 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/CompositeEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/CompositeEnvironmentRepository.java index f7354001..3ee8908b 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/CompositeEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/CompositeEnvironmentRepository.java @@ -37,7 +37,7 @@ public class CompositeEnvironmentRepository implements EnvironmentRepository { protected List environmentRepositories; - private boolean failOnError; + protected boolean failOnError; /** * Creates a new {@link CompositeEnvironmentRepository}. diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SearchPathCompositeEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SearchPathCompositeEnvironmentRepository.java index c9669ec1..1ddb0842 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SearchPathCompositeEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SearchPathCompositeEnvironmentRepository.java @@ -43,9 +43,19 @@ public class SearchPathCompositeEnvironmentRepository extends CompositeEnvironme public Locations getLocations(String application, String profile, String label) { List locations = new ArrayList<>(); for (EnvironmentRepository repo : this.environmentRepositories) { - if (repo instanceof SearchPathLocator) { - locations.addAll(Arrays - .asList(((SearchPathLocator) repo).getLocations(application, profile, label).getLocations())); + try { + if (repo instanceof SearchPathLocator) { + locations.addAll(Arrays.asList( + ((SearchPathLocator) repo).getLocations(application, profile, label).getLocations())); + } + } + catch (RepositoryException ex) { + if (failOnError) { + throw ex; + } + else { + log.info("Error finding locations for " + repo, ex); + } } } return new Locations(application, profile, label, null, locations.toArray(new String[locations.size()])); diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/CompositeEnvironmentRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/CompositeEnvironmentRepositoryTests.java index 5de61bc7..e5a43b37 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/CompositeEnvironmentRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/CompositeEnvironmentRepositoryTests.java @@ -33,6 +33,7 @@ import org.springframework.context.annotation.Primary; import org.springframework.core.Ordered; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -175,6 +176,36 @@ public class CompositeEnvironmentRepositoryTests { assertThat(propertySources.get(0).getName()).isEqualTo("p1"); } + @Test + public void testFailOnErrorFlagFalseForGetLocations() { + String sLoc1 = "loc1"; + Environment e1 = new Environment("app", "dev"); + SearchPathLocator.Locations loc1 = new SearchPathLocator.Locations("app", "dev", "label", "version", + new String[] { sLoc1 }); + List repos = new ArrayList(); + repos.add(new TestFailingLocationRepository(1, e1, loc1)); + + SearchPathCompositeEnvironmentRepository compositeRepo = new SearchPathCompositeEnvironmentRepository(repos, + false); + SearchPathLocator.Locations locations = compositeRepo.getLocations("app", "dev", "label"); + assertThat(locations.getLocations()).isEmpty(); + } + + @Test + public void testFailOnErrorFlagTrueForGetLocations() { + String sLoc1 = "loc1"; + Environment e1 = new Environment("app", "dev"); + SearchPathLocator.Locations loc1 = new SearchPathLocator.Locations("app", "dev", "label", "version", + new String[] { sLoc1 }); + List repos = new ArrayList(); + repos.add(new TestFailingLocationRepository(1, e1, loc1)); + + SearchPathCompositeEnvironmentRepository compositeRepo = new SearchPathCompositeEnvironmentRepository(repos, + true); + assertThatExceptionOfType(RepositoryException.class) + .isThrownBy(() -> compositeRepo.getLocations("app", "dev", "label")); + } + private static class TestOrderedEnvironmentRepository implements EnvironmentRepository, SearchPathLocator, Ordered { private Environment env; @@ -229,6 +260,19 @@ public class CompositeEnvironmentRepositoryTests { } + private static class TestFailingLocationRepository extends TestOrderedEnvironmentRepository { + + TestFailingLocationRepository(int order, Environment env, Locations locations) { + super(order, env, locations); + } + + @Override + public Locations getLocations(String application, String profile, String label) { + throw new RepositoryException("Failing for some reason"); + } + + } + @Configuration(proxyBeanMethods = false) static class OverrideCompositeConfig {