From 9410006659e08367f66c03d60e60a7e817ea91b8 Mon Sep 17 00:00:00 2001 From: John Niang Date: Sun, 30 Jun 2024 10:57:11 +0800 Subject: [PATCH] Fix the problem of fetching old value from plugin setting fetcher (#6216) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind bug /area core /area plugin /milestone 2.17.x #### What this PR does / why we need it: This PR makes sure the method `cache#put` is called before the event is published to avoid the event listener to fetch the old value from the cache. The problem was introduced by . #### Which issue(s) this PR fixes: Fixes #6213 #### Does this PR introduce a user-facing change? ```release-note 修复在插件配置变更监听器中始终获取到旧数据的问题 ``` --- .../app/plugin/DefaultReactiveSettingFetcher.java | 4 ++-- .../halo/app/plugin/DefaultSettingFetcherTest.java | 13 +++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/application/src/main/java/run/halo/app/plugin/DefaultReactiveSettingFetcher.java b/application/src/main/java/run/halo/app/plugin/DefaultReactiveSettingFetcher.java index 3b0ba917d..86492a44a 100644 --- a/application/src/main/java/run/halo/app/plugin/DefaultReactiveSettingFetcher.java +++ b/application/src/main/java/run/halo/app/plugin/DefaultReactiveSettingFetcher.java @@ -162,13 +162,13 @@ public class DefaultReactiveSettingFetcher if (configMapData != null) { configMapData.forEach((key, value) -> result.put(key, readTree(value))); } + // update cache + cache.put(pluginName, result); applicationContext.publishEvent(PluginConfigUpdatedEvent.builder() .source(this) .oldConfig(existData) .newConfig(result) .build()); - // update cache - cache.put(pluginName, result); }); return Result.doNotRetry(); } diff --git a/application/src/test/java/run/halo/app/plugin/DefaultSettingFetcherTest.java b/application/src/test/java/run/halo/app/plugin/DefaultSettingFetcherTest.java index 7c377d926..5e7f0d2ee 100644 --- a/application/src/test/java/run/halo/app/plugin/DefaultSettingFetcherTest.java +++ b/application/src/test/java/run/halo/app/plugin/DefaultSettingFetcherTest.java @@ -3,6 +3,8 @@ package run.halo.app.plugin; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -18,6 +20,7 @@ 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.Spy; import org.mockito.junit.jupiter.MockitoExtension; import org.skyscreamer.jsonassert.JSONAssert; import org.springframework.boot.test.mock.mockito.MockBean; @@ -64,7 +67,8 @@ class DefaultSettingFetcherTest { private DefaultReactiveSettingFetcher reactiveSettingFetcher; private DefaultSettingFetcher settingFetcher; - private final Cache cache = new ConcurrentMapCache(buildCacheKey(pluginContext.getName())); + @Spy + Cache cache = new ConcurrentMapCache(buildCacheKey(pluginContext.getName())); @BeforeEach void setUp() { @@ -115,7 +119,12 @@ class DefaultSettingFetcherTest { when(blockingClient.fetch(eq(ConfigMap.class), eq(pluginContext.getConfigMapName()))) .thenReturn(Optional.of(configMap)); reactiveSettingFetcher.reconcile(new Reconciler.Request(pluginContext.getConfigMapName())); - verify(applicationContext).publishEvent(any()); + + // Make sure the method cache#put is called before the event is published + // to avoid the event listener to fetch the old value from the cache + var inOrder = inOrder(cache, applicationContext); + inOrder.verify(cache).put(eq("fake"), any()); + inOrder.verify(applicationContext).publishEvent(isA(PluginConfigUpdatedEvent.class)); Map updatedValues = settingFetcher.getValues(); verify(client, times(1)).fetch(eq(ConfigMap.class), any());