minor code refactor + tests (#619)

* minor code refactor + tests

* more changes

* minor changes

* consistent names in method arguments

* latest master

* dummy commit to retrigger the build
This commit is contained in:
erabii
2020-09-29 20:19:06 -04:00
committed by GitHub
parent 2c863aee0f
commit 92cad59fdd
3 changed files with 169 additions and 22 deletions

View File

@@ -120,6 +120,7 @@
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>io.fabric8</groupId>
<artifactId>kubernetes-client</artifactId>
@@ -136,6 +137,7 @@
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>io.fabric8</groupId>
<artifactId>mockwebserver</artifactId>

View File

@@ -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<String, Object> s1 = mp1.getSource();
Map<String, Object> s2 = mp2.getSource();
return s1 == null ? s2 != null : !s1.equals(s2);
Map<String, Object> leftMap = left.getSource();
Map<String, Object> rightMap = right.getSource();
return !Objects.equals(leftMap, rightMap);
}
protected boolean changed(List<? extends MapPropertySource> l1, List<? extends MapPropertySource> l2) {
protected boolean changed(List<? extends MapPropertySource> left,
List<? extends MapPropertySource> 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 <E> LinkedList<E> toLinkedList(Iterable<E> it) {
LinkedList<E> list = new LinkedList<E>();
LinkedList<E> list = new LinkedList<>();
for (E e : it) {
list.add(e);
}
@@ -177,13 +179,15 @@ public abstract class ConfigurationChangeDetector {
Environment environment) {
List<MapPropertySource> 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 {

View File

@@ -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<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
Map<String, Object> 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<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
leftMap.put("anotherKey", value);
Map<String, Object> 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<MapPropertySource> left = Collections
.singletonList(new MapPropertySource("one", Collections.emptyMap()));
List<MapPropertySource> right = Collections.emptyList();
boolean changed = stub.changed(left, right);
Assert.assertFalse(changed);
}
@Test
public void testChangedListSameSizesButNotEqual() {
Object value = new Object();
Map<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
Map<String, Object> rightMap = new HashMap<>();
leftMap.put("anotherKey", value);
List<MapPropertySource> left = Collections
.singletonList(new MapPropertySource("one", leftMap));
List<MapPropertySource> 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<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
Map<String, Object> rightMap = new HashMap<>();
leftMap.put("key", value);
List<MapPropertySource> left = Collections
.singletonList(new MapPropertySource("one", leftMap));
List<MapPropertySource> 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);
}
}
}