diff --git a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/client/NacosPropertySource.java b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/client/NacosPropertySource.java index ee3d1c6c13..e71dbbca55 100644 --- a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/client/NacosPropertySource.java +++ b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/client/NacosPropertySource.java @@ -24,6 +24,7 @@ import java.util.Map; import com.alibaba.cloud.nacos.NacosConfigProperties; +import org.jspecify.annotations.Nullable; import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.PropertySource; @@ -55,19 +56,43 @@ public class NacosPropertySource extends MapPropertySource { */ private final boolean isRefreshable; + /** + * File extension this property source was parsed with (e.g. {@code yaml}, + * {@code properties}, {@code json}). Captured so that subsequent refreshes + * can re-parse updated config with the same loader instead of falling back + * to the properties default. The value is the same notion the rest of the + * codebase tracks as {@code NacosConfigProperties#fileExtension} and + * {@code NacosItemConfig#suffix}. + */ + private final @Nullable String fileExtension; + NacosPropertySource(String group, String dataId, Map source, Date timestamp, boolean isRefreshable) { + this(group, dataId, source, timestamp, isRefreshable, null); + } + + private NacosPropertySource(String group, String dataId, Map source, + Date timestamp, boolean isRefreshable, + @Nullable String fileExtension) { super(String.join(NacosConfigProperties.COMMAS, dataId, group), source); this.group = group; this.dataId = dataId; this.timestamp = timestamp; this.isRefreshable = isRefreshable; + this.fileExtension = fileExtension; } public NacosPropertySource(List> propertySources, String group, String dataId, Date timestamp, boolean isRefreshable) { this(group, dataId, getSourceMap(group, dataId, propertySources), timestamp, - isRefreshable); + isRefreshable, null); + } + + public NacosPropertySource(List> propertySources, String group, + String dataId, Date timestamp, boolean isRefreshable, + @Nullable String fileExtension) { + this(group, dataId, getSourceMap(group, dataId, propertySources), timestamp, + isRefreshable, fileExtension); } private static Map getSourceMap(String group, String dataId, @@ -130,4 +155,14 @@ public boolean isRefreshable() { return isRefreshable; } + /** + * Returns the file extension this property source was parsed with, or + * {@code null} if it was built through a constructor that does not carry + * the extension. Used internally by the refresh listener to re-parse + * pushed Nacos content with the original format. + */ + public @Nullable String getFileExtension() { + return fileExtension; + } + } diff --git a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/client/NacosPropertySourceBuilder.java b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/client/NacosPropertySourceBuilder.java index d0969743d5..58384ac188 100644 --- a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/client/NacosPropertySourceBuilder.java +++ b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/client/NacosPropertySourceBuilder.java @@ -75,7 +75,7 @@ public NacosPropertySource build(String dataId, String group, String fileExtensi List> propertySources = loadNacosData(dataId, group, fileExtension); NacosPropertySource nacosPropertySource = new NacosPropertySource(propertySources, - group, dataId, new Date(), isRefreshable); + group, dataId, new Date(), isRefreshable, fileExtension); NacosPropertySourceRepository.collectNacosPropertySource(nacosPropertySource); return nacosPropertySource; } diff --git a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/configdata/NacosConfigDataLoader.java b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/configdata/NacosConfigDataLoader.java index 541fb95c2f..cda33c6584 100644 --- a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/configdata/NacosConfigDataLoader.java +++ b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/configdata/NacosConfigDataLoader.java @@ -92,7 +92,7 @@ public NacosConfigDataLoader(DeferredLogFactory logFactory) { NacosPropertySource propertySource = new NacosPropertySource(propertySources, config.getGroup(), config.getDataId(), new Date(), - config.isRefreshEnabled()); + config.isRefreshEnabled(), config.getSuffix()); NacosPropertySourceRepository.collectNacosPropertySource(propertySource); diff --git a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/refresh/NacosPropertySourceRefreshListener.java b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/refresh/NacosPropertySourceRefreshListener.java index fd40fcb495..828843b4a2 100644 --- a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/refresh/NacosPropertySourceRefreshListener.java +++ b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/main/java/com/alibaba/cloud/nacos/refresh/NacosPropertySourceRefreshListener.java @@ -40,6 +40,7 @@ import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; +import org.springframework.util.StringUtils; public class NacosPropertySourceRefreshListener implements BeanPostProcessor, SmartApplicationListener, ApplicationContextAware { @@ -114,9 +115,25 @@ public void handle(NacosConfigRefreshEvent event) { log.warn("Event dataId or group is null, skipping refresh"); return; } - NacosPropertySource newProperSource = nacosPropertySourceBuilder.build(dataId, group, "properties", ((NacosPropertySource) prevpropertySource).isRefreshable()); + NacosPropertySource prevNacosPropertySource = (NacosPropertySource) prevpropertySource; + String fileExtension = prevNacosPropertySource.getFileExtension(); + if (!StringUtils.hasText(fileExtension)) { + // The source was built through the legacy constructor (e.g. by + // third-party code) and did not record its format. We cannot + // safely guess the extension from global configuration here, + // because shared-configs / extension-configs may declare their + // own suffix per entry. Fall back to "properties" — the + // historical default — and log so the case is diagnosable. + log.warn( + "Previous Nacos property source did not record a file extension," + + " falling back to 'properties'; non-properties formats" + + " may be mis-parsed. sourceName={}", sourceName); + fileExtension = "properties"; + } + NacosPropertySource newProperSource = nacosPropertySourceBuilder.build(dataId, group, fileExtension, prevNacosPropertySource.isRefreshable()); target.replace(sourceName, newProperSource); - log.info("Replace Nacos Property Source : " + sourceName); + log.info("Replace Nacos Property Source: {} (fileExtension={})", + sourceName, fileExtension); } } diff --git a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/test/java/com/alibaba/cloud/nacos/client/NacosPropertySourceBuilderTest.java b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/test/java/com/alibaba/cloud/nacos/client/NacosPropertySourceBuilderTest.java new file mode 100644 index 0000000000..ad290bcd06 --- /dev/null +++ b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/test/java/com/alibaba/cloud/nacos/client/NacosPropertySourceBuilderTest.java @@ -0,0 +1,65 @@ +/* + * Copyright 2013-present 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 com.alibaba.cloud.nacos.client; + +import com.alibaba.nacos.api.config.ConfigService; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link NacosPropertySourceBuilder}. + */ +public class NacosPropertySourceBuilderTest { + + @Test + void builtPropertySourcePreservesFileExtension() throws Exception { + ConfigService configService = Mockito.mock(ConfigService.class); + Mockito.when(configService.getConfig(ArgumentMatchers.anyString(), + ArgumentMatchers.anyString(), ArgumentMatchers.anyLong())) + .thenReturn("key: value"); + NacosPropertySourceBuilder builder = new NacosPropertySourceBuilder(configService, + 1000L); + + NacosPropertySource source = builder.build("app.yaml", "DEFAULT_GROUP", "yaml", + true); + + assertThat(source.getFileExtension()).isEqualTo("yaml"); + assertThat(source.getDataId()).isEqualTo("app.yaml"); + assertThat(source.getGroup()).isEqualTo("DEFAULT_GROUP"); + assertThat(source.isRefreshable()).isTrue(); + } + + @Test + void builtPropertySourcePreservesPropertiesExtension() throws Exception { + ConfigService configService = Mockito.mock(ConfigService.class); + Mockito.when(configService.getConfig(ArgumentMatchers.anyString(), + ArgumentMatchers.anyString(), ArgumentMatchers.anyLong())) + .thenReturn("key=value"); + NacosPropertySourceBuilder builder = new NacosPropertySourceBuilder(configService, + 1000L); + + NacosPropertySource source = builder.build("app", "DEFAULT_GROUP", "properties", + false); + + assertThat(source.getFileExtension()).isEqualTo("properties"); + assertThat(source.isRefreshable()).isFalse(); + } + +} diff --git a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/test/java/com/alibaba/cloud/nacos/client/NacosPropertySourceTest.java b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/test/java/com/alibaba/cloud/nacos/client/NacosPropertySourceTest.java new file mode 100644 index 0000000000..6d9360825e --- /dev/null +++ b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/test/java/com/alibaba/cloud/nacos/client/NacosPropertySourceTest.java @@ -0,0 +1,71 @@ +/* + * Copyright 2013-present 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 com.alibaba.cloud.nacos.client; + +import java.util.Collections; +import java.util.Date; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import org.springframework.core.env.PropertySource; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link NacosPropertySource}. + */ +public class NacosPropertySourceTest { + + @Test + void fileExtensionIsNullWhenUsingLegacyConstructor() { + List> sources = Collections.emptyList(); + NacosPropertySource source = new NacosPropertySource(sources, "DEFAULT_GROUP", + "app.yaml", new Date(), true); + assertThat(source.getFileExtension()).isNull(); + } + + @Test + void fileExtensionIsPreservedWhenProvided() { + List> sources = Collections.emptyList(); + NacosPropertySource source = new NacosPropertySource(sources, "DEFAULT_GROUP", + "app.yaml", new Date(), true, "yaml"); + assertThat(source.getFileExtension()).isEqualTo("yaml"); + } + + @Test + void fileExtensionCanBeNullExplicitly() { + List> sources = Collections.emptyList(); + NacosPropertySource source = new NacosPropertySource(sources, "DEFAULT_GROUP", + "app", new Date(), true, null); + assertThat(source.getFileExtension()).isNull(); + } + + @Test + void otherPropertiesAreNotAffectedByNewConstructor() { + Date timestamp = new Date(); + List> sources = Collections.emptyList(); + NacosPropertySource source = new NacosPropertySource(sources, "MY_GROUP", + "app.json", timestamp, false, "json"); + assertThat(source.getGroup()).isEqualTo("MY_GROUP"); + assertThat(source.getDataId()).isEqualTo("app.json"); + assertThat(source.getTimestamp()).isEqualTo(timestamp); + assertThat(source.isRefreshable()).isFalse(); + assertThat(source.getFileExtension()).isEqualTo("json"); + } + +} diff --git a/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/test/java/com/alibaba/cloud/nacos/refresh/NacosPropertySourceRefreshListenerTest.java b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/test/java/com/alibaba/cloud/nacos/refresh/NacosPropertySourceRefreshListenerTest.java new file mode 100644 index 0000000000..cb3b2440a2 --- /dev/null +++ b/spring-cloud-alibaba-starters/spring-alibaba-nacos-config/src/test/java/com/alibaba/cloud/nacos/refresh/NacosPropertySourceRefreshListenerTest.java @@ -0,0 +1,176 @@ +/* + * Copyright 2013-present 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 com.alibaba.cloud.nacos.refresh; + +import java.util.Collections; +import java.util.Date; + +import com.alibaba.cloud.nacos.NacosConfigManager; +import com.alibaba.cloud.nacos.NacosConfigProperties; +import com.alibaba.cloud.nacos.client.NacosPropertySource; +import com.alibaba.nacos.api.config.ConfigService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; + +import org.springframework.boot.context.event.ApplicationReadyEvent; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MutablePropertySources; +import org.springframework.core.env.PropertySource; +import org.springframework.core.env.StandardEnvironment; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link NacosPropertySourceRefreshListener}. + */ +public class NacosPropertySourceRefreshListenerTest { + + private static final String DATA_ID = "application.yaml"; + + private static final String GROUP = "DEFAULT_GROUP"; + + private NacosConfigManager nacosConfigManager; + + private ConfigService configService; + + private ConfigurableApplicationContext applicationContext; + + private MutablePropertySources propertySources; + + private NacosPropertySourceRefreshListener listener; + + @BeforeEach + void setUp() { + nacosConfigManager = Mockito.mock(NacosConfigManager.class); + configService = Mockito.mock(ConfigService.class); + NacosConfigProperties configProperties = new NacosConfigProperties(); + configProperties.setFileExtension("properties"); + Mockito.when(nacosConfigManager.getConfigService()).thenReturn(configService); + Mockito.when(nacosConfigManager.getNacosConfigProperties()) + .thenReturn(configProperties); + + ConfigurableEnvironment environment = new StandardEnvironment(); + propertySources = environment.getPropertySources(); + + applicationContext = Mockito.mock(ConfigurableApplicationContext.class); + Mockito.when(applicationContext.getEnvironment()).thenReturn(environment); + Mockito.when(applicationContext.containsBean( + "nacosConfigSpringCloudRefreshEventListener")).thenReturn(false); + + listener = new NacosPropertySourceRefreshListener(nacosConfigManager); + listener.setApplicationContext(applicationContext); + // Mark the listener as ready so handle(NacosConfigRefreshEvent) does its + // work; a Mockito mock of ApplicationReadyEvent is enough because + // handle(ApplicationReadyEvent) only flips the readiness flag. + listener.handle(Mockito.mock(ApplicationReadyEvent.class)); + } + + @Test + void refreshReusesYamlExtensionFromPreviousSource() throws Exception { + NacosPropertySource prev = new NacosPropertySource(Collections.emptyList(), + GROUP, DATA_ID, new Date(), true, "yaml"); + propertySources.addFirst(prev); + + Mockito.when(configService.getConfig(ArgumentMatchers.eq(DATA_ID), + ArgumentMatchers.eq(GROUP), ArgumentMatchers.anyLong())) + .thenReturn("foo:\n bar: baz"); + + NacosConfigRefreshEvent event = new NacosConfigRefreshEvent(this, null, + "test"); + event.setDataId(DATA_ID); + event.setGroup(GROUP); + listener.handle(event); + + String sourceName = String.join(NacosConfigProperties.COMMAS, DATA_ID, GROUP); + PropertySource replaced = propertySources.get(sourceName); + assertThat(replaced).isInstanceOf(NacosPropertySource.class); + NacosPropertySource newSource = (NacosPropertySource) replaced; + assertThat(newSource.getFileExtension()).isEqualTo("yaml"); + // The yaml content was parsed as yaml, producing a nested key — not a + // single bogus "foo:\n bar" key as would happen under the old + // hard-coded "properties" path. + assertThat(newSource.getProperty("foo.bar")).isEqualTo("baz"); + } + + @Test + void refreshReusesJsonExtensionFromPreviousSource() throws Exception { + NacosPropertySource prev = new NacosPropertySource(Collections.emptyList(), + GROUP, "conf.json", new Date(), true, "json"); + propertySources.addFirst(prev); + + Mockito.when(configService.getConfig(ArgumentMatchers.eq("conf.json"), + ArgumentMatchers.eq(GROUP), ArgumentMatchers.anyLong())) + .thenReturn("{\"foo\":\"bar\"}"); + + NacosConfigRefreshEvent event = new NacosConfigRefreshEvent(this, null, + "test"); + event.setDataId("conf.json"); + event.setGroup(GROUP); + listener.handle(event); + + String sourceName = String.join(NacosConfigProperties.COMMAS, "conf.json", + GROUP); + NacosPropertySource newSource = (NacosPropertySource) propertySources + .get(sourceName); + assertThat(newSource.getFileExtension()).isEqualTo("json"); + assertThat(newSource.getProperty("foo")).isEqualTo("bar"); + } + + @Test + void refreshFallsBackToPropertiesWhenPreviousSourceHasNoExtension() throws Exception { + // Previous source built via the legacy constructor — no fileExtension. + NacosPropertySource prev = new NacosPropertySource(Collections.emptyList(), + GROUP, DATA_ID, new Date(), true); + propertySources.addFirst(prev); + + Mockito.when(configService.getConfig(ArgumentMatchers.eq(DATA_ID), + ArgumentMatchers.eq(GROUP), ArgumentMatchers.anyLong())) + .thenReturn("foo=bar"); + + NacosConfigRefreshEvent event = new NacosConfigRefreshEvent(this, null, + "test"); + event.setDataId(DATA_ID); + event.setGroup(GROUP); + listener.handle(event); + + String sourceName = String.join(NacosConfigProperties.COMMAS, DATA_ID, GROUP); + NacosPropertySource newSource = (NacosPropertySource) propertySources + .get(sourceName); + assertThat(newSource.getFileExtension()).isEqualTo("properties"); + assertThat(newSource.getProperty("foo")).isEqualTo("bar"); + } + + @Test + void refreshSkipsWhenDataIdIsMissing() throws Exception { + NacosPropertySource prev = new NacosPropertySource(Collections.emptyList(), + GROUP, DATA_ID, new Date(), true, "yaml"); + propertySources.addFirst(prev); + + NacosConfigRefreshEvent event = new NacosConfigRefreshEvent(this, null, + "test"); + event.setGroup(GROUP); + listener.handle(event); + + Mockito.verify(configService, Mockito.never()).getConfig( + ArgumentMatchers.anyString(), ArgumentMatchers.anyString(), + ArgumentMatchers.anyLong()); + } + +}