Add basic type check for Item value (#4542)

* 增加配置项类型判断，后端以及sql修改。 * apollo配置项增加类型校验 * update CHANGES.md * Update apollo-portal/src/main/resources/static/i18n/zh-CN.json Co-authored-by: mghio <[email protected]> * add ddl sql to delta file * add ddl sql to delta file ( in a new foler v200-v210) * Update scripts/sql/delta/v200-v210/apolloconfigdb-v200-v210.sql Co-authored-by: Jason Song <[email protected]> * Update scripts/sql/apolloconfigdb.sql Co-authored-by: Jason Song <[email protected]> * [openapi] add item type * Optimize the code after review. * [openapi] add type check. * Fix unit tests. * Update apollo-portal/src/main/resources/static/scripts/directive/item-modal-directive.js Co-authored-by: Jason Song <[email protected]> * Update apollo-portal/src/main/resources/static/scripts/directive/item-modal-directive.js Co-authored-by: Jason Song <[email protected]> * Update apollo-portal/src/main/resources/static/scripts/directive/item-modal-directive.js Co-authored-by: Jason Song <[email protected]> * Update apollo-portal/src/main/resources/static/scripts/directive/item-modal-directive.js Co-authored-by: Jason Song <[email protected]> * Update apollo-portal/src/main/resources/static/scripts/directive/item-modal-directive.js Co-authored-by: Jason Song <[email protected]> * Fix adminservice unit tests. * Revert "Optimize the code after review." This reverts commit 6b983a0 * Re optimize the code after review. * 1. use english comments. 2. move openapi check to service side. * move openapi check to service side. * move openapi check to service side. * move type check to com.ctrip.framework.apollo.biz.service.ItemService. * add type in gray and related namespace pages. * correct comments and update com.ctrip.framework.apollo.biz.service.ItemSetService#doUpdateItems * Fix 'Type missing problem' while click 'Full Release' in Grayscale Version * correct comments * Add unit tests for ItemService. Co-authored-by: Bobji <[email protected]> Co-authored-by: mghio <[email protected]> Co-authored-by: Jason Song <[email protected]>
apolloconfig · Sep 18, 2022 · e1636d4 · e1636d4
1 parent 70b70c3
commit e1636d4
Show file tree

Hide file tree

Showing 22 changed files with 403 additions and 147 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -28,6 +28,7 @@ Apollo 2.1.0
 * [add configuration processor for portal developers](https://github.com/apolloconfig/apollo/pull/4521)
 * [Add a potential json value check feature](https://github.com/apolloconfig/apollo/pull/4519)
 * [Add index for table ReleaseHistory](https://github.com/apolloconfig/apollo/pull/4550)
+* [Add basic type check for Item value](https://github.com/apolloconfig/apollo/pull/4542)
 * [add an option to custom oidc userDisplayName](https://github.com/apolloconfig/apollo/pull/4507)
 * [fix openapi item with url illegalKey 400 error](https://github.com/apolloconfig/apollo/pull/4549)
 * [fix the exception occurred when publish/rollback namespaces with grayrelease](https://github.com/apolloconfig/apollo/pull/4564)

diff --git a/...vice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java b/...vice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java
@@ -143,7 +143,8 @@ public ItemDTO update(@PathVariable("appId") String appId,
 
     Item beforeUpdateItem = BeanUtils.transform(Item.class, managedEntity);
 
-    //protect. only value,comment,lastModifiedBy can be modified
+    //protect. only value,type,comment,lastModifiedBy can be modified
+    managedEntity.setType(entity.getType());
     managedEntity.setValue(entity.getValue());
     managedEntity.setComment(entity.getComment());
     managedEntity.setDataChangeLastModifiedBy(entity.getDataChangeLastModifiedBy());

diff --git a/apollo-adminservice/src/test/resources/controller/test-release.sql b/apollo-adminservice/src/test/resources/controller/test-release.sql
@@ -21,6 +21,6 @@ INSERT INTO AppNamespace (AppId, Name) VALUES ('someAppId', 'application');
 
 INSERT INTO Namespace (Id, AppId, ClusterName, NamespaceName) VALUES (100, 'someAppId', 'default', 'application');
 
-INSERT INTO Item (NamespaceId, `Key`, Value, Comment) VALUES (100, 'k1', 'v1', 'comment1');
-INSERT INTO Item (NamespaceId, `Key`, Value, Comment) VALUES (100, 'k2', 'v2', 'comment1');
-INSERT INTO Item (NamespaceId, `Key`, Value, Comment) VALUES (100, 'k3', 'v3', 'comment1');
+INSERT INTO Item (NamespaceId, `Key`, `Type`, Value, Comment) VALUES (100, 'k1', '0', 'v1', 'comment1');
+INSERT INTO Item (NamespaceId, `Key`, `Type`, Value, Comment) VALUES (100, 'k2', '0', 'v2', 'comment1');
+INSERT INTO Item (NamespaceId, `Key`, `Type`, Value, Comment) VALUES (100, 'k3', '0', 'v3', 'comment1');
diff --git a/apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Item.java b/apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Item.java
@@ -38,6 +38,9 @@ public class Item extends BaseEntity {
   @Column(name = "key", nullable = false)
   private String key;
 
+  @Column(name = "type")
+  private int type;
+
   @Column(name = "value")
   @Lob
   private String value;
@@ -88,8 +91,17 @@ public void setLineNum(Integer lineNum) {
     this.lineNum = lineNum;
   }
 
+  public int getType() {
+    return type;
+  }
+
+  public void setType(int type) {
+    this.type = type;
+  }
+
   public String toString() {
-    return toStringHelper().add("namespaceId", namespaceId).add("key", key).add("value", value)
-        .add("lineNum", lineNum).add("comment", comment).toString();
+    return toStringHelper().add("namespaceId", namespaceId).add("key", key)
+            .add("type", type).add("value", value)
+            .add("lineNum", lineNum).add("comment", comment).toString();
   }
 }
diff --git a/apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java b/apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java
@@ -145,6 +145,7 @@ public Page<Item> findItemsByNamespace(String appId, String clusterName, String
   @Transactional
   public Item save(Item entity) {
     checkItemKeyLength(entity.getKey());
+    checkItemType(entity.getType());
     checkItemValueLength(entity.getNamespaceId(), entity.getValue());
 
     entity.setId(0);//protection
@@ -185,6 +186,7 @@ public Item saveComment(Item entity) {
 
   @Transactional
   public Item update(Item item) {
+    checkItemType(item.getType());
     checkItemValueLength(item.getNamespaceId(), item.getValue());
     Item managedItem = itemRepository.findById(item.getId()).orElse(null);
     BeanUtils.copyEntityProperties(item, managedItem);
@@ -211,6 +213,13 @@ private boolean checkItemKeyLength(String key) {
     return true;
   }
 
+  private boolean checkItemType(int type) {
+    if (type < 0 || type > 3) {
+      throw new BadRequestException("type is invalid. type should be in [0, 3]. ");
+    }
+    return true;
+  }
+
   private int getItemValueLengthLimit(long namespaceId) {
     Map<Long, Integer> namespaceValueLengthOverride = bizConfig.namespaceValueLengthLimitOverride();
     if (namespaceValueLengthOverride != null && namespaceValueLengthOverride.containsKey(namespaceId)) {

diff --git a/apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemSetService.java b/apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemSetService.java
@@ -119,7 +119,8 @@ private void doUpdateItems(List<ItemDTO> toUpdateItems, Namespace namespace, Str
       }
       Item beforeUpdateItem = BeanUtils.transform(Item.class, managedItem);
 
-      //protect. only value,comment,lastModifiedBy,lineNum can be modified
+      //protect. only value,type,comment,lastModifiedBy can be modified
+      managedItem.setType(entity.getType());
       managedItem.setValue(entity.getValue());
       managedItem.setComment(entity.getComment());
       managedItem.setLineNum(entity.getLineNum());

diff --git a/apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java b/apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2022 Apollo Authors
+ *
+ * Licensed 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.
+ *
+ */
+package com.ctrip.framework.apollo.biz.service;
+
+import com.ctrip.framework.apollo.biz.AbstractIntegrationTest;
+import com.ctrip.framework.apollo.biz.entity.Item;
+import com.ctrip.framework.apollo.common.exception.BadRequestException;
+import org.junit.Assert;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.context.jdbc.Sql;
+
+public class ItemServiceTest extends AbstractIntegrationTest {
+
+    @Autowired
+    private ItemService itemService;
+
+    @Test
+    @Sql(scripts = "/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);
+
+        try {
+            itemService.save(item);
+            Assert.fail();
+        } catch (Exception e) {
+            Assert.assertTrue(e instanceof BadRequestException);
+        }
+
+        item.setType(0);
+        Item dbItem = itemService.save(item);
+        Assert.assertEquals(0, dbItem.getType());
+    }
+
+    @Test
+    @Sql(scripts = "/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.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);
+        Assert.assertEquals(2, dbItem.getType());
+        Assert.assertEquals("v1-new", dbItem.getValue());
+    }
+
+}
diff --git a/apollo-biz/src/test/resources/sql/item-test.sql b/apollo-biz/src/test/resources/sql/item-test.sql
@@ -0,0 +1,22 @@
+--
+-- Copyright 2022 Apollo Authors
+--
+-- Licensed 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.
+--
+INSERT INTO `item` (`Id`, `NamespaceId`, `Key`, `Type`, `Value`, `Comment`, `LineNum`)
+    VALUES
+        (9901, 1, 'k1', 0, 'v1', '', 1),
+        (9902, 2, 'k2', 2, 'v2', '', 2);
+
+
+