Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 9 commits into from
Nov 3, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
12a5a67
feat(biz): Added the value length limit function for AppId-level conf…
youngzil Oct 27, 2024
6ab0caa
fix：add CHANGES.md
youngzil Oct 27, 2024
c380c91
refactor(biz): Refactor configuration parsing logic and optimize test…
youngzil Oct 27, 2024
2bb1e5f
test(biz): Optimize appIdValueLengthLimitOverride test case
youngzil Oct 27, 2024
cd70ba7
refactor: Triggering a build
youngzil Oct 27, 2024
2a0aa08
refactor: Triggering a build2
youngzil Oct 27, 2024
ae8b256
Merge branch 'master' into feature/item-value-length-limit-for-appid
youngzil Oct 27, 2024
e1522bd
Merge branch 'master' into feature/item-value-length-limit-for-appid
youngzil Nov 2, 2024
f4b4e9b
fix(apollo-biz): Fix the parsing logic of configuration override valu…
youngzil Nov 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
58 changes: 37 additions & 21 deletions apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/config/BizConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,16 @@
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;

Expand All @@ -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();
Expand Down Expand Up @@ -106,6 +113,16 @@ public int itemValueLengthLimit() {
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);
}
Expand All @@ -128,17 +145,6 @@ public int itemNumLimit() {
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);
}
Expand Down Expand Up @@ -195,15 +201,7 @@ public int releaseHistoryRetentionSize() {

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() {
Expand Down Expand Up @@ -256,4 +254,22 @@ public boolean isAdminServiceAccessControlEnabled() {
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);
}

}
16 changes: 11 additions & 5 deletions apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ public Item update(Item item) {
}

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();
Expand All @@ -235,7 +235,7 @@ private boolean checkItemValueLength(long namespaceId, String value) {
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;
}
Expand All @@ -257,11 +257,17 @@ private boolean checkItemType(int type) {
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();
}

Expand Down
46 changes: 44 additions & 2 deletions apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/config/BizConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@

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;
Expand Down Expand Up @@ -93,7 +93,7 @@ public void testReleaseHistoryRetentionSizeOverride() {
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}";
Expand All @@ -107,6 +107,48 @@ public void testReleaseHistoryRetentionSizeOverride() {
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());
Comment on lines +138 to +146
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add tests for boundary values.

While the test verifies filtering of invalid values (0, -1), it should also test boundary conditions:

@Test
@DisplayName("Should handle boundary values")
public void testAppIdValueLengthLimitOverride_Boundaries() {
    String input = String.format(
        "{'%s':1,'%s':%d,'%s':%d}",
        "min-app", "max-app", Integer.MAX_VALUE,
        "invalid-app", Integer.MIN_VALUE
    );
    when(environment.getProperty("appid.value.length.limit.override")).thenReturn(input);
    
    Map<String, Integer> result = bizConfig.appIdValueLengthLimitOverride();
    
    assertTrue(result.containsKey("min-app"));
    assertTrue(result.containsKey("max-app"));
    assertFalse(result.containsKey("invalid-app"));
    assertEquals(Integer.MAX_VALUE, result.get("max-app").intValue());
    assertEquals(1, result.get("min-app").intValue());
}


overrideValue = result.get("appid2");
assertEquals(666, overrideValue);
}

@Test
public void testReleaseMessageNotificationBatchWithNAN() throws Exception {
String someNAN = "someNAN";
Expand Down
101 changes: 85 additions & 16 deletions apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand All @@ -57,16 +75,56 @@ public void testSaveItem() {
}

@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);
Expand Down Expand Up @@ -96,4 +154,15 @@ public void testSearchItem() {

}

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;
}

}
14 changes: 12 additions & 2 deletions docs/en/deployment/distributed-deployment-guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Loading
Loading