[SCB-2043] refactory as comment

apache · Jul 29, 2020 · 4cf527e · 4cf527e
1 parent 4856844
commit 4cf527e
Show file tree

Hide file tree

Showing 11 changed files with 106 additions and 97 deletions.
diff --git a/...andler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java b/...andler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
@@ -17,21 +17,20 @@
 
 package org.apache.servicecomb.qps;
 
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx;
+import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
 import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
 import org.apache.servicecomb.qps.strategy.AbstractQpsStrategy;
-import org.apache.servicecomb.qps.strategy.FixedWindowStrategy;
-import org.apache.servicecomb.qps.strategy.LeakyBucketStrategy;
-import org.apache.servicecomb.qps.strategy.StrategyType;
-import org.apache.servicecomb.qps.strategy.TokenBucketStrategy;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.netflix.config.DynamicProperty;
+import org.springframework.util.StringUtils;
 
 public class QpsControllerManager {
   private static final Logger LOGGER = LoggerFactory.getLogger(QpsControllerManager.class);
@@ -210,27 +209,24 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
   }
 
   private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
-      String strategyKey) {
-    AbstractQpsStrategy strategy;
-    AbstractQpsStrategy customStrategy = SPIServiceUtils
-        .getTargetService(AbstractQpsStrategy.class);
-    switch (StrategyType.parseStrategyType(strategyKey)) {
-      case FixedWindow:
-        strategy = new FixedWindowStrategy(globalConfigKey, limit);
-        break;
-      case LeakyBucket:
-        strategy = new LeakyBucketStrategy(globalConfigKey, limit);
-        break;
-      case TokenBucket:
-        strategy = new TokenBucketStrategy(globalConfigKey, limit, bucket);
-        break;
-      case Custom:
-        strategy = customStrategy;
-        break;
-      case SlidingWindow:
-      default:
-        strategy = new FixedWindowStrategy(globalConfigKey, limit);
+      String strategyName) {
+    if (StringUtils.isEmpty(strategyName)) {
+      strategyName = "FixedWindow";
     }
+    AbstractQpsStrategy strategy = null;
+    List<AbstractQpsStrategy> customStrategy = SPIServiceUtils
+        .getOrLoadSortedService(AbstractQpsStrategy.class);
+    for (AbstractQpsStrategy abstractQpsStrategy : customStrategy) {
+      if (abstractQpsStrategy.name().equals(strategyName)) {
+        strategy = abstractQpsStrategy.clone();
+      }
+    }
+    if (strategy == null) {
+      throw new ServiceCombException("the qps strategy name is not exist , please check.");
+    }
+    strategy.setKey(globalConfigKey);
+    strategy.setQpsLimit(limit);
+    strategy.setBucketLimit(bucket);
     return strategy;
   }
 

diff --git a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsStrategy.java b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsStrategy.java
@@ -20,4 +20,6 @@
 public interface QpsStrategy {
 
   boolean isLimitNewRequest();
+
+  String name();
 }
diff --git a/...lowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/AbstractQpsStrategy.java b/...lowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/AbstractQpsStrategy.java
@@ -20,19 +20,14 @@
 import org.apache.servicecomb.qps.QpsStrategy;
 
 
-public class AbstractQpsStrategy implements QpsStrategy {
+public abstract class AbstractQpsStrategy implements QpsStrategy {
 
   private Long qpsLimit;
 
   private Long bucketLimit;
 
   private String key;
 
-  public AbstractQpsStrategy(Long qpsLimit, String key) {
-    this.qpsLimit = qpsLimit;
-    this.key = key;
-  }
-
   public Long getBucketLimit() {
     return bucketLimit;
   }
@@ -42,9 +37,12 @@ public void setBucketLimit(Long bucketLimit) {
   }
 
   @Override
-  public boolean isLimitNewRequest() {
-    return true;
-  }
+  public abstract boolean isLimitNewRequest();
+
+  @Override
+  public abstract String name();
+
+  public abstract AbstractQpsStrategy clone();
 
   public void setQpsLimit(Long qpsLimit) {
     this.qpsLimit = qpsLimit;

diff --git a/...lowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/FixedWindowStrategy.java b/...lowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/FixedWindowStrategy.java
@@ -31,10 +31,7 @@ public class FixedWindowStrategy extends AbstractQpsStrategy {
 
   private static final int CYCLE_LENGTH = 1000;
 
-  public FixedWindowStrategy(String key, Long qpsLimit) {
-    super(qpsLimit, key);
-    this.msCycleBegin = System.currentTimeMillis();
-  }
+  private static final String STRATEGY_NAME = "FixedWindow";
 
   // return true means new request need to be rejected
   public boolean isLimitNewRequest() {
@@ -54,4 +51,14 @@ public boolean isLimitNewRequest() {
     // It is possible that operation level updated to null,but schema level or microservice level does not updated
     return newCount - lastRequestCount >= this.getQpsLimit();
   }
+
+  @Override
+  public String name() {
+    return STRATEGY_NAME;
+  }
+
+  @Override
+  public AbstractQpsStrategy clone() {
+    return new FixedWindowStrategy();
+  }
 }
diff --git a/...lowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/LeakyBucketStrategy.java b/...lowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/LeakyBucketStrategy.java
@@ -34,15 +34,7 @@ public class LeakyBucketStrategy extends AbstractQpsStrategy {
 
   private long remainder = 0;
 
-  public LeakyBucketStrategy(String key, Long qpsLimit) {
-    super(qpsLimit, key);
-    this.setBucketLimit(qpsLimit);
-  }
-
-  public LeakyBucketStrategy(String key, Long qpsLimit, Long bucketLimit) {
-    super(qpsLimit, key);
-    this.setBucketLimit(bucketLimit);
-  }
+  private static final String STRATEGY_NAME = "LeakyBucket";
 
   @Override
   public boolean isLimitNewRequest() {
@@ -72,4 +64,13 @@ public boolean isLimitNewRequest() {
     return true;
   }
 
+  @Override
+  public String name() {
+    return STRATEGY_NAME;
+  }
+
+  @Override
+  public AbstractQpsStrategy clone() {
+    return new LeakyBucketStrategy();
+  }
 }
diff --git a/...ndler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/StrategyType.java b/...ndler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/StrategyType.java
diff --git a/...lowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/TokenBucketStrategy.java b/...lowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/TokenBucketStrategy.java
@@ -19,7 +19,15 @@
 
 public class TokenBucketStrategy extends LeakyBucketStrategy {
 
-  public TokenBucketStrategy(String key, Long qpsLimit, Long bucketLimit) {
-    super(key, qpsLimit, bucketLimit);
+  private static final String STRATEGY_NAME = "TokenBucket";
+
+  @Override
+  public AbstractQpsStrategy clone() {
+    return new TokenBucketStrategy();
+  }
+
+  @Override
+  public String name() {
+    return STRATEGY_NAME;
   }
 }
diff --git a/.../main/resources/META-INF/services/org.apache.servicecomb.qps.strategy.AbstractQpsStrategy b/.../main/resources/META-INF/services/org.apache.servicecomb.qps.strategy.AbstractQpsStrategy
@@ -0,0 +1,20 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+org.apache.servicecomb.qps.strategy.FixedWindowStrategy
+org.apache.servicecomb.qps.strategy.LeakyBucketStrategy
+org.apache.servicecomb.qps.strategy.TokenBucketStrategy
diff --git a/...ntrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java b/...ntrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java
@@ -70,7 +70,9 @@ public void afterTest() {
 
   @Test
   public void testQpsController() {
-    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy("abc", 100L);
+    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy();
+    qpsStrategy.setKey("abc");
+    qpsStrategy.setQpsLimit(100L);
     Assert.assertEquals(false, qpsStrategy.isLimitNewRequest());
 
     qpsStrategy.setQpsLimit(1L);
@@ -80,7 +82,9 @@ public void testQpsController() {
   @Test
   public void testHandle() throws Exception {
     String key = "svc.schema.opr";
-    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy("key", 12L);
+    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy();
+    qpsStrategy.setKey("key");
+    qpsStrategy.setQpsLimit(12L);
     Mockito.when(invocation.getOperationMeta()).thenReturn(operationMeta);
     Mockito.when(operationMeta.getSchemaQualifiedName()).thenReturn("schema.opr");
     Mockito.when(invocation.getSchemaId()).thenReturn("schema");
@@ -113,7 +117,9 @@ protected QpsStrategy create(String qualifiedNameKey) {
   @Test
   public void testHandleIsLimitNewRequestAsFalse() throws Exception {
     String key = "service.schema.id";
-    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy("service", 12L);
+    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy();
+    qpsStrategy.setKey("service");
+    qpsStrategy.setQpsLimit(12L);
     Mockito.when(invocation.getMicroserviceName()).thenReturn("service");
     Mockito.when(invocation.getOperationMeta()).thenReturn(operationMeta);
 

diff --git a/...ntrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java b/...ntrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java
@@ -99,7 +99,9 @@ public void testGlobalQpsControl(final @Injectable Invocation invocation,
 
   @Test
   public void testQpsController() {
-    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy("abc", 100L);
+    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy();
+    qpsStrategy.setKey("abc");
+    qpsStrategy.setQpsLimit(100L);
     assertFalse(qpsStrategy.isLimitNewRequest());
 
     qpsStrategy.setQpsLimit(1L);
@@ -144,7 +146,10 @@ public void testHandle() throws Exception {
     new MockUp<QpsControllerManager>() {
       @Mock
       protected QpsStrategy create(String qualifiedNameKey) {
-        return new FixedWindowStrategy(qualifiedNameKey, 1L);
+        AbstractQpsStrategy strategy = new FixedWindowStrategy();
+        strategy.setKey(qualifiedNameKey);
+        strategy.setQpsLimit(1L);
+        return strategy;
       }
     };
 
@@ -171,7 +176,10 @@ public void testHandleIsLimitNewRequestAsFalse() throws Exception {
     new MockUp<QpsControllerManager>() {
       @Mock
       protected QpsStrategy create(String qualifiedNameKey) {
-        return new FixedWindowStrategy(qualifiedNameKey, 1L);
+        AbstractQpsStrategy strategy = new FixedWindowStrategy();
+        strategy.setKey(qualifiedNameKey);
+        strategy.setQpsLimit(1L);
+        return strategy;
       }
     };
     handler.handle(invocation, asyncResp);

diff --git a/...ers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestQpsStrategy.java b/...ers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestQpsStrategy.java
@@ -31,7 +31,9 @@ public class TestQpsStrategy {
 
   @Test
   public void testFixedWindowStrategy() {
-    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy("abc", 100L);
+    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy();
+    qpsStrategy.setKey("abc");
+    qpsStrategy.setQpsLimit(100L);
     Assert.assertEquals(false, qpsStrategy.isLimitNewRequest());
 
     qpsStrategy.setQpsLimit(1L);
@@ -41,7 +43,9 @@ public void testFixedWindowStrategy() {
 
   @Test
   public void testLeakyBucketStrategy() {
-    LeakyBucketStrategy qpsStrategy = new LeakyBucketStrategy("abc", 100L);
+    LeakyBucketStrategy qpsStrategy = new LeakyBucketStrategy();
+    qpsStrategy.setKey("abc");
+    qpsStrategy.setQpsLimit(100L);
     Assert.assertEquals(false, qpsStrategy.isLimitNewRequest());
 
     qpsStrategy.setQpsLimit(1L);