Implement a better equals and hashcode method ConfigServerConfigDataResource (#2278)

Spring Boot keeps a list of ConfigDataResources and will call contains on the list to determine whether it should load the resource.  Our hashcode and equals methods were using variables in ConfigServerConfigDataResource which had no bearing on the configuration that would be loaded, so this change simplifies this to make sure two ConfigServerConnfigDataResources would be considered equal if they would end up making the same request to the config server for configuration.

Co-authored-by: Ryan Baxter <524254+ryanjbaxter@users.noreply.github.com>
This commit is contained in:
Ryan Baxter
2023-06-16 15:34:46 -04:00
committed by GitHub
parent e9c8570803
commit 9f3ad4f04b
4 changed files with 383 additions and 9 deletions

View File

@@ -48,12 +48,17 @@ public class ConfigClientProperties {
*/
public static final String PREFIX = "spring.cloud.config";
/**
* Default application name.
*/
public static final String DEFAULT_APPLICATION = "application";
/**
* Placeholder string that allows ${spring.cloud.config.name} to override
* ${spring.application.name:application}.
*/
public static final String NAME_PLACEHOLDER = "${" + ConfigClientProperties.PREFIX
+ ".name:${spring.application.name:application}}";
+ ".name:${spring.application.name:" + DEFAULT_APPLICATION + "}}";
/**
* Name of config discovery enabled property.
@@ -94,12 +99,12 @@ public class ConfigClientProperties {
/**
* Name of application used to fetch remote properties.
*/
@Value("${spring.application.name:application}")
@Value("${spring.application.name:" + DEFAULT_APPLICATION + "}")
private String name;
/**
* The label name to use to pull remote configuration properties. The default is set
* on the server (generally "master" for a git based server).
* on the server (generally "main" for a git based server).
*/
private String label;

View File

@@ -16,6 +16,7 @@
package org.springframework.cloud.config.client;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
@@ -25,7 +26,13 @@ import org.apache.commons.logging.Log;
import org.springframework.boot.context.config.ConfigDataResource;
import org.springframework.boot.context.config.Profiles;
import org.springframework.core.style.ToStringCreator;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
import org.springframework.web.util.UriComponents;
import org.springframework.web.util.UriComponentsBuilder;
import static org.springframework.cloud.config.client.ConfigClientProperties.DEFAULT_APPLICATION;
import static org.springframework.cloud.config.client.ConfigClientProperties.DEFAULT_PROFILE;
public class ConfigServerConfigDataResource extends ConfigDataResource {
@@ -95,6 +102,41 @@ public class ConfigServerConfigDataResource extends ConfigDataResource {
this.retryProperties = retryProperties;
}
private String getApplicationName() {
return ObjectUtils.isEmpty(this.properties.getName()) ? DEFAULT_APPLICATION : this.getProperties().getName();
}
private String getProfilesForEquals() {
return ObjectUtils.isEmpty(this.getProfiles()) ? DEFAULT_PROFILE : this.getProfiles();
}
private boolean urisEqual(String[] thatUris) {
if (this.properties.getUri().length != thatUris.length) {
return false;
}
for (String uri : this.properties.getUri()) {
if (Arrays.stream(thatUris).noneMatch(thatUri -> uriEqual(uri, thatUri))) {
return false;
}
}
return true;
}
private boolean uriEqual(String thisUriString, String thatUriString) {
UriComponents thisUri = UriComponentsBuilder.fromHttpUrl(thisUriString).build();
UriComponents thatUri = UriComponentsBuilder.fromHttpUrl(thatUriString).build();
return Objects.equals(thisUri.getHost(), thatUri.getHost())
&& Objects.equals(thisUri.getPort(), thatUri.getPort())
&& Objects.equals(thisUri.getPath(), thatUri.getPath());
}
private int urisHashCode(String[] uris) {
return Arrays.stream(uris).mapToInt(uriString -> {
UriComponents uriComponents = UriComponentsBuilder.fromHttpUrl(uriString).build();
return Objects.hash(uriComponents.getHost(), uriComponents.getPath(), uriComponents.getPort());
}).sum();
}
@Override
public boolean equals(Object o) {
if (this == o) {
@@ -104,19 +146,25 @@ public class ConfigServerConfigDataResource extends ConfigDataResource {
return false;
}
ConfigServerConfigDataResource that = (ConfigServerConfigDataResource) o;
return Objects.equals(this.properties, that.properties) && Objects.equals(this.optional, that.optional)
&& Objects.equals(this.profiles, that.profiles);
return urisEqual(that.properties.getUri())
&& Objects.equals(this.getApplicationName(), that.getApplicationName())
&& Objects.equals(this.properties.getLabel(), that.properties.getLabel())
&& Objects.equals(this.getProfilesForEquals(), that.getProfilesForEquals())
&& Objects.equals(this.optional, that.optional);
}
@Override
public int hashCode() {
return Objects.hash(this.properties, this.optional, this.profiles);
String[] uris = properties.getUri();
String name = properties.getName();
String label = properties.getLabel();
return Objects.hash(urisHashCode(uris), name, label, optional, getProfilesForEquals());
}
@Override
public String toString() {
return new ToStringCreator(this).append("uris", properties.getUri()).append("optional", optional)
.append("profiles", getAcceptedProfiles()).toString();
.append("profiles", getProfiles()).toString();
}

View File

@@ -46,7 +46,7 @@ import static org.mockito.Mockito.when;
* spring.application.name will be set.
*
* At this point Boot has collected all active profiles so it will load all
* spring.config.import statements again with active profiles. The subsequent 2 calls then
* spring.config.import statements again with active profiles. The subsequent call then
* will not have spring.application.name set in the context so the config server config
* data loader will make a request to the config server with the correct application name.
*
@@ -73,7 +73,7 @@ public class ConfigClientConfigDataLoaderTest {
verify(rest).exchange(eq("http://localhost:8888/{name}/{profile}"), eq(HttpMethod.GET),
ArgumentMatchers.any(HttpEntity.class), eq(Environment.class), eq("application"),
ArgumentMatchers.<String>any());
verify(rest, times(2)).exchange(eq("http://localhost:8888/{name}/{profile}"), eq(HttpMethod.GET),
verify(rest, times(1)).exchange(eq("http://localhost:8888/{name}/{profile}"), eq(HttpMethod.GET),
ArgumentMatchers.any(HttpEntity.class), eq(Environment.class), eq("foo"),
ArgumentMatchers.<String>any());
}

View File

@@ -0,0 +1,321 @@
/*
* Copyright 2013-2023 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.config.client;
import java.util.Arrays;
import java.util.Collections;
import org.junit.jupiter.api.Test;
import org.springframework.boot.context.config.Profiles;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
/**
* @author Ryan Baxter
*/
class ConfigServerConfigDataResourceTests {
@Test
void testEquals() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true,
mock(Profiles.class));
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testEqualsDifferentSchemes() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "https://localhost:8888" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true,
mock(Profiles.class));
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testEqualsDifferentHosts() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://remotehost:8888" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true,
mock(Profiles.class));
assertThat(r1).isNotEqualTo(r2);
assertThat(r1.hashCode()).isNotEqualTo(r2.hashCode());
}
@Test
void testEqualsDifferentPorts() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true,
mock(Profiles.class));
assertThat(r1).isNotEqualTo(r2);
assertThat(r1.hashCode()).isNotEqualTo(r2.hashCode());
}
@Test
void testEqualsOptionalFalse() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, false,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, false,
mock(Profiles.class));
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testEqualsSamePath() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888/p1" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888/p1" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true,
mock(Profiles.class));
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testEqualsDifferentPath() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888/p1" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888/p2" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true,
mock(Profiles.class));
assertThat(r1).isNotEqualTo(r2);
assertThat(r1.hashCode()).isNotEqualTo(r2.hashCode());
}
@Test
void testMultipleUrisDifferentOrder() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888", "http://localhost:9999", "http://localhost:7777" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:7777", "http://localhost:8888", "http://localhost:9999" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true,
mock(Profiles.class));
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testMultipleUrisDifferentOrderWithDifferentSchemes() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888", "http://localhost:9999", "http://localhost:7777" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties
.setUri(new String[] { "https://localhost:7777", "https://localhost:8888", "https://localhost:9999" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true,
mock(Profiles.class));
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testMultipleUrisDifferentLengths() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888", "http://localhost:9999", "http://localhost:7777" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:7777", "http://localhost:8888", "http://localhost:9999",
"http://localhost:6666" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true,
mock(Profiles.class));
assertThat(r1).isNotEqualTo(r2);
assertThat(r1.hashCode()).isNotEqualTo(r2.hashCode());
}
@Test
void testEqualsOptionalFalseAndTrue() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true,
mock(Profiles.class));
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, false,
mock(Profiles.class));
assertThat(r1).isNotEqualTo(r2);
assertThat(r1.hashCode()).isNotEqualTo(r2.hashCode());
}
@Test
void testEqualsNullProfilesAndDefaultProfile() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true, null);
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true,
mock(Profiles.class));
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testEqualsNullProfiles() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true, null);
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true, null);
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testEqualsProfilesAndProperties() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
Profiles r1Profiles = mock(Profiles.class);
when(r1Profiles.getAccepted()).thenReturn(Collections.singletonList("foo"));
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
r2Properties.setProfile("foo");
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true, r1Profiles);
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true, null);
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testEqualsProfiles() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
Profiles r1Profiles = mock(Profiles.class);
when(r1Profiles.getAccepted()).thenReturn(Collections.singletonList("foo"));
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
Profiles r2Profiles = mock(Profiles.class);
when(r2Profiles.getAccepted()).thenReturn(Collections.singletonList("foo"));
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true, r1Profiles);
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true, r2Profiles);
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testEqualsProfilesDifferent() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
Profiles r1Profiles = mock(Profiles.class);
when(r1Profiles.getAccepted()).thenReturn(Arrays.asList("foo", "bar"));
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
Profiles r2Profiles = mock(Profiles.class);
when(r2Profiles.getAccepted()).thenReturn(Collections.singletonList("foo"));
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true, r1Profiles);
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true, r2Profiles);
assertThat(r1).isNotEqualTo(r2);
assertThat(r1.hashCode()).isNotEqualTo(r2.hashCode());
}
@Test
void testEqualsProfilesDifferentOrder() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
Profiles r1Profiles = mock(Profiles.class);
when(r1Profiles.getAccepted()).thenReturn(Arrays.asList("foo", "bar"));
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
Profiles r2Profiles = mock(Profiles.class);
when(r2Profiles.getAccepted()).thenReturn(Arrays.asList("bar", "foo"));
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true, r1Profiles);
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true, r2Profiles);
assertThat(r1).isNotEqualTo(r2);
assertThat(r1.hashCode()).isNotEqualTo(r2.hashCode());
}
@Test
void testEqualsProfilesProperties() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
Profiles r1Profiles = mock(Profiles.class);
when(r1Profiles.getAccepted()).thenReturn(Arrays.asList("foo", "bar"));
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
r2Properties.setProfile("foo,bar");
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true, r1Profiles);
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true, null);
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testEqualsLabels() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
r1Properties.setLabel("foo");
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
r2Properties.setLabel("foo");
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true, null);
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true, null);
assertThat(r1).isEqualTo(r2);
assertThat(r1.hashCode()).isEqualTo(r2.hashCode());
}
@Test
void testEqualsDifferentLabels() {
ConfigClientProperties r1Properties = new ConfigClientProperties();
r1Properties.setUri(new String[] { "http://localhost:8888" });
r1Properties.setLabel("foo");
ConfigClientProperties r2Properties = new ConfigClientProperties();
r2Properties.setUri(new String[] { "http://localhost:8888" });
ConfigServerConfigDataResource r1 = new ConfigServerConfigDataResource(r1Properties, true, null);
ConfigServerConfigDataResource r2 = new ConfigServerConfigDataResource(r2Properties, true, null);
assertThat(r1).isNotEqualTo(r2);
assertThat(r1.hashCode()).isNotEqualTo(r2.hashCode());
}
}