This is an automated email from the ASF dual-hosted git repository.
Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new 1911e5b2ce [type:fix] Preserve parent metadata and ancestor hierarchy
on tag upd… (#7800)
1911e5b2ce is described below
commit 1911e5b2ced8b74f88b8c712aaa1bf803e56e3fa
Author: Harsh Raj <[email protected]>
AuthorDate: Fri Oct 23 08:44:17 2026 +0530
[type:fix] Preserve parent metadata and ancestor hierarchy on tag upd…
(#7800)
* [type:fix] Preserve parent metadata and ancestor hierarchy on tag update
(#7772)
* [type:fix] Address review comments on root ext consistency and null tag
safety (#7772)
---------
Co-authored-by: Harsh <[email protected]>
---
.../shenyu/admin/service/impl/TagServiceImpl.java | 106 +++++++++++++--------
.../shenyu/admin/service/TagServiceTest.java | 31 ++++++
.../admin/service/TagUpdateTransactionTest.java | 106 ++++++++++++++++++++-
3 files changed, 202 insertions(+), 41 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/TagServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/TagServiceImpl.java
index 963d0e1bff..4c2b91e5bf 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/TagServiceImpl.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/TagServiceImpl.java
@@ -35,12 +35,13 @@ import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import java.sql.Timestamp;
-import java.util.List;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.Set;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -95,8 +96,20 @@ public class TagServiceImpl implements TagService {
public int update(final TagDTO tagDTO) {
TagDO before = tagMapper.selectByPrimaryKey(tagDTO.getId());
Assert.notNull(before, "the updated tag is not found");
+ String parentTagId = StringUtils.isNotEmpty(tagDTO.getParentTagId()) ?
tagDTO.getParentTagId() : before.getParentTagId();
+ if (StringUtils.isEmpty(parentTagId)) {
+ parentTagId = AdminConstants.TAG_ROOT_PARENT_ID;
+ }
TagDO tagDO = TagDO.buildTagDO(tagDTO);
- updateSubTags(tagDTO);
+ tagDO.setParentTagId(parentTagId);
+ if (AdminConstants.TAG_ROOT_PARENT_ID.equals(parentTagId)) {
+ tagDO.setExt(buildRootExt(before.getExt()));
+ } else {
+ TagDO parentTagDO = tagMapper.selectByPrimaryKey(parentTagId);
+ Assert.notNull(parentTagDO, "parent tag is not found");
+ tagDO.setExt(buildExtParamByParentTag(parentTagDO,
before.getExt()));
+ }
+ updateSubTags(tagDO);
return tagMapper.updateByPrimaryKeySelective(tagDO);
}
@@ -167,25 +180,20 @@ public class TagServiceImpl implements TagService {
/**
* update sub tags.
*
- * @param tagDTO tagDTO
+ * @param tagDO tagDO
*/
- private void updateSubTags(final TagDTO tagDTO) {
- List<TagDO> allData = tagMapper.selectByQuery(new TagQuery());
+ private void updateSubTags(final TagDO tagDO) {
+ List<TagDO> allData = Optional.ofNullable(tagMapper.selectByQuery(new
TagQuery())).orElseGet(Lists::newArrayList);
Map<String, TagDO> allDataMap = allData.stream().collect(
- Collectors.toMap(BaseDO::getId, Function.identity(), (a, b) ->
b, ConcurrentHashMap::new));
- TagDO update = TagDO.buildTagDO(tagDTO);
- allDataMap.put(update.getId(), update);
- Map<String, List<String>> relationMap = new ConcurrentHashMap<>();
- allDataMap.keySet().stream().map(allDataMap::get).forEach(tagDO -> {
- if
(CollectionUtils.isEmpty(relationMap.get(tagDO.getParentTagId()))) {
- relationMap.put(tagDO.getParentTagId(),
Lists.newArrayList(tagDO.getId()));
- } else {
- List<String> list = relationMap.get(tagDO.getParentTagId());
- list.add(tagDO.getId());
- relationMap.put(tagDO.getParentTagId(), list);
+ Collectors.toMap(BaseDO::getId, Function.identity(), (a, b) ->
b, HashMap::new));
+ allDataMap.put(tagDO.getId(), tagDO);
+ Map<String, List<String>> relationMap = new HashMap<>();
+ allDataMap.values().forEach(item -> {
+ if (StringUtils.isNotEmpty(item.getParentTagId())) {
+ relationMap.computeIfAbsent(item.getParentTagId(), k ->
Lists.newArrayList()).add(item.getId());
}
});
- recurseUpdateTag(allDataMap, relationMap, tagDTO.getId(), new
HashSet<>());
+ recurseUpdateTag(allDataMap, relationMap, tagDO.getId(), new
HashSet<>());
}
/**
@@ -204,12 +212,30 @@ public class TagServiceImpl implements TagService {
List<String> subTagIds = relationMap.get(id);
subTagIds.forEach(tagId -> {
TagDO tagDO = allData.get(tagId);
- tagDO.setExt(buildExtParamByParentTag(allData.get(id)));
+ TagDO parentTagDO = allData.get(id);
+ if (Objects.isNull(tagDO) || Objects.isNull(parentTagDO)) {
+ return;
+ }
+ tagDO.setExt(buildExtParamByParentTag(parentTagDO,
tagDO.getExt()));
tagMapper.updateByPrimaryKey(tagDO);
recurseUpdateTag(allData, relationMap, tagId, visited);
});
}
+ /**
+ * buildRootExt.
+ *
+ * @param currentExt currentExt
+ * @return ext
+ */
+ private String buildRootExt(final String currentExt) {
+ final TagDO.TagExt ext = StringUtils.isNotEmpty(currentExt)
+ ?
Optional.ofNullable(GsonUtils.getInstance().fromJson(currentExt,
TagDO.TagExt.class)).orElse(new TagDO.TagExt())
+ : new TagDO.TagExt();
+ ext.setParent(null);
+ return GsonUtils.getInstance().toJson(ext);
+ }
+
/**
* buildExtParam.
*
@@ -217,26 +243,28 @@ public class TagServiceImpl implements TagService {
* @return ext
*/
private String buildExtParamByParentTag(final TagDO parentTagDO) {
- String ext = "";
- if (parentTagDO.getId().equals(AdminConstants.TAG_ROOT_PARENT_ID)) {
- final TagDO.TagExt parent = new TagDO.TagExt();
- TagDO.TagExt tagExt = new TagDO.TagExt();
- tagExt.setDesc(parentTagDO.getTagDesc());
- tagExt.setName(parentTagDO.getTagName());
- tagExt.setId(parentTagDO.getId());
- tagExt.setRefreshTime(parent.getRefreshTime());
- tagExt.setApiDocMd5(parent.getApiDocMd5());
- parent.setParent(tagExt);
- ext = GsonUtils.getInstance().toJson(parent);
- } else {
- TagDO.TagExt parentTagExt =
Optional.ofNullable(GsonUtils.getInstance().fromJson(parentTagDO.getExt(),
TagDO.TagExt.class)).orElse(new TagDO.TagExt());
- final TagDO.TagExt tagExt = new TagDO.TagExt();
- parentTagExt.setDesc(parentTagDO.getTagDesc());
- parentTagExt.setName(parentTagDO.getTagName());
- parentTagExt.setId(parentTagDO.getId());
- tagExt.setParent(parentTagExt);
- ext = GsonUtils.getInstance().toJson(tagExt);
+ return buildExtParamByParentTag(parentTagDO, null);
+ }
+
+ /**
+ * buildExtParam.
+ *
+ * @param parentTagDO parentTagDO
+ * @param currentExt currentExt
+ * @return ext
+ */
+ private String buildExtParamByParentTag(final TagDO parentTagDO, final
String currentExt) {
+ if (AdminConstants.TAG_ROOT_PARENT_ID.equals(parentTagDO.getId())) {
+ return buildRootExt(currentExt);
}
- return ext;
+ TagDO.TagExt parentTagExt =
Optional.ofNullable(GsonUtils.getInstance().fromJson(parentTagDO.getExt(),
TagDO.TagExt.class)).orElse(new TagDO.TagExt());
+ final TagDO.TagExt tagExt = StringUtils.isNotEmpty(currentExt)
+ ?
Optional.ofNullable(GsonUtils.getInstance().fromJson(currentExt,
TagDO.TagExt.class)).orElse(new TagDO.TagExt())
+ : new TagDO.TagExt();
+ parentTagExt.setDesc(parentTagDO.getTagDesc());
+ parentTagExt.setName(parentTagDO.getTagName());
+ parentTagExt.setId(parentTagDO.getId());
+ tagExt.setParent(parentTagExt);
+ return GsonUtils.getInstance().toJson(tagExt);
}
}
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/TagServiceTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/TagServiceTest.java
index 79cb1a4ea9..adc1d3a8c5 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/TagServiceTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/TagServiceTest.java
@@ -40,6 +40,7 @@ import java.util.List;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
@@ -98,6 +99,36 @@ public class TagServiceTest {
assertEquals(cnt, 1);
}
+ @Test
+ public void testUpdateWithNullParentTagId() {
+ TagDTO tagDTO = new TagDTO();
+ tagDTO.setId("123");
+ tagDTO.setName("name");
+ tagDTO.setParentTagId(null);
+ TagDO before = buildTagDO();
+ before.setParentTagId(null);
+ given(this.tagMapper.selectByPrimaryKey("123")).willReturn(before);
+ given(this.tagMapper.updateByPrimaryKeySelective(any())).willReturn(1);
+ int cnt = tagService.update(tagDTO);
+ assertEquals(cnt, 1);
+ assertNull(tagDTO.getParentTagId());
+ }
+
+ @Test
+ public void testUpdateWithNonExistentTag() {
+ TagDTO tagDTO = buildTagDTO();
+ given(this.tagMapper.selectByPrimaryKey("789")).willReturn(null);
+ assertThrows(ValidFailException.class, () ->
tagService.update(tagDTO));
+ }
+
+ @Test
+ public void testUpdateWithNonExistentParentTag() {
+ TagDTO tagDTO = buildTagDTO();
+
given(this.tagMapper.selectByPrimaryKey("789")).willReturn(buildTagDO());
+ given(this.tagMapper.selectByPrimaryKey("456")).willReturn(null);
+ assertThrows(ValidFailException.class, () ->
tagService.update(tagDTO));
+ }
+
@Test
public void testDelete() {
given(this.tagMapper.deleteByIds(any())).willReturn(1);
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/TagUpdateTransactionTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/TagUpdateTransactionTest.java
index a974042007..6394a4e7b7 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/TagUpdateTransactionTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/TagUpdateTransactionTest.java
@@ -20,8 +20,10 @@ package org.apache.shenyu.admin.service;
import jakarta.annotation.Resource;
import org.apache.shenyu.admin.AbstractSpringIntegrationTest;
-import org.apache.shenyu.admin.model.dto.TagDTO;
import org.apache.shenyu.admin.exception.ValidFailException;
+import org.apache.shenyu.admin.model.dto.TagDTO;
+import org.apache.shenyu.admin.model.entity.TagDO;
+import org.apache.shenyu.common.utils.GsonUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -32,6 +34,8 @@ import org.springframework.jdbc.core.JdbcTemplate;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
/**
@@ -49,11 +53,13 @@ public class TagUpdateTransactionTest extends
AbstractSpringIntegrationTest {
public void setup() {
jdbcTemplate.update("INSERT INTO tag (id, tag_name, tag_desc,
parent_tag_id, ext) VALUES ('update-parent', 'old', '', '0', '{}')");
jdbcTemplate.update("INSERT INTO tag (id, tag_name, tag_desc,
parent_tag_id, ext) VALUES ('update-child', 'child', '', 'update-parent',
'{}')");
+ jdbcTemplate.update("INSERT INTO tag (id, tag_name, tag_desc,
parent_tag_id, ext) VALUES ('update-grandchild', 'grandchild', '',
'update-child', '{}')");
+ jdbcTemplate.update("INSERT INTO tag (id, tag_name, tag_desc,
parent_tag_id, ext) VALUES ('update-parent-2', 'parent2', '', '0', '{}')");
}
@AfterEach
public void cleanup() {
- jdbcTemplate.update("DELETE FROM tag WHERE id IN ('update-child',
'update-parent')");
+ jdbcTemplate.update("DELETE FROM tag WHERE id IN ('update-grandchild',
'update-child', 'update-parent', 'update-parent-2')");
}
@Test
@@ -91,6 +97,102 @@ public class TagUpdateTransactionTest extends
AbstractSpringIntegrationTest {
assertNotEquals("{}", childExt());
}
+ @Test
+ public void testUpdateChildPreservesFullAncestryAndUpdatesExt() {
+ TagDTO dto = new TagDTO();
+ dto.setId("update-child");
+ dto.setParentTagId("update-parent");
+ dto.setName("new-child-name");
+ dto.setTagDesc("child-desc");
+ assertEquals(1, tagService.update(dto));
+
+ String childExt = jdbcTemplate.queryForObject("SELECT ext FROM tag
WHERE id = 'update-child'", String.class);
+ TagDO.TagExt childTagExt = GsonUtils.getInstance().fromJson(childExt,
TagDO.TagExt.class);
+ assertNotNull(childTagExt);
+ assertNotNull(childTagExt.getParent());
+ assertEquals("update-parent", childTagExt.getParent().getId());
+ assertEquals("old", childTagExt.getParent().getName());
+
+ String grandchildExt = jdbcTemplate.queryForObject("SELECT ext FROM
tag WHERE id = 'update-grandchild'", String.class);
+ TagDO.TagExt grandchildTagExt =
GsonUtils.getInstance().fromJson(grandchildExt, TagDO.TagExt.class);
+ assertNotNull(grandchildTagExt);
+ assertNotNull(grandchildTagExt.getParent());
+ assertEquals("update-child", grandchildTagExt.getParent().getId());
+ assertEquals("new-child-name", grandchildTagExt.getParent().getName());
+ assertNotNull(grandchildTagExt.getParent().getParent());
+ assertEquals("update-parent",
grandchildTagExt.getParent().getParent().getId());
+ assertEquals("old",
grandchildTagExt.getParent().getParent().getName());
+ }
+
+ @Test
+ public void testReparentChildTagUpdatesExtAndDescendantsAncestry() {
+ TagDTO dto = new TagDTO();
+ dto.setId("update-child");
+ dto.setParentTagId("update-parent-2");
+ dto.setName("child-moved");
+ dto.setTagDesc("child-desc");
+ assertEquals(1, tagService.update(dto));
+
+ String childExt = jdbcTemplate.queryForObject("SELECT ext FROM tag
WHERE id = 'update-child'", String.class);
+ TagDO.TagExt childTagExt = GsonUtils.getInstance().fromJson(childExt,
TagDO.TagExt.class);
+ assertNotNull(childTagExt);
+ assertNotNull(childTagExt.getParent());
+ assertEquals("update-parent-2", childTagExt.getParent().getId());
+
+ String grandchildExt = jdbcTemplate.queryForObject("SELECT ext FROM
tag WHERE id = 'update-grandchild'", String.class);
+ TagDO.TagExt grandchildTagExt =
GsonUtils.getInstance().fromJson(grandchildExt, TagDO.TagExt.class);
+ assertNotNull(grandchildTagExt);
+ assertNotNull(grandchildTagExt.getParent());
+ assertEquals("update-child", grandchildTagExt.getParent().getId());
+ assertNotNull(grandchildTagExt.getParent().getParent());
+ assertEquals("update-parent-2",
grandchildTagExt.getParent().getParent().getId());
+ }
+
+ @Test
+ public void testPromoteChildToRootClearsParentInExt() {
+ TagDTO dto = new TagDTO();
+ dto.setId("update-child");
+ dto.setParentTagId("0");
+ dto.setName("child-root");
+ dto.setTagDesc("child-desc");
+ assertEquals(1, tagService.update(dto));
+
+ String childExt = jdbcTemplate.queryForObject("SELECT ext FROM tag
WHERE id = 'update-child'", String.class);
+ TagDO.TagExt childTagExt = GsonUtils.getInstance().fromJson(childExt,
TagDO.TagExt.class);
+ assertNotNull(childTagExt);
+ assertNull(childTagExt.getParent());
+
+ String grandchildExt = jdbcTemplate.queryForObject("SELECT ext FROM
tag WHERE id = 'update-grandchild'", String.class);
+ TagDO.TagExt grandchildTagExt =
GsonUtils.getInstance().fromJson(grandchildExt, TagDO.TagExt.class);
+ assertNotNull(grandchildTagExt);
+ assertNotNull(grandchildTagExt.getParent());
+ assertEquals("update-child", grandchildTagExt.getParent().getId());
+ assertNull(grandchildTagExt.getParent().getParent());
+ }
+
+ @Test
+ public void testUpdateWithNonExistentParentThrowsValidFailException() {
+ TagDTO dto = new TagDTO();
+ dto.setId("update-child");
+ dto.setParentTagId("non-existent-parent");
+ dto.setName("child");
+ assertThrows(ValidFailException.class, () -> tagService.update(dto));
+ }
+
+ @Test
+ public void testUpdateTagWithoutParentTagIdInDtoPreservesRootExt() {
+ TagDTO dto = new TagDTO();
+ dto.setId("update-parent");
+ dto.setName("new-parent-name");
+ dto.setTagDesc("desc");
+ assertEquals(1, tagService.update(dto));
+
+ String ext = jdbcTemplate.queryForObject("SELECT ext FROM tag WHERE id
= 'update-parent'", String.class);
+ TagDO.TagExt tagExt = GsonUtils.getInstance().fromJson(ext,
TagDO.TagExt.class);
+ assertNotNull(tagExt);
+ assertNull(tagExt.getParent());
+ }
+
private String childExt() {
return jdbcTemplate.queryForObject("SELECT ext FROM tag WHERE id =
'update-child'", String.class);
}
