Skip to content

Commit

Permalink
fix codeReview comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@panxiaojun233
panxiaojun233 committed Feb 6, 2023
1 parent 4099daa commit c82a4ba
Show file tree
Hide file tree
Showing 31 changed files with 111 additions and 101 deletions.
2 changes: 1 addition & 1 deletion sentinel-core/src/main/java/com/alibaba/csp/sentinel/traffic/ClusterManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import java.util.List;
import java.util.Set;

import com.alibaba.csp.sentinel.traffic.rule.traffic.router.TrafficRouterRuleManager;
import com.alibaba.csp.sentinel.traffic.rule.router.TrafficRouterRuleManager;

/**
* @author panxiaojun233
Expand Down
11 changes: 11 additions & 0 deletions sentinel-core/src/main/java/com/alibaba/csp/sentinel/traffic/Instance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,15 @@ public Map<String, String> getMetadata() {
public void setMetadata(Map<String, String> metadata) {
this.metadata = metadata;
}

@Override
public String toString() {
return "Instance{" +
"appName='" + appName + '\'' +
", host='" + host + '\'' +
", port=" + port +
", metadata=" + metadata +
", targetInstance=" + targetInstance +
'}';
}
}
4 changes: 2 additions & 2 deletions ...ntinel/traffic/TrafficRuleSubscriber.java → ...p/sentinel/traffic/RemoteAppObserver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
package com.alibaba.csp.sentinel.traffic;

/**
* The subscriber of traffic rule.
* The observer for remote provider app change event.
* @author panxiaojun233
*/
public interface TrafficRuleSubscriber {
public interface RemoteAppObserver {

void subscribe(String app);

Expand Down
22 changes: 11 additions & 11 deletions sentinel-core/src/main/java/com/alibaba/csp/sentinel/traffic/TrafficRouterFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@
import java.util.concurrent.ThreadLocalRandom;

import com.alibaba.csp.sentinel.traffic.rule.TrafficRoutingRuleGroup;
import com.alibaba.csp.sentinel.traffic.rule.traffic.router.Route;
import com.alibaba.csp.sentinel.traffic.rule.traffic.router.RouteDetail;
import com.alibaba.csp.sentinel.traffic.rule.traffic.router.TrafficRouter;
import com.alibaba.csp.sentinel.traffic.rule.traffic.router.TrafficRouterRuleManager;
import com.alibaba.csp.sentinel.traffic.rule.traffic.router.destination.Destination;
import com.alibaba.csp.sentinel.traffic.rule.traffic.router.destination.RouteDestination;
import com.alibaba.csp.sentinel.traffic.rule.traffic.router.match.RequestMatch;
import com.alibaba.csp.sentinel.traffic.rule.traffic.router.match.StringMatch;
import com.alibaba.csp.sentinel.traffic.rule.vritual.workload.Subset;
import com.alibaba.csp.sentinel.traffic.rule.vritual.workload.VirtualWorkload;
import com.alibaba.csp.sentinel.traffic.rule.router.Route;
import com.alibaba.csp.sentinel.traffic.rule.router.RouteDetail;
import com.alibaba.csp.sentinel.traffic.rule.router.TrafficRouter;
import com.alibaba.csp.sentinel.traffic.rule.router.TrafficRouterRuleManager;
import com.alibaba.csp.sentinel.traffic.rule.router.destination.Destination;
import com.alibaba.csp.sentinel.traffic.rule.router.destination.RouteDestination;
import com.alibaba.csp.sentinel.traffic.rule.router.match.RequestMatch;
import com.alibaba.csp.sentinel.traffic.rule.router.match.StringMatch;
import com.alibaba.csp.sentinel.traffic.rule.workload.Subset;
import com.alibaba.csp.sentinel.traffic.rule.workload.VirtualWorkload;
import com.alibaba.csp.sentinel.util.StringUtil;

/**
Expand Down Expand Up @@ -170,7 +170,7 @@ protected List<RouteDestination> getRouteDestination(TrafficRoutingRuleGroup tra
protected List<RouteDestination> getRouteDestination(List<Route> routes, TrafficContext context) {

for (Route route : routes) {
List<RouteDetail> routeDetailList = route.getRoutedetail();
List<RouteDetail> routeDetailList = route.getRouteDetail();
if (routeDetailList != null && routeDetailList.size() > 0) {
for (RouteDetail routeDetail : routeDetailList) {
List<RequestMatch> requestMatchList = routeDetail.getMatch();
Expand Down
4 changes: 2 additions & 2 deletions ...nel-core/src/main/java/com/alibaba/csp/sentinel/traffic/rule/TrafficRoutingRuleGroup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
import java.util.LinkedList;
import java.util.List;

import com.alibaba.csp.sentinel.traffic.rule.traffic.router.TrafficRouter;
import com.alibaba.csp.sentinel.traffic.rule.vritual.workload.VirtualWorkload;
import com.alibaba.csp.sentinel.traffic.rule.router.TrafficRouter;
import com.alibaba.csp.sentinel.traffic.rule.workload.VirtualWorkload;

/**
* @author panxiaojun233
Expand Down
18 changes: 9 additions & 9 deletions ...el/traffic/rule/traffic/router/Route.java → ...p/sentinel/traffic/rule/router/Route.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router;
package com.alibaba.csp.sentinel.traffic.rule.router;

import java.util.List;

import com.alibaba.csp.sentinel.traffic.rule.traffic.router.match.StringMatch;
import com.alibaba.csp.sentinel.traffic.rule.router.match.StringMatch;

/**
* @author panxiaojun233
*/
public class Route {
private String name;
private List<StringMatch> services;
private List<RouteDetail> routedetail;
private List<RouteDetail> routeDetail;

public String getName() {
return name;
Expand All @@ -43,20 +43,20 @@ public void setServices(List<StringMatch> services) {
this.services = services;
}

public List<RouteDetail> getRoutedetail() {
return routedetail;
public List<RouteDetail> getRouteDetail() {
return routeDetail;
}

public void setRoutedetail(List<RouteDetail> routedetail) {
this.routedetail = routedetail;
public void setRoutedetail(List<RouteDetail> routeDetail) {
this.routeDetail = routeDetail;
}

@Override
public String toString() {
return "DubboRoute{" +
return "Route{" +
"name='" + name + '\'' +
", services=" + services +
", routedetail=" + routedetail +
", routeDetail=" + routeDetail +
'}';
}
}
6 changes: 3 additions & 3 deletions ...ffic/rule/traffic/router/RouteDetail.java → ...inel/traffic/rule/router/RouteDetail.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router;
package com.alibaba.csp.sentinel.traffic.rule.router;

import java.util.List;

import com.alibaba.csp.sentinel.traffic.rule.traffic.router.destination.RouteDestination;
import com.alibaba.csp.sentinel.traffic.rule.traffic.router.match.RequestMatch;
import com.alibaba.csp.sentinel.traffic.rule.router.destination.RouteDestination;
import com.alibaba.csp.sentinel.traffic.rule.router.match.RequestMatch;

/**
* @author panxiaojun233
Expand Down
2 changes: 1 addition & 1 deletion ...ic/rule/traffic/router/TrafficRouter.java → ...el/traffic/rule/router/TrafficRouter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router;
package com.alibaba.csp.sentinel.traffic.rule.router;

import java.util.Collections;
import java.util.List;
Expand Down
10 changes: 5 additions & 5 deletions ...ffic/router/TrafficRouterRuleManager.java → ...rule/router/TrafficRouterRuleManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router;
package com.alibaba.csp.sentinel.traffic.rule.router;

import java.util.List;
import java.util.concurrent.ConcurrentHashMap;

import com.alibaba.csp.sentinel.spi.SpiLoader;
import com.alibaba.csp.sentinel.traffic.TrafficRuleSubscriber;
import com.alibaba.csp.sentinel.traffic.RemoteAppObserver;
import com.alibaba.csp.sentinel.traffic.rule.TrafficRoutingRuleGroup;

/**
Expand All @@ -28,7 +28,7 @@
public class TrafficRouterRuleManager {

private static final ConcurrentHashMap<String, Object> trafficRouterRuleListenerMap = new ConcurrentHashMap<>();
private static List<TrafficRuleSubscriber> subscribers = SpiLoader.of(TrafficRuleSubscriber.class)
private static List<RemoteAppObserver> subscribers = SpiLoader.of(RemoteAppObserver.class)
.loadInstanceList();
private static volatile TrafficRoutingRuleGroup trafficRoutingRuleGroup = new TrafficRoutingRuleGroup();
private static final Object object = new Object();
Expand All @@ -44,15 +44,15 @@ public static TrafficRoutingRuleGroup getTrafficRoutingRules() {
public static synchronized void register(String app) {
trafficRouterRuleListenerMap.put(app, object);

for (TrafficRuleSubscriber subscriber : subscribers) {
for (RemoteAppObserver subscriber : subscribers) {
subscriber.subscribe(app);
}
}

public static synchronized void unregister(String app) {
trafficRouterRuleListenerMap.remove(app);
// todo unsubscribe
for (TrafficRuleSubscriber subscriber : subscribers) {
for (RemoteAppObserver subscriber : subscribers) {
subscriber.unSubscribe(app);
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...affic/router/destination/Destination.java → .../rule/router/destination/Destination.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router.destination;
package com.alibaba.csp.sentinel.traffic.rule.router.destination;

/**
* @author panxiaojun233
Expand Down
2 changes: 1 addition & 1 deletion .../router/destination/RouteDestination.java → .../router/destination/RouteDestination.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router.destination;
package com.alibaba.csp.sentinel.traffic.rule.router.destination;

/**
* @author panxiaojun233
Expand Down
2 changes: 1 addition & 1 deletion ...e/traffic/router/match/ArgumentMatch.java → ...ffic/rule/router/match/ArgumentMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router.match;
package com.alibaba.csp.sentinel.traffic.rule.router.match;

/**
* @author panxiaojun233
Expand Down
2 changes: 1 addition & 1 deletion .../rule/traffic/router/match/BoolMatch.java → .../traffic/rule/router/match/BoolMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router.match;
package com.alibaba.csp.sentinel.traffic.rule.router.match;

/**
* @author panxiaojun233
Expand Down
2 changes: 1 addition & 1 deletion ...ule/traffic/router/match/DoubleMatch.java → ...raffic/rule/router/match/DoubleMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router.match;
package com.alibaba.csp.sentinel.traffic.rule.router.match;

/**
* @author panxiaojun233
Expand Down
2 changes: 1 addition & 1 deletion ...raffic/router/match/DoubleRangeMatch.java → ...c/rule/router/match/DoubleRangeMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router.match;
package com.alibaba.csp.sentinel.traffic.rule.router.match;

/**
* @author panxiaojun233
Expand Down
2 changes: 1 addition & 1 deletion ...e/traffic/router/match/ListBoolMatch.java → ...ffic/rule/router/match/ListBoolMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router.match;
package com.alibaba.csp.sentinel.traffic.rule.router.match;

import java.util.List;

Expand Down
2 changes: 1 addition & 1 deletion ...traffic/router/match/ListDoubleMatch.java → ...ic/rule/router/match/ListDoubleMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router.match;
package com.alibaba.csp.sentinel.traffic.rule.router.match;

import java.util.List;

Expand Down
2 changes: 1 addition & 1 deletion ...traffic/router/match/ListStringMatch.java → ...ic/rule/router/match/ListStringMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router.match;
package com.alibaba.csp.sentinel.traffic.rule.router.match;

import java.util.List;

Expand Down
45 changes: 22 additions & 23 deletions ...ule/traffic/router/match/MethodMatch.java → ...raffic/rule/router/match/MethodMatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.alibaba.csp.sentinel.traffic.rule.traffic.router.match;
package com.alibaba.csp.sentinel.traffic.rule.router.match;

import java.util.List;
import java.util.Map;
Expand All @@ -29,9 +29,9 @@ public class MethodMatch {

private StringMatch name;
private StringMatch path;
private Integer argc;
private List<ArgumentMatch> args;
private List<StringMatch> argp;
private Integer argumentCount;
private List<ArgumentMatch> arguments;
private List<StringMatch> argumentParameters;
private StringMatch group;
private StringMatch version;
private Map<String, StringMatch> headers;
Expand All @@ -52,28 +52,28 @@ public void setPath(StringMatch path) {
this.path = path;
}

public Integer getArgc() {
return argc;
public Integer getArgumentCount() {
return argumentCount;
}

public void setArgc(Integer argc) {
this.argc = argc;
public void setArgumentCount(Integer argumentCount) {
this.argumentCount = argumentCount;
}

public List<ArgumentMatch> getArgs() {
return args;
public List<ArgumentMatch> getArguments() {
return arguments;
}

public void setArgs(List<ArgumentMatch> args) {
this.args = args;
public void setArguments(List<ArgumentMatch> arguments) {
this.arguments = arguments;
}

public List<StringMatch> getArgp() {
return argp;
public List<StringMatch> getArgumentParameters() {
return argumentParameters;
}

public void setArgp(List<StringMatch> argp) {
this.argp = argp;
public void setArgumentParameters(List<StringMatch> argumentParameters) {
this.argumentParameters = argumentParameters;
}

public StringMatch getGroup() {
Expand Down Expand Up @@ -105,9 +105,9 @@ public String toString() {
return "MethodMatch{" +
"name=" + name +
", path=" + path +
", argc=" + argc +
", args=" + args +
", argp=" + argp +
", argumentCount=" + argumentCount +
", arguments=" + arguments +
", argumentParameters=" + argumentParameters +
", group=" + group +
", version=" + version +
", headers=" + headers +
Expand Down Expand Up @@ -137,14 +137,14 @@ public boolean isMatch(TrafficContext context) {
}
}

Integer argc = getArgc();
Integer argc = getArgumentCount();
List<Object> arguments = context.getArgs();
if (argc != null &&
((argc != 0 && (arguments == null || arguments.size() == 0)) || (argc != arguments.size()))) {
return false;
}

List<StringMatch> argp = getArgp();
List<StringMatch> argp = getArgumentParameters();
// todo string or class
List<String> parameterTypes = context.getParamTypes();
if (argp != null && argp.size() > 0) {
Expand All @@ -156,15 +156,14 @@ public boolean isMatch(TrafficContext context) {
}

for (int index = 0; index < argp.size(); index++) {
// todo
boolean match = argp.get(index).isMatch(parameterTypes.get(index));
if (!match) {
return false;
}
}
}

List<ArgumentMatch> args = getArgs();
List<ArgumentMatch> args = getArguments();
if (args != null && args.size() > 0) {
if (arguments == null || arguments.size() == 0) {
return false;
Expand Down
Loading

0 comments on commit c82a4ba

Please sign in to comment.