Skip to content

Commit

Permalink
refactor: use LinkedHashMultimap to replace MultiValueMap (#2629)
Browse files Browse the repository at this point in the history
#### What type of PR is this?
/kind improvement
/area core
/milestone 2.0
#### What this PR does / why we need it:
ReverseProxyRouterFunctionRegistry 的注册方法之前使用了一个 LinkedMultiValueMap<String, String>,相当于是一个 Map<String, List<String>>,当插件启动后扫描到 ReverseProxy 资源保存到数据库会触发 ReverseProxyReconciler 调用`ReverseProxyRouterFunctionRegistry#register`,如果此时插件的 PluginApplicationContext 还没有被创建好,register 时会在中途因为获取不到 PluginApplicationContext 而失败然后 requeue,而 LinkedMultiValueMap 此时已经保存了一条记录 requeue后再执行 LinkedMultiValueMap(即`Map<String, List<String>>`) 这个 value 是一个 List 需要更多的判断来防止重复,需要用一个 Map<String, Set<String>>这样的结构来保证 requeue 时 ReverseProxyRouterFunctionRegistry 中集合结构之前的数据一致性这样更方便,所以将从 spring 的 LinkedMultiValueMap 切换到  guava 提供的 LinkedHashMultimap(它的结构是Map<K, Set<V>)

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

```release-note
None
```
  • Loading branch information
guqing authored Oct 26, 2022
1 parent ee032f8 commit fa3e0c4
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package run.halo.app.plugin.resources;

import com.google.common.collect.LinkedHashMultimap;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.locks.StampedLock;
import org.springframework.stereotype.Component;
import org.springframework.util.Assert;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.reactive.function.server.RouterFunction;
import org.springframework.web.reactive.function.server.ServerResponse;
import reactor.core.publisher.Mono;
Expand All @@ -27,8 +27,8 @@ public class ReverseProxyRouterFunctionRegistry {
private final StampedLock lock = new StampedLock();
private final Map<String, RouterFunction<ServerResponse>> proxyNameRouterFunctionRegistry =
new HashMap<>();
private final MultiValueMap<String, String> pluginIdReverseProxyMap =
new LinkedMultiValueMap<>();
private final LinkedHashMultimap<String, String> pluginIdReverseProxyMap =
LinkedHashMultimap.create();

public ReverseProxyRouterFunctionRegistry(
ReverseProxyRouterFunctionFactory reverseProxyRouterFunctionFactory) {
Expand All @@ -47,11 +47,7 @@ public Mono<Void> register(String pluginId, ReverseProxy reverseProxy) {
final String proxyName = reverseProxy.getMetadata().getName();
long stamp = lock.writeLock();
try {
List<String> reverseProxyNames = pluginIdReverseProxyMap.get(pluginId);
if (reverseProxyNames != null && reverseProxyNames.contains(proxyName)) {
return Mono.empty();
}
pluginIdReverseProxyMap.add(pluginId, proxyName);
pluginIdReverseProxyMap.put(pluginId, proxyName);

// Obtain plugin application context
PluginApplicationContext pluginApplicationContext =
Expand All @@ -71,8 +67,8 @@ public Mono<Void> register(String pluginId, ReverseProxy reverseProxy) {
* Only for test.
*/
protected int reverseProxySize(String pluginId) {
List<String> names = pluginIdReverseProxyMap.get(pluginId);
return names == null ? 0 : names.size();
Set<String> names = pluginIdReverseProxyMap.get(pluginId);
return names.size();
}

/**
Expand All @@ -84,10 +80,7 @@ public Mono<Void> remove(String pluginId) {
Assert.notNull(pluginId, "The plugin id must not be null.");
long stamp = lock.writeLock();
try {
List<String> proxyNames = pluginIdReverseProxyMap.remove(pluginId);
if (proxyNames == null) {
return Mono.empty();
}
Set<String> proxyNames = pluginIdReverseProxyMap.removeAll(pluginId);
for (String proxyName : proxyNames) {
proxyNameRouterFunctionRegistry.remove(proxyName);
}
Expand All @@ -103,11 +96,7 @@ public Mono<Void> remove(String pluginId) {
public Mono<Void> remove(String pluginId, String reverseProxyName) {
long stamp = lock.writeLock();
try {
List<String> proxyNames = pluginIdReverseProxyMap.get(pluginId);
if (proxyNames == null) {
return Mono.empty();
}
proxyNames.remove(reverseProxyName);
pluginIdReverseProxyMap.remove(pluginId, reverseProxyName);
proxyNameRouterFunctionRegistry.remove(reverseProxyName);
return Mono.empty();
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,7 @@ void tearDown() {

@Test
void register() {
ReverseProxy mock = Mockito.mock(ReverseProxy.class);
Metadata metadata = new Metadata();
metadata.setName("test-reverse-proxy");
when(mock.getMetadata()).thenReturn(metadata);
RouterFunction<ServerResponse> routerFunction = request -> Mono.empty();

when(reverseProxyRouterFunctionFactory.create(any(), any()))
.thenReturn(Mono.just(routerFunction));
ReverseProxy mock = getMockReverseProxy();
registry.register("fake-plugin", mock)
.as(StepVerifier::create)
.verifyComplete();
Expand All @@ -72,6 +65,42 @@ void register() {

assertThat(registry.reverseProxySize("fake-plugin")).isEqualTo(1);

verify(reverseProxyRouterFunctionFactory, times(1)).create(any(), any());
verify(reverseProxyRouterFunctionFactory, times(2)).create(any(), any());
}

@Test
void remove() {
ReverseProxy mock = getMockReverseProxy();
registry.register("fake-plugin", mock)
.as(StepVerifier::create)
.verifyComplete();

registry.remove("fake-plugin").block();

assertThat(registry.reverseProxySize("fake-plugin")).isEqualTo(0);
}

@Test
void removeByKeyValue() {
ReverseProxy mock = getMockReverseProxy();
registry.register("fake-plugin", mock)
.as(StepVerifier::create)
.verifyComplete();

registry.remove("fake-plugin", "test-reverse-proxy").block();

assertThat(registry.reverseProxySize("fake-plugin")).isEqualTo(0);
}

private ReverseProxy getMockReverseProxy() {
ReverseProxy mock = Mockito.mock(ReverseProxy.class);
Metadata metadata = new Metadata();
metadata.setName("test-reverse-proxy");
when(mock.getMetadata()).thenReturn(metadata);
RouterFunction<ServerResponse> routerFunction = request -> Mono.empty();

when(reverseProxyRouterFunctionFactory.create(any(), any()))
.thenReturn(Mono.just(routerFunction));
return mock;
}
}

0 comments on commit fa3e0c4

Please sign in to comment.