diff --git a/spring-cloud-kubernetes-config/pom.xml b/spring-cloud-kubernetes-config/pom.xml index c6fc4afd..3038062e 100644 --- a/spring-cloud-kubernetes-config/pom.xml +++ b/spring-cloud-kubernetes-config/pom.xml @@ -120,6 +120,7 @@ + io.fabric8 kubernetes-client @@ -136,6 +137,7 @@ + io.fabric8 mockwebserver diff --git a/spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetector.java b/spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetector.java index a272f28b..165b1c2b 100644 --- a/spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetector.java +++ b/spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetector.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; import javax.annotation.PreDestroy; @@ -76,34 +77,34 @@ public abstract class ConfigurationChangeDetector { /** * Determines if two property sources are different. - * @param mp1 map property sources 1 - * @param mp2 map property sources 2 + * @param left left map property sources + * @param right right map property sources * @return {@code true} if source has changed */ - protected boolean changed(MapPropertySource mp1, MapPropertySource mp2) { - if (mp1 == mp2) { + protected boolean changed(MapPropertySource left, MapPropertySource right) { + if (left == right) { return false; } - if (mp1 == null && mp2 != null || mp1 != null && mp2 == null) { + if (left == null || right == null) { return true; } - - Map s1 = mp1.getSource(); - Map s2 = mp2.getSource(); - - return s1 == null ? s2 != null : !s1.equals(s2); + Map leftMap = left.getSource(); + Map rightMap = right.getSource(); + return !Objects.equals(leftMap, rightMap); } - protected boolean changed(List l1, List l2) { + protected boolean changed(List left, + List right) { - if (l1.size() != l2.size()) { - this.log.warn("The current number of ConfigMap PropertySources does not match " - + "the ones loaded from the Kubernetes - No reload will take place"); + if (left.size() != right.size()) { + this.log.warn( + "The current number of ConfigMap PropertySources does not match " + + "the ones loaded from the Kubernetes - No reload will take place"); return false; } - for (int i = 0; i < l1.size(); i++) { - if (changed(l1.get(i), l2.get(i))) { + for (int i = 0; i < left.size(); i++) { + if (changed(left.get(i), right.get(i))) { return true; } } @@ -146,8 +147,9 @@ public abstract class ConfigurationChangeDetector { else if (sourceClass.isInstance(source)) { managedSources.add(sourceClass.cast(source)); } - else if (BootstrapPropertySource.class.isInstance(source)) { - PropertySource propertySource = ((BootstrapPropertySource) source).getDelegate(); + else if (source instanceof BootstrapPropertySource) { + PropertySource propertySource = ((BootstrapPropertySource) source) + .getDelegate(); if (sourceClass.isInstance(propertySource)) { sources.add(propertySource); } @@ -158,7 +160,7 @@ public abstract class ConfigurationChangeDetector { } private LinkedList toLinkedList(Iterable it) { - LinkedList list = new LinkedList(); + LinkedList list = new LinkedList<>(); for (E e : it) { list.add(e); } @@ -177,13 +179,15 @@ public abstract class ConfigurationChangeDetector { Environment environment) { List result = new ArrayList<>(); - PropertySource propertySource = propertySourceLocator.locate(environment); + PropertySource propertySource = propertySourceLocator.locate(environment); if (propertySource instanceof MapPropertySource) { result.add((MapPropertySource) propertySource); } else if (propertySource instanceof CompositePropertySource) { - result.addAll(((CompositePropertySource) propertySource).getPropertySources().stream() - .filter(p -> p instanceof MapPropertySource).map(p -> (MapPropertySource) p) + result.addAll(((CompositePropertySource) propertySource).getPropertySources() + .stream() + .filter(p -> p instanceof MapPropertySource) + .map(p -> (MapPropertySource) p) .collect(Collectors.toList())); } else { diff --git a/spring-cloud-kubernetes-config/src/test/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetectorTest.java b/spring-cloud-kubernetes-config/src/test/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetectorTest.java new file mode 100644 index 00000000..7122b794 --- /dev/null +++ b/spring-cloud-kubernetes-config/src/test/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetectorTest.java @@ -0,0 +1,141 @@ +/* + * Copyright 2013-2020 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.kubernetes.config.reload; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import io.fabric8.kubernetes.client.KubernetesClient; +import org.junit.Assert; +import org.junit.Test; + +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; + +/** + * @author wind57 + */ +public class ConfigurationChangeDetectorTest { + + private final ConfigurationChangeDetectorStub stub = new ConfigurationChangeDetectorStub( + null, null, null, null); + + @Test + public void testChangedTwoNulls() { + boolean changed = stub.changed(null, (MapPropertySource) null); + Assert.assertFalse(changed); + } + + @Test + public void testChangedLeftNullRightNonNull() { + MapPropertySource right = new MapPropertySource("rightNonNull", + Collections.emptyMap()); + boolean changed = stub.changed(null, right); + Assert.assertTrue(changed); + } + + @Test + public void testChangedLeftNonNullRightNull() { + MapPropertySource left = new MapPropertySource("leftNonNull", + Collections.emptyMap()); + boolean changed = stub.changed(left, null); + Assert.assertTrue(changed); + } + + @Test + public void testChangedEqualMaps() { + Object value = new Object(); + Map leftMap = new HashMap<>(); + leftMap.put("key", value); + Map rightMap = new HashMap<>(); + rightMap.put("key", value); + MapPropertySource left = new MapPropertySource("left", leftMap); + MapPropertySource right = new MapPropertySource("right", rightMap); + boolean changed = stub.changed(left, right); + Assert.assertFalse(changed); + } + + @Test + public void testChangedNonEqualMaps() { + Object value = new Object(); + Map leftMap = new HashMap<>(); + leftMap.put("key", value); + leftMap.put("anotherKey", value); + Map rightMap = new HashMap<>(); + rightMap.put("key", value); + MapPropertySource left = new MapPropertySource("left", leftMap); + MapPropertySource right = new MapPropertySource("right", rightMap); + boolean changed = stub.changed(left, right); + Assert.assertTrue(changed); + } + + @Test + public void testChangedListsDifferentSizes() { + List left = Collections + .singletonList(new MapPropertySource("one", Collections.emptyMap())); + List right = Collections.emptyList(); + boolean changed = stub.changed(left, right); + Assert.assertFalse(changed); + } + + @Test + public void testChangedListSameSizesButNotEqual() { + Object value = new Object(); + Map leftMap = new HashMap<>(); + leftMap.put("key", value); + Map rightMap = new HashMap<>(); + leftMap.put("anotherKey", value); + List left = Collections + .singletonList(new MapPropertySource("one", leftMap)); + List right = Collections + .singletonList(new MapPropertySource("two", rightMap)); + boolean changed = stub.changed(left, right); + Assert.assertTrue(changed); + } + + @Test + public void testChangedListSameSizesEqual() { + Object value = new Object(); + Map leftMap = new HashMap<>(); + leftMap.put("key", value); + Map rightMap = new HashMap<>(); + leftMap.put("key", value); + List left = Collections + .singletonList(new MapPropertySource("one", leftMap)); + List right = Collections + .singletonList(new MapPropertySource("two", rightMap)); + boolean changed = stub.changed(left, right); + Assert.assertTrue(changed); + } + + /** + * only needed to test some protected methods it defines + */ + private static final class ConfigurationChangeDetectorStub + extends ConfigurationChangeDetector { + + private ConfigurationChangeDetectorStub(ConfigurableEnvironment environment, + ConfigReloadProperties properties, KubernetesClient kubernetesClient, + ConfigurationUpdateStrategy strategy) { + super(environment, properties, kubernetesClient, strategy); + } + + } + +}