Browse Source

feat(biz): Added the value length limit function for AppId-level configuration items (#5264)

* feat(biz): Added the value length limit function for AppId-level configuration items

* fix:add CHANGES.md

* refactor(biz): Refactor configuration parsing logic and optimize test cases

* test(biz): Optimize appIdValueLengthLimitOverride test case

* refactor: Triggering a build

* refactor: Triggering a build2

* fix(apollo-biz): Fix the parsing logic of configuration override values and add value validity verification
yangzl 4 tháng trước cách đây
mục cha
commit
2b31ac6d6b

+ 1 - 0
CHANGES.md

@@ -18,6 +18,7 @@ Apollo 2.4.0
 * [Refactor: Configuration files uniformly use Kebab style](https://github.com/apolloconfig/apollo/pull/5262)
 * [Feature: openapi query namespace support not fill item](https://github.com/apolloconfig/apollo/pull/5249)
 * [Refactor: align database ClusterName and NamespaceName fields lengths](https://github.com/apolloconfig/apollo/pull/5263)
+* [Feature: Added the value length limit function for AppId-level configuration items](https://github.com/apolloconfig/apollo/pull/5264)
 
 ------------------
 All issues and pull requests are [here](https://github.com/apolloconfig/apollo/milestone/15?closed=1)

+ 37 - 21
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/config/BizConfig.java

@@ -30,12 +30,16 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
-import java.util.stream.Collectors;
+import java.util.function.Predicate;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Component;
 
 @Component
 public class BizConfig extends RefreshableConfig {
 
+  private final static Logger logger = LoggerFactory.getLogger(BizConfig.class);
+
   private static final int DEFAULT_ITEM_KEY_LENGTH = 128;
   private static final int DEFAULT_ITEM_VALUE_LENGTH = 20000;
 
@@ -58,6 +62,9 @@ public class BizConfig extends RefreshableConfig {
 
   private static final Gson GSON = new Gson();
 
+  private static final Type appIdValueLengthOverrideTypeReference =
+      new TypeToken<Map<String, Integer>>() {
+      }.getType();
   private static final Type namespaceValueLengthOverrideTypeReference =
       new TypeToken<Map<Long, Integer>>() {
       }.getType();
@@ -106,6 +113,16 @@ public class BizConfig extends RefreshableConfig {
     return checkInt(limit, 5, Integer.MAX_VALUE, DEFAULT_ITEM_VALUE_LENGTH);
   }
 
+  public Map<String, Integer> appIdValueLengthLimitOverride() {
+    String appIdValueLengthOverrideString = getValue("appid.value.length.limit.override");
+    return parseOverrideConfig(appIdValueLengthOverrideString, appIdValueLengthOverrideTypeReference, value -> value > 0);
+  }
+
+  public Map<Long, Integer> namespaceValueLengthLimitOverride() {
+    String namespaceValueLengthOverrideString = getValue("namespace.value.length.limit.override");
+    return parseOverrideConfig(namespaceValueLengthOverrideString, namespaceValueLengthOverrideTypeReference, value -> value > 0);
+  }
+
   public boolean isNamespaceNumLimitEnabled() {
     return getBooleanProperty("namespace.num.limit.enabled", false);
   }
@@ -128,17 +145,6 @@ public class BizConfig extends RefreshableConfig {
     return checkInt(limit, 5, Integer.MAX_VALUE, DEFAULT_MAX_ITEM_NUM);
   }
 
-  public Map<Long, Integer> namespaceValueLengthLimitOverride() {
-    String namespaceValueLengthOverrideString = getValue("namespace.value.length.limit.override");
-    Map<Long, Integer> namespaceValueLengthOverride = Maps.newHashMap();
-    if (!Strings.isNullOrEmpty(namespaceValueLengthOverrideString)) {
-      namespaceValueLengthOverride =
-          GSON.fromJson(namespaceValueLengthOverrideString, namespaceValueLengthOverrideTypeReference);
-    }
-
-    return namespaceValueLengthOverride;
-  }
-
   public boolean isNamespaceLockSwitchOff() {
     return !getBooleanProperty("namespace.lock.switch", false);
   }
@@ -195,15 +201,7 @@ public class BizConfig extends RefreshableConfig {
 
   public Map<String, Integer> releaseHistoryRetentionSizeOverride() {
     String overrideString = getValue("apollo.release-history.retention.size.override");
-    Map<String, Integer> releaseHistoryRetentionSizeOverride = Maps.newHashMap();
-    if (!Strings.isNullOrEmpty(overrideString)) {
-      releaseHistoryRetentionSizeOverride =
-          GSON.fromJson(overrideString, releaseHistoryRetentionSizeOverrideTypeReference);
-    }
-    return releaseHistoryRetentionSizeOverride.entrySet()
-        .stream()
-        .filter(entry -> entry.getValue() >= 1)
-        .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+    return parseOverrideConfig(overrideString, releaseHistoryRetentionSizeOverrideTypeReference, value -> value > 0);
   }
 
   public int releaseMessageCacheScanInterval() {
@@ -256,4 +254,22 @@ public class BizConfig extends RefreshableConfig {
   public String getAdminServiceAccessTokens() {
     return getValue("admin-service.access.tokens");
   }
+
+  private <K, V> Map<K, V> parseOverrideConfig(String configValue, Type typeReference, Predicate<V> valueFilter) {
+    Map<K, V> result = Maps.newHashMap();
+    if (!Strings.isNullOrEmpty(configValue)) {
+      try {
+        Map<K, V> parsed = GSON.fromJson(configValue, typeReference);
+        for (Map.Entry<K, V> entry : parsed.entrySet()) {
+          if (entry.getValue() != null && valueFilter.test(entry.getValue())) {
+            result.put(entry.getKey(), entry.getValue());
+          }
+        }
+      } catch (Exception e) {
+        logger.error("Invalid override config value: {}", configValue, e);
+      }
+    }
+    return Collections.unmodifiableMap(result);
+  }
+
 }

+ 11 - 5
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java

@@ -217,8 +217,8 @@ public class ItemService {
   }
 
   private boolean checkItemValueLength(long namespaceId, String value) {
-    int limit = getItemValueLengthLimit(namespaceId);
     Namespace currentNamespace = namespaceService.findOne(namespaceId);
+    int limit = getItemValueLengthLimit(currentNamespace);
     if(currentNamespace != null) {
       Matcher m = clusterPattern.matcher(currentNamespace.getClusterName());
       boolean isGray = m.matches();
@@ -235,7 +235,7 @@ public class ItemService {
   private int getGrayNamespaceItemValueLengthLimit(Namespace grayNamespace, int grayNamespaceLimit) {
     Namespace parentNamespace = namespaceService.findParentNamespace(grayNamespace);
     if (parentNamespace != null) {
-      int parentLimit = getItemValueLengthLimit(parentNamespace.getId());
+      int parentLimit = getItemValueLengthLimit(grayNamespace);
       if (parentLimit > grayNamespaceLimit) {
         return parentLimit;
       }
@@ -257,11 +257,17 @@ public class ItemService {
     return true;
   }
 
-  private int getItemValueLengthLimit(long namespaceId) {
+  private int getItemValueLengthLimit(Namespace namespace) {
     Map<Long, Integer> namespaceValueLengthOverride = bizConfig.namespaceValueLengthLimitOverride();
-    if (namespaceValueLengthOverride != null && namespaceValueLengthOverride.containsKey(namespaceId)) {
-      return namespaceValueLengthOverride.get(namespaceId);
+    if (namespaceValueLengthOverride != null && namespaceValueLengthOverride.containsKey(namespace.getId())) {
+      return namespaceValueLengthOverride.get(namespace.getId());
     }
+
+    Map<String, Integer> appIdValueLengthOverride = bizConfig.appIdValueLengthLimitOverride();
+    if (appIdValueLengthOverride != null && appIdValueLengthOverride.containsKey(namespace.getAppId())) {
+      return appIdValueLengthOverride.get(namespace.getAppId());
+    }
+
     return bizConfig.itemValueLengthLimit();
   }
 

+ 44 - 2
apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/config/BizConfigTest.java

@@ -18,13 +18,13 @@ package com.ctrip.framework.apollo.biz.config;
 
 import com.ctrip.framework.apollo.biz.repository.ServerConfigRepository;
 import com.ctrip.framework.apollo.biz.service.BizDBPropertySource;
+import java.util.Map;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.core.env.ConfigurableEnvironment;
-import org.springframework.core.env.Environment;
 import org.springframework.test.util.ReflectionTestUtils;
 
 import javax.sql.DataSource;
@@ -93,7 +93,7 @@ public class BizConfigTest {
     int someOverrideLimit = 10;
     String overrideValueString = "{'a+b+c+b':10}";
     when(environment.getProperty("apollo.release-history.retention.size.override")).thenReturn(overrideValueString);
-    int  overrideValue = bizConfig.releaseHistoryRetentionSizeOverride().get("a+b+c+b");
+    int overrideValue = bizConfig.releaseHistoryRetentionSizeOverride().get("a+b+c+b");
     assertEquals(someOverrideLimit, overrideValue);
 
     overrideValueString = "{'a+b+c+b':0,'a+b+d+b':2}";
@@ -107,6 +107,48 @@ public class BizConfigTest {
     assertEquals(0, bizConfig.releaseHistoryRetentionSizeOverride().size());
   }
 
+  @Test
+  public void testAppIdValueLengthLimitOverride() {
+    when(environment.getProperty("appid.value.length.limit.override")).thenReturn(null);
+    Map<String, Integer> result = bizConfig.appIdValueLengthLimitOverride();
+    assertTrue(result.isEmpty());
+
+    String input = "{}";
+    when(environment.getProperty("appid.value.length.limit.override")).thenReturn(input);
+    result = bizConfig.appIdValueLengthLimitOverride();
+    assertTrue(result.isEmpty());
+
+    input = "invalid json";
+    when(environment.getProperty("appid.value.length.limit.override")).thenReturn(input);
+    result = bizConfig.appIdValueLengthLimitOverride();
+    assertTrue(result.isEmpty());
+
+    input = "{'appid1':555}";
+    when(environment.getProperty("appid.value.length.limit.override")).thenReturn(input);
+    int overrideValue = bizConfig.appIdValueLengthLimitOverride().get("appid1");
+    assertEquals(1, bizConfig.appIdValueLengthLimitOverride().size());
+    assertEquals(555, overrideValue);
+
+    input = "{'appid1':555,'appid2':666}";
+    when(environment.getProperty("appid.value.length.limit.override")).thenReturn(input);
+    overrideValue = bizConfig.appIdValueLengthLimitOverride().get("appid2");
+    assertEquals(2, bizConfig.appIdValueLengthLimitOverride().size());
+    assertEquals(666, overrideValue);
+
+    input = "{'appid1':555,'appid2':666,'appid3':0,'appid4':-1}";
+    when(environment.getProperty("appid.value.length.limit.override")).thenReturn(input);
+    result = bizConfig.appIdValueLengthLimitOverride();
+
+    assertTrue(result.containsKey("appid1"));
+    assertTrue(result.containsKey("appid2"));
+    assertFalse(result.containsKey("appid3"));
+    assertFalse(result.containsKey("appid4"));
+    assertEquals(2, result.size());
+
+    overrideValue = result.get("appid2");
+    assertEquals(666, overrideValue);
+  }
+
   @Test
   public void testReleaseMessageNotificationBatchWithNAN() throws Exception {
     String someNAN = "someNAN";

+ 85 - 16
apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java

@@ -16,12 +16,20 @@
  */
 package com.ctrip.framework.apollo.biz.service;
 
+import static org.mockito.Mockito.when;
+
 import com.ctrip.framework.apollo.biz.AbstractIntegrationTest;
+import com.ctrip.framework.apollo.biz.config.BizConfig;
 import com.ctrip.framework.apollo.biz.entity.Item;
+import com.ctrip.framework.apollo.biz.repository.ItemRepository;
 import com.ctrip.framework.apollo.common.dto.ItemInfoDTO;
 import com.ctrip.framework.apollo.common.exception.BadRequestException;
+import java.util.HashMap;
+import java.util.Map;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mock;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
@@ -32,18 +40,28 @@ public class ItemServiceTest extends AbstractIntegrationTest {
     @Autowired
     private ItemService itemService;
 
+    @Autowired
+    private ItemRepository itemRepository;
+    @Autowired
+    private NamespaceService namespaceService;
+    @Autowired
+    private AuditService auditService;
+
+    @Mock
+    private BizConfig bizConfig;
+
+    private ItemService itemService2;
+
+    @Before
+    public void setUp() throws Exception {
+        itemService2 = new ItemService(itemRepository, namespaceService, auditService, bizConfig);
+    }
+
     @Test
-    @Sql(scripts = "/sql/item-test.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD)
+    @Sql(scripts = {"/sql/namespace-test.sql","/sql/item-test.sql"}, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD)
     @Sql(scripts = "/sql/clean.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD)
     public void testSaveItem() {
-        Item item = new Item();
-        item.setNamespaceId(3);
-        item.setKey("k3");
-        item.setType(-1);
-        item.setValue("v3");
-        item.setComment("");
-        item.setLineNum(3);
-
+        Item item = createItem(1L, "k3", "v3", -1);
         try {
             itemService.save(item);
             Assert.fail();
@@ -57,16 +75,56 @@ public class ItemServiceTest extends AbstractIntegrationTest {
     }
 
     @Test
-    @Sql(scripts = "/sql/item-test.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD)
+    @Sql(scripts = {"/sql/namespace-test.sql", "/sql/item-test.sql"}, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD)
+    @Sql(scripts = "/sql/clean.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD)
+    public void testSaveItemWithNamespaceValueLengthLimitOverride() {
+
+        long namespaceId = 1L;
+        String itemValue = "test-demo";
+
+        Map<Long, Integer> namespaceValueLengthOverride = new HashMap<>();
+        namespaceValueLengthOverride.put(namespaceId, itemValue.length() - 1);
+        when(bizConfig.namespaceValueLengthLimitOverride()).thenReturn(namespaceValueLengthOverride);
+        when(bizConfig.itemKeyLengthLimit()).thenReturn(100);
+
+        Item item = createItem(namespaceId, "k3", itemValue, 2);
+        try {
+            itemService2.save(item);
+            Assert.fail();
+        } catch (Exception e) {
+            Assert.assertTrue(e instanceof BadRequestException && e.getMessage().contains("value too long"));
+        }
+    }
+
+    @Test
+    @Sql(scripts = {"/sql/namespace-test.sql", "/sql/item-test.sql"}, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD)
+    @Sql(scripts = "/sql/clean.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD)
+    public void testSaveItemWithAppIdValueLengthLimitOverride() {
+
+        String appId = "testApp";
+        long namespaceId = 1L;
+        String itemValue = "test-demo";
+
+        Map<String, Integer> appIdValueLengthOverride = new HashMap<>();
+        appIdValueLengthOverride.put(appId, itemValue.length() - 1);
+        when(bizConfig.appIdValueLengthLimitOverride()).thenReturn(appIdValueLengthOverride);
+        when(bizConfig.itemKeyLengthLimit()).thenReturn(100);
+
+        Item item = createItem(namespaceId, "k3", itemValue, 2);
+        try {
+            itemService2.save(item);
+            Assert.fail();
+        } catch (Exception e) {
+            Assert.assertTrue(e instanceof BadRequestException && e.getMessage().contains("value too long"));
+        }
+    }
+
+    @Test
+    @Sql(scripts = {"/sql/namespace-test.sql","/sql/item-test.sql"}, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD)
     @Sql(scripts = "/sql/clean.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD)
     public void testUpdateItem() {
-        Item item = new Item();
+        Item item = createItem(1, "k1", "v1-new", 2);
         item.setId(9901);
-        item.setNamespaceId(1);
-        item.setKey("k1");
-        item.setType(2);
-        item.setValue("v1-new");
-        item.setComment("");
         item.setLineNum(1);
 
         Item dbItem = itemService.update(item);
@@ -96,4 +154,15 @@ public class ItemServiceTest extends AbstractIntegrationTest {
 
     }
 
+    private Item createItem(long namespaceId, String key, String value, int type) {
+        Item item = new Item();
+        item.setNamespaceId(namespaceId);
+        item.setKey(key);
+        item.setValue(value);
+        item.setType(type);
+        item.setComment("");
+        item.setLineNum(3);
+        return item;
+    }
+
 }

+ 12 - 2
docs/en/deployment/distributed-deployment-guide.md

@@ -1539,9 +1539,19 @@ The default configuration is 128.
 
 The default configuration is 20000.
 
-#### 3.2.5.1 `namespace.value.length.limit.override` - Maximum length limit for namespace's configuration item value
+#### 3.2.5.1 appid.value.length.limit.override - The maximum length limit of the configuration item value of the appId dimension
 
-This configuration is used to override the `item.value.length.limit` configuration to achieve fine-grained control of the namespace's value maximum length limit, the configured value is a json format, the key of the json is the id value of the namespace in the database, the format is as follows.
+This configuration is used to override the configuration of `item.value.length.limit` to control the maximum length limit of the value at the appId granularity. The configured value is in a json format, and the key of the json is appId. The format is as follows:
+```
+appid.value.length.limit.override = {"appId-demo1":200,"appId-demo2":300}
+```
+The above configuration specifies that the maximum length limit of the value in all namespaces under `appId-demo1` is 200, and the maximum length limit of the value in all namespaces under `appId-demo2` is 300
+
+When a new namespace is created under `appId-demo1` or `appId-demo2`, it will automatically inherit the maximum length limit of the value of the namespace, unless the maximum length limit of the value of the configuration item of the namespace is overridden by `namespace.value.length.limit.override`.
+
+#### 3.2.5.2 `namespace.value.length.limit.override` - Maximum length limit for namespace's configuration item value
+
+This configuration is used to override the `item.value.length.limit` or `appid.value.length.limit.override` configuration to achieve fine-grained control of the namespace's value maximum length limit, the configured value is a json format, the key of the json is the id value of the namespace in the database, the format is as follows.
 
 ```
 namespace.value.length.limit.override = {1:200,3:20}

+ 11 - 2
docs/zh/deployment/distributed-deployment-guide.md

@@ -1481,9 +1481,18 @@ http://5.5.5.5:8080/eureka/,http://6.6.6.6:8080/eureka/
 
 默认配置是20000。
 
-#### 3.2.5.1 namespace.value.length.limit.override - namespace 的配置项 value 最大长度限制
+#### 3.2.5.1 appid.value.length.limit.override - appId 维度的配置项 value 最大长度限制
+此配置用来覆盖 `item.value.length.limit` 的配置,做到控制 appId 粒度下的 value 最大长度限制,配置的值是一个 json 格式,json 的 key 为 appId,格式如下:
+```
+appid.value.length.limit.override = {"appId-demo1":200,"appId-demo2":300}
+```
+以上配置指定了 `appId-demo1` 下的所有 namespace 中的 value 最大长度限制为 200,`appId-demo2` 下的所有 namespace 中的 value 最大长度限制为 300
+
+当 `appId-demo1` 或 `appId-demo2` 下新建的 namespace 时,会自动继承该 namespace 的 value 最大长度限制,除非该 namespace 的配置项 value 最大长度限制被 `namespace.value.length.limit.override` 覆盖。
+
+#### 3.2.5.2 namespace.value.length.limit.override - namespace 的配置项 value 最大长度限制
 
-此配置用来覆盖 `item.value.length.limit` 的配置,做到细粒度控制 namespace 的 value 最大长度限制,配置的值是一个 json 格式,json 的 key 为 namespace 在数据库中的 id 值,格式如下:
+此配置用来覆盖 `item.value.length.limit` 或者 `appid.value.length.limit.override` 的配置,做到细粒度控制 namespace 的 value 最大长度限制,配置的值是一个 json 格式,json 的 key 为 namespace 在数据库中的 id 值,格式如下:
 ```
 namespace.value.length.limit.override = {1:200,3:20}
 ```