Skip to content

Commit

Permalink
Fix the problem of fetching old value from plugin setting fetcher (#6216
Browse files Browse the repository at this point in the history
)

#### 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 <#6141>.

#### Which issue(s) this PR fixes:

Fixes #6213 

#### Does this PR introduce a user-facing change?

```release-note
修复在插件配置变更监听器中始终获取到旧数据的问题
```
  • Loading branch information
JohnNiang authored Jun 30, 2024
1 parent 8e97814 commit 9410006
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,13 @@ public Result reconcile(Request request) {
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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
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;
Expand All @@ -18,6 +20,7 @@
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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -115,7 +119,12 @@ void getValuesWithUpdateCache() throws JSONException {
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<String, JsonNode> updatedValues = settingFetcher.getValues();
verify(client, times(1)).fetch(eq(ConfigMap.class), any());
Expand Down

0 comments on commit 9410006

Please sign in to comment.