Skip to content

Commit 4d91b37

Browse files
committed
fix diff method can not be added to RadixTree
1 parent c31164a commit 4d91b37

File tree

7 files changed

+149
-3
lines changed

7 files changed

+149
-3
lines changed

dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/DefaultRequestMappingRegistry.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ private void register0(RequestMapping mapping, HandlerMeta handler, AtomicIntege
169169
try {
170170
Registration registration = new Registration(mapping, handler);
171171
for (PathExpression path : mapping.getPathCondition().getExpressions()) {
172-
Registration exists = tree.addPath(path, registration);
172+
Registration exists = tree.addPath(path, registration, Registration::isOverlap);
173173
if (exists == null) {
174174
counter.incrementAndGet();
175175
if (LOGGER.isDebugEnabled()) {

dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RadixTree.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@
3131
import java.util.List;
3232
import java.util.Map;
3333
import java.util.Map.Entry;
34+
import java.util.Objects;
3435
import java.util.function.BiConsumer;
36+
import java.util.function.BiFunction;
3537
import java.util.function.Predicate;
3638

3739
/**
@@ -63,13 +65,25 @@ public RadixTree() {
6365
this(true, '/');
6466
}
6567

68+
/**
69+
* This is a default implementation that does not check for equality.
70+
*/
6671
public T addPath(PathExpression path, T value) {
72+
return addPath(path, value, (t, t2) -> Boolean.TRUE);
73+
}
74+
75+
/**
76+
* When the path is the same, the predicate is used to determine whether the values are considered equal.
77+
* If the predicate returns true, the existing value is returned directly.
78+
*/
79+
public T addPath(PathExpression path, T value, BiFunction<T, T, Boolean> predicate) {
80+
Objects.requireNonNull(predicate);
6781
if (path.isDirect()) {
6882
KeyString key = new KeyString(path.getPath(), caseSensitive);
6983
List<Match<T>> matches = directPathMap.computeIfAbsent(key, k -> new ArrayList<>());
7084
for (int i = 0, size = matches.size(); i < size; i++) {
7185
Match<T> match = matches.get(i);
72-
if (match.getValue().equals(value)) {
86+
if (predicate.apply(match.getValue(), value)) {
7387
return match.getValue();
7488
}
7589
}
@@ -85,7 +99,9 @@ public T addPath(PathExpression path, T value) {
8599
List<Pair<PathExpression, T>> values = child.values;
86100
for (int j = 0, size = values.size(); j < size; j++) {
87101
if (values.get(j).getLeft().equals(path)) {
88-
return values.get(j).getRight();
102+
if (predicate.apply(values.get(j).getRight(), value)) {
103+
return values.get(j).getRight();
104+
}
89105
}
90106
}
91107
values.add(Pair.of(path, value));

dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/Registration.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ public boolean equals(Object obj) {
4747
return mapping.equals(((Registration) obj).mapping);
4848
}
4949

50+
public boolean isOverlap(Registration registration) {
51+
return registration == null || mapping.isOverlap(registration.mapping);
52+
}
53+
5054
@Override
5155
public int hashCode() {
5256
return mapping.hashCode();

dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RequestMapping.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,25 @@ public boolean equals(Object obj) {
347347
&& Objects.equals(sig, other.sig);
348348
}
349349

350+
public boolean isOverlap(RequestMapping requestMapping) {
351+
if (this == requestMapping) {
352+
return true;
353+
}
354+
if (requestMapping == null) {
355+
return false;
356+
}
357+
return Objects.equals(pathCondition, requestMapping.pathCondition)
358+
&& (this.methodsCondition == null
359+
|| requestMapping.methodsCondition == null
360+
|| this.methodsCondition.isOverlap(requestMapping.methodsCondition))
361+
&& Objects.equals(paramsCondition, requestMapping.paramsCondition)
362+
&& Objects.equals(headersCondition, requestMapping.headersCondition)
363+
&& Objects.equals(consumesCondition, requestMapping.consumesCondition)
364+
&& Objects.equals(producesCondition, requestMapping.producesCondition)
365+
&& Objects.equals(customCondition, requestMapping.customCondition)
366+
&& Objects.equals(sig, requestMapping.sig);
367+
}
368+
350369
@Override
351370
public String toString() {
352371
StringBuilder sb = new StringBuilder("RequestMapping{name='");

dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/MethodsCondition.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.apache.dubbo.remoting.http12.HttpRequest;
2020

2121
import java.util.Arrays;
22+
import java.util.Collections;
2223
import java.util.HashSet;
2324
import java.util.Set;
2425

@@ -88,6 +89,10 @@ public int compareTo(MethodsCondition other, HttpRequest request) {
8889
return 0;
8990
}
9091

92+
public boolean isOverlap(MethodsCondition methodsCondition) {
93+
return methodsCondition == null || !Collections.disjoint(this.getMethods(), methodsCondition.getMethods());
94+
}
95+
9196
@Override
9297
public int hashCode() {
9398
return methods.hashCode();

dubbo-rpc/dubbo-rpc-triple/src/test/groovy/org/apache/dubbo/rpc/protocol/tri/rest/mapping/RadixTreeTest.groovy

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
package org.apache.dubbo.rpc.protocol.tri.rest.mapping
1919

20+
import org.apache.dubbo.rpc.protocol.tri.rest.mapping.condition.PathExpression
21+
2022
import spock.lang.Specification
2123

2224
class RadixTreeTest extends Specification {
@@ -59,6 +61,42 @@ class RadixTreeTest extends Specification {
5961
'/a/b/c/d' | 0
6062
}
6163

64+
def "test repeat add,no predicate function"() {
65+
given:
66+
def tree = new RadixTree<Boolean>()
67+
Boolean val1 = tree.addPath('/a/*/*', false)
68+
Boolean val2 = tree.addPath('/a/*/*', true)
69+
expect:
70+
val1 == null;
71+
val2 == false;
72+
}
73+
74+
def "test repeat add,use predicate function"() {
75+
given:
76+
def tree = new RadixTree<Boolean>()
77+
Boolean val1 = tree.addPath(PathExpression.parse('/a/*/*'), false, { a, b -> a == b })
78+
Boolean val2 = tree.addPath(PathExpression.parse('/a/*/*'), true, { a, b -> a == b })
79+
Boolean val3 = tree.addPath(PathExpression.parse('/a/*/*'), true, { a, b -> a == b })
80+
81+
expect:
82+
val1 == null;
83+
val2 == null;
84+
val3 == true;
85+
}
86+
87+
def "test repeat add,use predicate function and Registration"() {
88+
given:
89+
def tree = new RadixTree<Boolean>()
90+
Boolean val1 = tree.addPath(PathExpression.parse('/a/*/*'), false, { a, b -> a == b })
91+
Boolean val2 = tree.addPath(PathExpression.parse('/a/*/*'), true, { a, b -> a == b })
92+
Boolean val3 = tree.addPath(PathExpression.parse('/a/*/*'), true, { a, b -> a == b })
93+
94+
expect:
95+
val1 == null;
96+
val2 == null;
97+
val3 == true;
98+
}
99+
62100
def "test sub path match"() {
63101
given:
64102
def tree = new RadixTree<String>();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the 'License'); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an 'AS IS' BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.dubbo.rpc.protocol.tri.rest.mapping
19+
20+
import org.apache.dubbo.remoting.http12.HttpMethods
21+
import org.apache.dubbo.rpc.protocol.tri.rest.mapping.meta.HandlerMeta
22+
23+
import spock.lang.Specification
24+
25+
class RegistrationSpec extends Specification {
26+
27+
def "isMethodOverlap should return true when methods overlap"() {
28+
given:
29+
def mapping1 = new RequestMapping.Builder().method(HttpMethods.GET.name(), HttpMethods.PUT.name()).build()
30+
def mapping2 = new RequestMapping.Builder().method(HttpMethods.PUT.name(), HttpMethods.POST.name()).build()
31+
def meta = GroovyMock(HandlerMeta)
32+
def reg1 = new Registration(mapping1, meta)
33+
def reg2 = new Registration(mapping2, meta)
34+
35+
expect:
36+
reg1.isOverlap(reg2)
37+
}
38+
39+
def "isMethodOverlap should return false when methods do not overlap"() {
40+
given:
41+
def mapping1 = new RequestMapping.Builder().method(HttpMethods.GET.name()).build()
42+
def mapping2 = new RequestMapping.Builder().method(HttpMethods.PUT.name()).build()
43+
44+
def meta = GroovyMock(HandlerMeta)
45+
def reg1 = new Registration(mapping1, meta)
46+
def reg2 = new Registration(mapping2, meta)
47+
48+
expect:
49+
!reg1.isOverlap(reg2)
50+
}
51+
52+
def "isMethodOverlap should return true if any MethodsCondition is null"() {
53+
given:
54+
def mapping1 = new RequestMapping.Builder().build()
55+
def mapping2 = new RequestMapping.Builder().build()
56+
57+
def meta = GroovyMock(HandlerMeta)
58+
def reg1 = new Registration(mapping1, meta)
59+
def reg2 = new Registration(mapping2, meta)
60+
61+
expect:
62+
reg1.isOverlap(reg2)
63+
}
64+
}

0 commit comments

Comments
 (0)