mirror of https://github.com/halo-dev/halo
refactor: use LinkedHashMultimap to replace MultiValueMap (#2629)
#### 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 ```pull/2632/head
parent
ee032f8cb9
commit
fa3e0c44f4
|
@ -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;
|
||||
|
@ -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) {
|
||||
|
@ -47,11 +47,7 @@ public class ReverseProxyRouterFunctionRegistry {
|
|||
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 =
|
||||
|
@ -71,8 +67,8 @@ public class ReverseProxyRouterFunctionRegistry {
|
|||
* 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();
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -84,10 +80,7 @@ public class ReverseProxyRouterFunctionRegistry {
|
|||
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);
|
||||
}
|
||||
|
@ -103,11 +96,7 @@ public class ReverseProxyRouterFunctionRegistry {
|
|||
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 {
|
||||
|
|
|
@ -51,14 +51,7 @@ class ReverseProxyRouterFunctionRegistryTest {
|
|||
|
||||
@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();
|
||||
|
@ -72,6 +65,42 @@ class ReverseProxyRouterFunctionRegistryTest {
|
|||
|
||||
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;
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue