moremind commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1712529095
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/SelectorMapper.java:
##########
@@ -44,13 +44,22 @@ public interface SelectorMapper extends ExistProvider {
@Override
Boolean existed(@Param("id") Serializable id);
+ /**
+ * select selector by id.
+ *
+ * @param id primary key.
+ * @param namespaceId namespaceId.
+ * @return {@linkplain SelectorDO}
+ */
+ SelectorDO selectByIdAndNamespaceId(String id, String namespaceId);
+
/**
* select selector by id.
*
* @param id primary key.
* @return {@linkplain SelectorDO}
*/
- SelectorDO selectById(String id);
+ SelectorDO selectByOnlyId(String id);
Review Comment:
just named selectById
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/model/entity/SelectorDO.java:
##########
@@ -393,14 +421,15 @@ public static SelectorData transFrom(final SelectorDO
selectorDO, final String p
.conditionList(conditionDataList)
.matchRestful(selectorDO.getMatchRestful())
.beforeConditionList(beforeConditionDataList)
+ .namespaceId(selectorDO.getNamespaceId())
.build();
}
-
+
/**
* selector data transform.
*
- * @param selectorDO selector entity
- * @param pluginName plugin name
+ * @param selectorDO selector entity
+ * @param pluginName plugin name
Review Comment:
not change the code
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/SelectorController.java:
##########
@@ -138,12 +143,12 @@ public ShenyuAdminResult batchEnabled(@Valid @RequestBody
final BatchCommonDTO b
/**
* delete Selectors.
*
- * @param ids primary key.
+ * @param batchNamespaceCommonDTO batchNamespaceCommonDTO.
* @return {@linkplain ShenyuAdminResult}
*/
@DeleteMapping("/batch")
- public ShenyuAdminResult deleteSelector(@RequestBody @NotEmpty final
List<@NotBlank String> ids) {
Review Comment:
you should delete selected ids in namespace
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/NamespacePluginRelMapper.java:
##########
@@ -65,20 +65,19 @@ public interface NamespacePluginRelMapper extends
ExistProvider {
/**
* select plugin by namespacePluginId.
*
- * @param id primary key.
+ * @param pluginId primary key.
* @param namespaceId namespace id.
* @return {@linkplain PluginVO}
*/
- NamespacePluginVO selectById(String id, String namespaceId);
+ NamespacePluginVO selectByPluginIdAndNamespaceId(String pluginId, String
namespaceId);
Review Comment:
this is id or pluginId?
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/SelectorService.java:
##########
@@ -107,55 +107,61 @@ default int createOrUpdate(SelectorDTO selectorDTO) {
/**
* delete selectors.
*
- * @param ids primary key.
+ * @param ids primary key.
+ * @param namespaceId namespaceId.
* @return rows int
*/
- int delete(List<String> ids);
+ int delete(List<String> ids, String namespaceId);
/**
* find selector by id.
*
- * @param id primary key.
+ * @param id primary key.
+ * @param namespaceId namespaceId.
* @return {@linkplain SelectorVO}
*/
- SelectorVO findById(String id);
+ SelectorVO findById(String id, String namespaceId);
/**
* find selector by name.
*
- * @param name the name
+ * @param name the name
+ * @param namespaceId namespaceId.
+ * Therefore, it is recommended to: {@linkplain
SelectorService#findListByName(java.lang.String, java.lang.String)}
* @return selector do
* @deprecated sice 2.6.0 Deprecated. By querying under this condition,
multiple data are usually obtained.
- * Therefore, it is recommended to: {@linkplain
SelectorService#findListByName(java.lang.String)}
*/
@Deprecated
- SelectorDO findByName(String name);
+ SelectorDO findByName(String name, String namespaceId);
Review Comment:
change name
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/SelectorController.java:
##########
@@ -138,12 +143,12 @@ public ShenyuAdminResult batchEnabled(@Valid @RequestBody
final BatchCommonDTO b
/**
* delete Selectors.
*
- * @param ids primary key.
+ * @param batchNamespaceCommonDTO batchNamespaceCommonDTO.
* @return {@linkplain ShenyuAdminResult}
*/
@DeleteMapping("/batch")
- public ShenyuAdminResult deleteSelector(@RequestBody @NotEmpty final
List<@NotBlank String> ids) {
Review Comment:
the api is error, the api is batch operation
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/SelectorService.java:
##########
@@ -107,55 +107,61 @@ default int createOrUpdate(SelectorDTO selectorDTO) {
/**
* delete selectors.
*
- * @param ids primary key.
+ * @param ids primary key.
+ * @param namespaceId namespaceId.
* @return rows int
*/
- int delete(List<String> ids);
+ int delete(List<String> ids, String namespaceId);
/**
* find selector by id.
*
- * @param id primary key.
+ * @param id primary key.
+ * @param namespaceId namespaceId.
* @return {@linkplain SelectorVO}
*/
- SelectorVO findById(String id);
+ SelectorVO findById(String id, String namespaceId);
Review Comment:
named findByIdAndNamespaceId
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/SelectorService.java:
##########
@@ -107,55 +107,61 @@ default int createOrUpdate(SelectorDTO selectorDTO) {
/**
* delete selectors.
*
- * @param ids primary key.
+ * @param ids primary key.
+ * @param namespaceId namespaceId.
* @return rows int
*/
- int delete(List<String> ids);
+ int delete(List<String> ids, String namespaceId);
Review Comment:
change method name
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/model/event/plugin/BatchPluginDeletedEvent.java:
##########
@@ -31,14 +31,14 @@
* BatchPluginDeletedEvent.
*/
public class BatchPluginDeletedEvent extends BatchPluginChangedEvent {
-
+
private final List<String> deletedPluginIds;
-
+
/**
* Create a new {@code PluginChangedEvent}.operator is unknown.
*
- * @param source Current plugin state
- * @param operator operator
+ * @param source Current plugin state
+ * @param operator operator
Review Comment:
not change the comment
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/PluginServiceImpl.java:
##########
@@ -130,6 +135,12 @@ public String delete(final List<String> ids) {
if (CollectionUtils.isEmpty(plugins)) {
return AdminConstants.SYS_PLUGIN_ID_NOT_EXIST;
}
+ Optional<PluginDO> exist = plugins.stream()
+ .filter(value ->
!Objects.isNull(this.namespacePluginRelMapper.selectByPluginId(value.getId())))
Review Comment:
use Objects.nonNull()
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/SelectorService.java:
##########
@@ -107,55 +107,61 @@ default int createOrUpdate(SelectorDTO selectorDTO) {
/**
* delete selectors.
*
- * @param ids primary key.
+ * @param ids primary key.
+ * @param namespaceId namespaceId.
* @return rows int
*/
- int delete(List<String> ids);
+ int delete(List<String> ids, String namespaceId);
/**
* find selector by id.
*
- * @param id primary key.
+ * @param id primary key.
+ * @param namespaceId namespaceId.
* @return {@linkplain SelectorVO}
*/
- SelectorVO findById(String id);
+ SelectorVO findById(String id, String namespaceId);
/**
* find selector by name.
*
- * @param name the name
+ * @param name the name
+ * @param namespaceId namespaceId.
+ * Therefore, it is recommended to: {@linkplain
SelectorService#findListByName(java.lang.String, java.lang.String)}
* @return selector do
* @deprecated sice 2.6.0 Deprecated. By querying under this condition,
multiple data are usually obtained.
- * Therefore, it is recommended to: {@linkplain
SelectorService#findListByName(java.lang.String)}
*/
@Deprecated
- SelectorDO findByName(String name);
+ SelectorDO findByName(String name, String namespaceId);
/**
* find selector list by name.
*
- * @param name name
+ * @param name name
+ * @param namespaceId namespaceId.
* @return list
*/
- List<SelectorDO> findListByName(String name);
+ List<SelectorDO> findListByName(String name, String namespaceId);
Review Comment:
change name
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/PluginServiceImpl.java:
##########
@@ -130,6 +135,12 @@ public String delete(final List<String> ids) {
if (CollectionUtils.isEmpty(plugins)) {
return AdminConstants.SYS_PLUGIN_ID_NOT_EXIST;
}
+ Optional<PluginDO> exist = plugins.stream()
+ .filter(value ->
!Objects.isNull(this.namespacePluginRelMapper.selectByPluginId(value.getId())))
+ .findAny();
+ if (exist.isPresent()) {
Review Comment:
why findAny?
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/SelectorService.java:
##########
@@ -107,55 +107,61 @@ default int createOrUpdate(SelectorDTO selectorDTO) {
/**
* delete selectors.
*
- * @param ids primary key.
+ * @param ids primary key.
+ * @param namespaceId namespaceId.
* @return rows int
*/
- int delete(List<String> ids);
+ int delete(List<String> ids, String namespaceId);
/**
* find selector by id.
*
- * @param id primary key.
+ * @param id primary key.
+ * @param namespaceId namespaceId.
* @return {@linkplain SelectorVO}
*/
- SelectorVO findById(String id);
+ SelectorVO findById(String id, String namespaceId);
/**
* find selector by name.
*
- * @param name the name
+ * @param name the name
+ * @param namespaceId namespaceId.
+ * Therefore, it is recommended to: {@linkplain
SelectorService#findListByName(java.lang.String, java.lang.String)}
* @return selector do
* @deprecated sice 2.6.0 Deprecated. By querying under this condition,
multiple data are usually obtained.
- * Therefore, it is recommended to: {@linkplain
SelectorService#findListByName(java.lang.String)}
*/
@Deprecated
- SelectorDO findByName(String name);
+ SelectorDO findByName(String name, String namespaceId);
/**
* find selector list by name.
*
- * @param name name
+ * @param name name
+ * @param namespaceId namespaceId.
* @return list
*/
- List<SelectorDO> findListByName(String name);
+ List<SelectorDO> findListByName(String name, String namespaceId);
/**
* Find by name and plugin id selector do.
*
- * @param name the name
- * @param pluginName the plugin name
+ * @param name the name
+ * @param pluginName the plugin name
+ * @param namespaceId the namespaceId
* @return the selector do
*/
- SelectorDO findByNameAndPluginName(String name, String pluginName);
+ SelectorDO findByNameAndPluginName(String name, String pluginName, String
namespaceId);
Review Comment:
change name
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/SelectorService.java:
##########
@@ -107,55 +107,61 @@ default int createOrUpdate(SelectorDTO selectorDTO) {
/**
* delete selectors.
*
- * @param ids primary key.
+ * @param ids primary key.
+ * @param namespaceId namespaceId.
* @return rows int
*/
- int delete(List<String> ids);
+ int delete(List<String> ids, String namespaceId);
/**
* find selector by id.
*
- * @param id primary key.
+ * @param id primary key.
+ * @param namespaceId namespaceId.
* @return {@linkplain SelectorVO}
*/
- SelectorVO findById(String id);
+ SelectorVO findById(String id, String namespaceId);
/**
* find selector by name.
*
- * @param name the name
+ * @param name the name
+ * @param namespaceId namespaceId.
+ * Therefore, it is recommended to: {@linkplain
SelectorService#findListByName(java.lang.String, java.lang.String)}
* @return selector do
* @deprecated sice 2.6.0 Deprecated. By querying under this condition,
multiple data are usually obtained.
- * Therefore, it is recommended to: {@linkplain
SelectorService#findListByName(java.lang.String)}
*/
@Deprecated
- SelectorDO findByName(String name);
+ SelectorDO findByName(String name, String namespaceId);
/**
* find selector list by name.
*
- * @param name name
+ * @param name name
+ * @param namespaceId namespaceId.
* @return list
*/
- List<SelectorDO> findListByName(String name);
+ List<SelectorDO> findListByName(String name, String namespaceId);
/**
* Find by name and plugin id selector do.
*
- * @param name the name
- * @param pluginName the plugin name
+ * @param name the name
+ * @param pluginName the plugin name
+ * @param namespaceId the namespaceId
* @return the selector do
*/
- SelectorDO findByNameAndPluginName(String name, String pluginName);
+ SelectorDO findByNameAndPluginName(String name, String pluginName, String
namespaceId);
/**
* findByNameAndPluginNameForUpdate.
*
- * @param name name
- * @param pluginName pluginName
+ * @param name name
+ * @param pluginName pluginName
+ * @param namespaceId namespaceId.
* @return SelectorDO
*/
- SelectorDO findByNameAndPluginNameForUpdate(String name, String
pluginName);
+ SelectorDO findByNameAndPluginNameForUpdate(String name, String
pluginName, String namespaceId);
Review Comment:
change name
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/SelectorService.java:
##########
@@ -202,10 +210,11 @@ default int createOrUpdate(SelectorDTO selectorDTO) {
/**
* Find by plugin id list.
*
- * @param pluginId the plugin id
+ * @param pluginId the plugin id
+ * @param namespaceId the namespaceId
* @return the list
*/
- List<SelectorData> findByPluginId(String pluginId);
+ List<SelectorData> findByPluginId(String pluginId, String namespaceId);
Review Comment:
please modify the same question.
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/publish/NamespacePluginEventPublisher.java:
##########
@@ -77,9 +80,21 @@ public void onDeleted(final NamespacePluginVO
namespacePluginVO) {
@Override
public void onDeleted(final Collection<NamespacePluginVO> namespacePlugin)
{
- publish(new BatchNamespacePluginChangedEvent(namespacePlugin, null,
EventTypeEnum.PLUGIN_UPDATE, SessionUtil.visitorName()));
- publisher.publishEvent(new DataChangedEvent(ConfigGroupEnum.PLUGIN,
DataEventTypeEnum.UPDATE,
+ String namespaceId = ((Collection<?>) namespacePlugin)
+ .stream()
+ .map(NamespacePluginVO.class::cast)
+ .findFirst()
+ .map(NamespacePluginVO::getNamespaceId)
+ .orElse("");
Review Comment:
why orElse("")???
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryUpstreamServiceImpl.java:
##########
@@ -154,7 +154,7 @@ public List<DiscoverySyncData> listAll() {
if (StringUtils.hasLength(discoveryRelDO.getSelectorId())) {
String selectorId = discoveryRelDO.getSelectorId();
discoverySyncData.setSelectorId(selectorId);
- SelectorDO selectorDO = selectorMapper.selectById(selectorId);
+ SelectorDO selectorDO =
selectorMapper.selectByOnlyId(selectorId);
Review Comment:
not change
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/UpstreamCheckService.java:
##########
@@ -437,7 +439,8 @@ public void fetchUpstreamData() {
}
Map<String, String> pluginMap =
pluginDOList.stream().filter(Objects::nonNull)
.collect(Collectors.toMap(PluginDO::getId, PluginDO::getName,
(value1, value2) -> value1));
- final List<SelectorDO> selectorDOList =
selectorMapper.findByPluginIds(new ArrayList<>(pluginMap.keySet()));
+ // todo:[To be refactored with namespace] Temporarily hardcode
Review Comment:
if complete, please remove todo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
