From c979e2e8810081b794b014ed521f5848c186ffae Mon Sep 17 00:00:00 2001 From: "Shanyou Yu (Sean Yu)" Date: Mon, 17 Apr 2023 11:59:21 +0800 Subject: [PATCH] fix: custom spi metadata loaded bug (#973) --- CHANGELOG.md | 2 +- pom.xml | 2 +- .../metadata/StaticMetadataManager.java | 28 +++---- .../metadata/StaticMetadataManagerTest.java | 80 ++++++++++++++++--- spring-cloud-tencent-dependencies/pom.xml | 2 +- 5 files changed, 87 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15c44dd50..19a925081 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ # Change Log --- - +- [fix: custom spi metadata loaded bug.](https://github.com/Tencent/spring-cloud-tencent/pull/973) diff --git a/pom.xml b/pom.xml index 9dc723048..91bbb915e 100644 --- a/pom.xml +++ b/pom.xml @@ -88,7 +88,7 @@ - 1.12.0-2021.0.6-SNAPSHOT + 1.11.1-2021.0.6 5.3.25 diff --git a/spring-cloud-tencent-commons/src/main/java/com/tencent/cloud/common/metadata/StaticMetadataManager.java b/spring-cloud-tencent-commons/src/main/java/com/tencent/cloud/common/metadata/StaticMetadataManager.java index 3bf69dacd..d37deb006 100644 --- a/spring-cloud-tencent-commons/src/main/java/com/tencent/cloud/common/metadata/StaticMetadataManager.java +++ b/spring-cloud-tencent-commons/src/main/java/com/tencent/cloud/common/metadata/StaticMetadataManager.java @@ -184,26 +184,24 @@ public class StaticMetadataManager { @SuppressWarnings("DuplicatedCode") private void parseCustomMetadata(List instanceMetadataProviders) { - if (CollectionUtils.isEmpty(instanceMetadataProviders)) { - customSPIMetadata = Collections.emptyMap(); - customSPITransitiveMetadata = Collections.emptyMap(); - customSPIDisposableMetadata = Collections.emptyMap(); - return; + // init customSPIMetadata + customSPIMetadata = new HashMap<>(); + customSPITransitiveMetadata = new HashMap<>(); + customSPIDisposableMetadata = new HashMap<>(); + if (!CollectionUtils.isEmpty(instanceMetadataProviders)) { + instanceMetadataProviders.forEach(this::parseCustomMetadata); } - - instanceMetadataProviders.forEach(this::parseCustomMetadata); - + customSPIMetadata = Collections.unmodifiableMap(customSPIMetadata); + customSPITransitiveMetadata = Collections.unmodifiableMap(customSPITransitiveMetadata); + customSPIDisposableMetadata = Collections.unmodifiableMap(customSPIDisposableMetadata); } @SuppressWarnings("DuplicatedCode") private void parseCustomMetadata(InstanceMetadataProvider instanceMetadataProvider) { // resolve all metadata Map allMetadata = instanceMetadataProvider.getMetadata(); - if (allMetadata == null) { - customSPIMetadata = Collections.emptyMap(); - } - else { - customSPIMetadata = Collections.unmodifiableMap(allMetadata); + if (!CollectionUtils.isEmpty(allMetadata)) { + customSPIMetadata.putAll(allMetadata); } // resolve transitive metadata @@ -216,7 +214,7 @@ public class StaticMetadataManager { } } } - customSPITransitiveMetadata = Collections.unmodifiableMap(transitiveMetadata); + customSPITransitiveMetadata.putAll(transitiveMetadata); Set disposableKeys = instanceMetadataProvider.getDisposableMetadataKeys(); Map disposableMetadata = new HashMap<>(); @@ -227,7 +225,7 @@ public class StaticMetadataManager { } } } - customSPIDisposableMetadata = Collections.unmodifiableMap(disposableMetadata); + customSPIDisposableMetadata.putAll(disposableMetadata); } private void merge() { diff --git a/spring-cloud-tencent-commons/src/test/java/com/tencent/cloud/common/metadata/StaticMetadataManagerTest.java b/spring-cloud-tencent-commons/src/test/java/com/tencent/cloud/common/metadata/StaticMetadataManagerTest.java index 8dbda4cfe..784213c57 100644 --- a/spring-cloud-tencent-commons/src/test/java/com/tencent/cloud/common/metadata/StaticMetadataManagerTest.java +++ b/spring-cloud-tencent-commons/src/test/java/com/tencent/cloud/common/metadata/StaticMetadataManagerTest.java @@ -18,6 +18,7 @@ package com.tencent.cloud.common.metadata; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -26,15 +27,27 @@ import java.util.Set; import com.tencent.cloud.common.metadata.config.MetadataLocalProperties; import com.tencent.cloud.common.spi.InstanceMetadataProvider; +import com.tencent.cloud.common.spi.impl.DefaultInstanceMetadataProvider; +import com.tencent.cloud.common.util.ApplicationContextAwareUtils; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import uk.org.webcompere.systemstubs.environment.EnvironmentVariables; import uk.org.webcompere.systemstubs.jupiter.SystemStub; import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension; +import static com.tencent.cloud.common.constant.MetadataConstant.DefaultMetadata.DEFAULT_METADATA_SOURCE_SERVICE_NAME; +import static com.tencent.cloud.common.constant.MetadataConstant.DefaultMetadata.DEFAULT_METADATA_SOURCE_SERVICE_NAMESPACE; +import static com.tencent.polaris.test.common.Consts.NAMESPACE_TEST; +import static com.tencent.polaris.test.common.Consts.SERVICE_PROVIDER; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.when; /** @@ -53,6 +66,26 @@ public class StaticMetadataManagerTest { @Mock private MetadataLocalProperties metadataLocalProperties; + private static MockedStatic mockedApplicationContextAwareUtils; + + @BeforeAll + static void beforeAll() { + mockedApplicationContextAwareUtils = Mockito.mockStatic(ApplicationContextAwareUtils.class); + mockedApplicationContextAwareUtils.when(() -> ApplicationContextAwareUtils.getProperties(anyString())) + .thenReturn("unit-test"); + } + + @AfterAll + static void afterAll() { + mockedApplicationContextAwareUtils.close(); + } + + @BeforeEach + void setUp() { + MetadataContext.LOCAL_NAMESPACE = NAMESPACE_TEST; + MetadataContext.LOCAL_SERVICE = SERVICE_PROVIDER; + } + @Test public void testParseConfigMetadata() { Map content = new HashMap<>(); @@ -63,6 +96,7 @@ public class StaticMetadataManagerTest { when(metadataLocalProperties.getContent()).thenReturn(content); when(metadataLocalProperties.getTransitive()).thenReturn(Collections.singletonList("k1")); + when(metadataLocalProperties.getDisposable()).thenReturn(Collections.singletonList("k1")); StaticMetadataManager metadataManager = new StaticMetadataManager(metadataLocalProperties, null); @@ -75,6 +109,10 @@ public class StaticMetadataManagerTest { assertThat(transitiveMetadata.size()).isEqualTo(1); assertThat(transitiveMetadata.get("k1")).isEqualTo("v1"); + Map disposableMetadata = metadataManager.getConfigDisposableMetadata(); + assertThat(disposableMetadata.size()).isEqualTo(1); + assertThat(disposableMetadata.get("k1")).isEqualTo("v1"); + assertThat(metadataManager.getZone()).isEqualTo("zone1"); assertThat(metadataManager.getRegion()).isEqualTo("region1"); @@ -93,17 +131,25 @@ public class StaticMetadataManagerTest { when(metadataLocalProperties.getTransitive()).thenReturn(Collections.singletonList("k1")); StaticMetadataManager metadataManager = new StaticMetadataManager(metadataLocalProperties, - Collections.singletonList(new MockedMetadataProvider())); + Arrays.asList(new MockedMetadataProvider(), new DefaultInstanceMetadataProvider(null))); Map metadata = metadataManager.getAllCustomMetadata(); - assertThat(metadata.size()).isEqualTo(3); + assertThat(metadata.size()).isEqualTo(5); assertThat(metadata.get("k1")).isEqualTo("v1"); assertThat(metadata.get("k2")).isEqualTo("v22"); assertThat(metadata.get("k3")).isEqualTo("v33"); + assertThat(metadata.get(DEFAULT_METADATA_SOURCE_SERVICE_NAMESPACE)).isEqualTo(NAMESPACE_TEST); + assertThat(metadata.get(DEFAULT_METADATA_SOURCE_SERVICE_NAME)).isEqualTo(SERVICE_PROVIDER); Map transitiveMetadata = metadataManager.getCustomSPITransitiveMetadata(); assertThat(transitiveMetadata.size()).isEqualTo(1); - assertThat(metadata.get("k2")).isEqualTo("v22"); + assertThat(transitiveMetadata.get("k2")).isEqualTo("v22"); + + Map disposableMetadata = metadataManager.getCustomSPIDisposableMetadata(); + assertThat(disposableMetadata.size()).isEqualTo(3); + assertThat(disposableMetadata.get("k3")).isEqualTo("v33"); + assertThat(disposableMetadata.get(DEFAULT_METADATA_SOURCE_SERVICE_NAMESPACE)).isEqualTo(NAMESPACE_TEST); + assertThat(disposableMetadata.get(DEFAULT_METADATA_SOURCE_SERVICE_NAME)).isEqualTo(SERVICE_PROVIDER); assertThat(metadataManager.getZone()).isEqualTo("zone2"); assertThat(metadataManager.getRegion()).isEqualTo("region1"); @@ -126,29 +172,38 @@ public class StaticMetadataManagerTest { when(metadataLocalProperties.getTransitive()).thenReturn(Collections.singletonList("k1")); StaticMetadataManager metadataManager = new StaticMetadataManager(metadataLocalProperties, - Collections.singletonList(new MockedMetadataProvider())); + Arrays.asList(new MockedMetadataProvider(), new DefaultInstanceMetadataProvider(null))); Map metadata = metadataManager.getMergedStaticMetadata(); - assertThat(metadata.size()).isEqualTo(6); + assertThat(metadata.size()).isEqualTo(8); assertThat(metadata.get("k1")).isEqualTo("v1"); assertThat(metadata.get("k2")).isEqualTo("v22"); assertThat(metadata.get("k3")).isEqualTo("v33"); + assertThat(metadata.get(DEFAULT_METADATA_SOURCE_SERVICE_NAMESPACE)).isEqualTo(NAMESPACE_TEST); + assertThat(metadata.get(DEFAULT_METADATA_SOURCE_SERVICE_NAME)).isEqualTo(SERVICE_PROVIDER); Map transitiveMetadata = metadataManager.getMergedStaticTransitiveMetadata(); assertThat(transitiveMetadata.size()).isEqualTo(2); - assertThat(metadata.get("k1")).isEqualTo("v1"); - assertThat(metadata.get("k2")).isEqualTo("v22"); + assertThat(transitiveMetadata.get("k1")).isEqualTo("v1"); + assertThat(transitiveMetadata.get("k2")).isEqualTo("v22"); + + Map disposableMetadata = metadataManager.getMergedStaticDisposableMetadata(); + assertThat(disposableMetadata.size()).isEqualTo(3); + assertThat(disposableMetadata.get("k3")).isEqualTo("v33"); + assertThat(disposableMetadata.get(DEFAULT_METADATA_SOURCE_SERVICE_NAMESPACE)).isEqualTo(NAMESPACE_TEST); + assertThat(disposableMetadata.get(DEFAULT_METADATA_SOURCE_SERVICE_NAME)).isEqualTo(SERVICE_PROVIDER); assertThat(metadataManager.getAllEnvMetadata()).isEmpty(); assertThat(metadataManager.getEnvTransitiveMetadata()).isEmpty(); assertThat(metadataManager.getZone()).isEqualTo("zone2"); assertThat(metadataManager.getRegion()).isEqualTo("region1"); + assertThat(metadataManager.getCampus()).isEqualTo("campus2"); Map locationInfo = metadataManager.getLocationMetadata(); assertThat(locationInfo.get("zone")).isEqualTo("zone2"); assertThat(locationInfo.get("region")).isEqualTo("region1"); - assertThat(locationInfo.get("campus")).isEqualTo("campus1"); + assertThat(locationInfo.get("campus")).isEqualTo("campus2"); } @Test @@ -195,6 +250,13 @@ public class StaticMetadataManagerTest { return transitiveKeys; } + @Override + public Set getDisposableMetadataKeys() { + Set transitiveKeys = new HashSet<>(); + transitiveKeys.add("k3"); + return transitiveKeys; + } + @Override public String getRegion() { return "region1"; @@ -207,7 +269,7 @@ public class StaticMetadataManagerTest { @Override public String getCampus() { - return null; + return "campus2"; } } } diff --git a/spring-cloud-tencent-dependencies/pom.xml b/spring-cloud-tencent-dependencies/pom.xml index 640fe212b..14eae51e9 100644 --- a/spring-cloud-tencent-dependencies/pom.xml +++ b/spring-cloud-tencent-dependencies/pom.xml @@ -70,7 +70,7 @@ - 1.12.0-2021.0.6-SNAPSHOT + 1.11.1-2021.0.6 1.12.0