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 2b3ff1743f fix(admin): grant event permissions atomically (#7254)
2b3ff1743f is described below
commit 2b3ff1743f37ae3ac59e1d735daca78ce0fe8f6a
Author: Liming Deng <[email protected]>
AuthorDate: Wed Sep 30 09:59:42 2026 +0800
fix(admin): grant event permissions atomically (#7254)
---
.../shenyu/admin/mapper/DataPermissionMapper.java | 9 ++
.../service/impl/DataPermissionServiceImpl.java | 35 +++---
.../resources/mappers/data-permission-sqlmap.xml | 17 +++
.../mapper/DataPermissionBatchDialectTest.java | 69 ++++++++++++
.../service/PermissionListenerIntegrationTest.java | 120 +++++++++++++++++++++
5 files changed, 234 insertions(+), 16 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/DataPermissionMapper.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/DataPermissionMapper.java
index 4285ce6546..0c1a016eff 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/DataPermissionMapper.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/DataPermissionMapper.java
@@ -46,6 +46,15 @@ public interface DataPermissionMapper {
*/
List<DataPermissionDO> listByUserId(String userId);
+ /**
+ * Find users already granted access to a selector or rule.
+ *
+ * @param dataId selector or rule id
+ * @param dataType permission type
+ * @return granted user ids
+ */
+ List<String> selectUserIds(@Param("dataId") String dataId,
@Param("dataType") Integer dataType);
+
/**
* deleteSelector data permission by user id and data id.
* @param dataId data id
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DataPermissionServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DataPermissionServiceImpl.java
index e0781a26d7..3fa9bfa1fc 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DataPermissionServiceImpl.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DataPermissionServiceImpl.java
@@ -265,6 +265,7 @@ public class DataPermissionServiceImpl implements
DataPermissionService {
* @param event event
*/
@EventListener(SelectorCreatedEvent.class)
+ @Transactional(rollbackFor = Exception.class)
public void onSelectorCreated(final SelectorCreatedEvent event) {
// check selector add
Boolean existed;
@@ -278,20 +279,15 @@ public class DataPermissionServiceImpl implements
DataPermissionService {
dataPermissionDTO.setUserId(JwtUtils.getUserInfo().getUserId());
dataPermissionDTO.setDataId(event.getSelector().getId());
dataPermissionDTO.setDataType(AdminConstants.SELECTOR_DATA_TYPE);
-
dataPermissionMapper.insertSelective(DataPermissionDO.buildPermissionDO(dataPermissionDTO));
+
grantMissingPermissions(Collections.singletonList(dataPermissionDTO.getUserId()),
dataPermissionDTO.getDataId(), dataPermissionDTO.getDataType());
} else {
String namespaceId = event.getSelector().getNamespaceId();
if (StringUtils.isNoneBlank(namespaceId)) {
// support namespace
List<NamespaceUserRelDO> namespaceUserRelDOList =
namespaceUserRelMapper.selectListByNamespaceId(namespaceId);
if (CollectionUtils.isNotEmpty(namespaceUserRelDOList)) {
- namespaceUserRelDOList.forEach(namespaceUserRelDO -> {
- DataPermissionDTO dataPermissionDTO = new
DataPermissionDTO();
-
dataPermissionDTO.setUserId(namespaceUserRelDO.getUserId());
-
dataPermissionDTO.setDataId(event.getSelector().getId());
-
dataPermissionDTO.setDataType(AdminConstants.SELECTOR_DATA_TYPE);
-
dataPermissionMapper.insertSelective(DataPermissionDO.buildPermissionDO(dataPermissionDTO));
- });
+
grantMissingPermissions(namespaceUserRelDOList.stream().map(NamespaceUserRelDO::getUserId).collect(Collectors.toList()),
+ event.getSelector().getId(),
AdminConstants.SELECTOR_DATA_TYPE);
}
}
}
@@ -303,6 +299,7 @@ public class DataPermissionServiceImpl implements
DataPermissionService {
* @param event event
*/
@EventListener(RuleCreatedEvent.class)
+ @Transactional(rollbackFor = Exception.class)
public void onRuleCreated(final RuleCreatedEvent event) {
// check rule add
Boolean existed;
@@ -316,26 +313,32 @@ public class DataPermissionServiceImpl implements
DataPermissionService {
dataPermissionDTO.setUserId(JwtUtils.getUserInfo().getUserId());
dataPermissionDTO.setDataId(event.getRule().getId());
dataPermissionDTO.setDataType(AdminConstants.RULE_DATA_TYPE);
-
dataPermissionMapper.insertSelective(DataPermissionDO.buildPermissionDO(dataPermissionDTO));
+
grantMissingPermissions(Collections.singletonList(dataPermissionDTO.getUserId()),
dataPermissionDTO.getDataId(), dataPermissionDTO.getDataType());
} else {
String namespaceId = event.getRule().getNamespaceId();
if (StringUtils.isNoneBlank(namespaceId)) {
// support namespace
List<NamespaceUserRelDO> namespaceUserRelDOList =
namespaceUserRelMapper.selectListByNamespaceId(namespaceId);
if (CollectionUtils.isNotEmpty(namespaceUserRelDOList)) {
- namespaceUserRelDOList.forEach(namespaceUserRelDO -> {
- DataPermissionDTO dataPermissionDTO = new
DataPermissionDTO();
-
dataPermissionDTO.setUserId(namespaceUserRelDO.getUserId());
- dataPermissionDTO.setDataId(event.getRule().getId());
-
dataPermissionDTO.setDataType(AdminConstants.RULE_DATA_TYPE);
-
dataPermissionMapper.insertSelective(DataPermissionDO.buildPermissionDO(dataPermissionDTO));
- });
+
grantMissingPermissions(namespaceUserRelDOList.stream().map(NamespaceUserRelDO::getUserId).collect(Collectors.toList()),
+ event.getRule().getId(),
AdminConstants.RULE_DATA_TYPE);
}
}
}
}
+ private void grantMissingPermissions(final List<String> userIds, final
String dataId, final Integer dataType) {
+ Set<String> existingUsers = new
HashSet<>(dataPermissionMapper.selectUserIds(dataId, dataType));
+ List<DataPermissionDO> permissions = userIds.stream().distinct()
+ .filter(userId -> !existingUsers.contains(userId))
+ .map(userId ->
DataPermissionDO.buildCreatePermissionDO(dataId, userId, dataType))
+ .collect(Collectors.toList());
+ if (CollectionUtils.isNotEmpty(permissions)) {
+ dataPermissionMapper.insertBatch(permissions);
+ }
+ }
+
/**
* listen {@link BatchSelectorDeletedEvent} delete data permission.
*
diff --git a/shenyu-admin/src/main/resources/mappers/data-permission-sqlmap.xml
b/shenyu-admin/src/main/resources/mappers/data-permission-sqlmap.xml
index e2fd69ffc4..a8ca52367e 100644
--- a/shenyu-admin/src/main/resources/mappers/data-permission-sqlmap.xml
+++ b/shenyu-admin/src/main/resources/mappers/data-permission-sqlmap.xml
@@ -156,6 +156,12 @@
</trim>
</insert>
+ <select id="selectUserIds" resultType="java.lang.String">
+ SELECT user_id FROM data_permission
+ WHERE data_id = #{dataId,jdbcType=VARCHAR}
+ AND data_type = #{dataType,jdbcType=INTEGER}
+ </select>
+
<insert id="insertBatch">
INSERT INTO data_permission (id,
user_id,
@@ -172,4 +178,15 @@
</foreach>
</insert>
+ <insert id="insertBatch" databaseId="oracle">
+ INSERT INTO data_permission (id, user_id, data_id, data_type)
+ <foreach collection="dataPermissionList" item="permission" separator="
UNION ALL ">
+ SELECT CAST(#{permission.id,jdbcType=VARCHAR} AS VARCHAR2(128)),
+ CAST(#{permission.userId,jdbcType=VARCHAR} AS
VARCHAR2(128)),
+ CAST(#{permission.dataId,jdbcType=VARCHAR} AS
VARCHAR2(128)),
+ CAST(#{permission.dataType,jdbcType=INTEGER} AS NUMBER(10))
+ FROM dual
+ </foreach>
+ </insert>
+
</mapper>
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/mapper/DataPermissionBatchDialectTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/mapper/DataPermissionBatchDialectTest.java
new file mode 100644
index 0000000000..0e598f12d5
--- /dev/null
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/mapper/DataPermissionBatchDialectTest.java
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+
+package org.apache.shenyu.admin.mapper;
+
+import org.apache.ibatis.builder.xml.XMLMapperBuilder;
+import org.apache.ibatis.mapping.Environment;
+import org.apache.ibatis.session.Configuration;
+import org.apache.ibatis.session.SqlSession;
+import org.apache.ibatis.session.SqlSessionFactoryBuilder;
+import org.apache.ibatis.transaction.jdbc.JdbcTransactionFactory;
+import org.apache.shenyu.admin.model.entity.DataPermissionDO;
+import org.junit.jupiter.api.Test;
+import org.springframework.core.io.ClassPathResource;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.datasource.DriverManagerDataSource;
+
+import java.io.InputStream;
+import java.util.List;
+import java.util.UUID;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Oracle-mode regression for batch permission grants.
+ */
+public class DataPermissionBatchDialectTest {
+
+ @Test
+ public void testOracleBatchInsertAndUserLookup() throws Exception {
+ DriverManagerDataSource dataSource = new
DriverManagerDataSource("jdbc:h2:mem:permission-" + UUID.randomUUID()
+ + ";DB_CLOSE_DELAY=-1;MODE=Oracle", "sa", "");
+ JdbcTemplate jdbc = new JdbcTemplate(dataSource);
+ try {
+ jdbc.execute("CREATE TABLE data_permission (id VARCHAR(128)
PRIMARY KEY, user_id VARCHAR(128) NOT NULL, data_id VARCHAR(128), data_type
INTEGER)");
+ Configuration configuration = new Configuration(new
Environment("test", new JdbcTransactionFactory(), dataSource));
+ configuration.setDatabaseId("oracle");
+ ClassPathResource resource = new
ClassPathResource("mappers/data-permission-sqlmap.xml");
+ try (InputStream input = resource.getInputStream()) {
+ new XMLMapperBuilder(input, configuration,
resource.toString(), configuration.getSqlFragments()).parse();
+ }
+ try (SqlSession session = new
SqlSessionFactoryBuilder().build(configuration).openSession()) {
+ DataPermissionMapper mapper =
session.getMapper(DataPermissionMapper.class);
+ assertEquals(2,
mapper.insertBatch(List.of(DataPermissionDO.buildCreatePermissionDO("data",
"first", 0),
+ DataPermissionDO.buildCreatePermissionDO("data",
"second", 0))));
+ assertEquals(2, mapper.selectUserIds("data", 0).size());
+ assertEquals(0, mapper.selectUserIds("data", 1).size());
+ }
+ } finally {
+ jdbc.execute("SHUTDOWN");
+ }
+ }
+}
+
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/PermissionListenerIntegrationTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/PermissionListenerIntegrationTest.java
new file mode 100644
index 0000000000..2d686305b9
--- /dev/null
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/PermissionListenerIntegrationTest.java
@@ -0,0 +1,120 @@
+/*
+ * 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.
+ */
+
+
+package org.apache.shenyu.admin.service;
+
+import jakarta.annotation.Resource;
+import org.apache.shenyu.admin.AbstractSpringIntegrationTest;
+import org.apache.shenyu.admin.mapper.NamespaceUserRelMapper;
+import org.apache.shenyu.admin.model.entity.NamespaceUserRelDO;
+import org.apache.shenyu.admin.model.entity.RuleDO;
+import org.apache.shenyu.admin.model.entity.SelectorDO;
+import org.apache.shenyu.admin.model.event.rule.RuleCreatedEvent;
+import org.apache.shenyu.admin.model.event.selector.SelectorCreatedEvent;
+import org.apache.shenyu.admin.service.impl.DataPermissionServiceImpl;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.dao.DataIntegrityViolationException;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.transaction.PlatformTransactionManager;
+import org.springframework.transaction.support.TransactionTemplate;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.when;
+
+/**
+ * Verify permission listeners grant namespace users atomically.
+ */
+public class PermissionListenerIntegrationTest extends
AbstractSpringIntegrationTest {
+
+ @Resource
+ private DataPermissionServiceImpl permissionService;
+
+ @Resource
+ private JdbcTemplate jdbcTemplate;
+
+ @Resource
+ private PlatformTransactionManager transactionManager;
+
+ @MockBean
+ private NamespaceUserRelMapper namespaceUserRelMapper;
+
+ @AfterEach
+ public void cleanup() {
+ jdbcTemplate.update("DELETE FROM data_permission WHERE data_id =
'listener-data'");
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void testRepeatedEventDoesNotDuplicateGrants(final boolean rule) {
+
when(namespaceUserRelMapper.selectListByNamespaceId("listener-namespace")).thenReturn(List.of(user("first"),
user("second"), user("first")));
+ invokeListener(rule);
+ invokeListener(rule);
+ assertEquals(2, countPermissions());
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void testInvalidUserRollsBackAllGrants(final boolean rule) {
+
when(namespaceUserRelMapper.selectListByNamespaceId("listener-namespace")).thenReturn(List.of(user("first"),
user(null)));
+ assertThrows(DataIntegrityViolationException.class, () ->
invokeListener(rule));
+ assertEquals(0, countPermissions());
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void testOuterRollbackRemovesAllGrants(final boolean rule) {
+
when(namespaceUserRelMapper.selectListByNamespaceId("listener-namespace")).thenReturn(List.of(user("first"),
user("second")));
+ new
TransactionTemplate(transactionManager).executeWithoutResult(status -> {
+ invokeListener(rule);
+ assertEquals(2, countPermissions());
+ status.setRollbackOnly();
+ });
+ assertEquals(0, countPermissions());
+ }
+
+ private void invokeListener(final boolean rule) {
+ if (rule) {
+ RuleDO data = new RuleDO();
+ data.setId("listener-data");
+ data.setNamespaceId("listener-namespace");
+ permissionService.onRuleCreated(new RuleCreatedEvent(data,
"test"));
+ } else {
+ SelectorDO data = new SelectorDO();
+ data.setId("listener-data");
+ data.setNamespaceId("listener-namespace");
+ permissionService.onSelectorCreated(new SelectorCreatedEvent(data,
"test"));
+ }
+ }
+
+ private NamespaceUserRelDO user(final String id) {
+ NamespaceUserRelDO user = new NamespaceUserRelDO();
+ user.setUserId(id);
+ return user;
+ }
+
+ private int countPermissions() {
+ return jdbcTemplate.queryForObject("SELECT COUNT(*) FROM
data_permission WHERE data_id = 'listener-data'", Integer.class);
+ }
+}
+
