You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shenyu.apache.org by "xcsnx (via GitHub)" <gi...@apache.org> on 2024/08/01 16:29:32 UTC
[PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
xcsnx opened a new pull request, #5604:
URL: https://github.com/apache/shenyu/pull/5604
<!-- Describe your PR here; e.g. Fixes #issueNo -->
<!--
Thank you for proposing a pull request. This template will guide you through the essential steps necessary for a pull request.
-->
Make sure that:
- [ ] You have read the [contribution guidelines](https://shenyu.apache.org/community/contributor-guide).
- [ ] You submit test cases (unit or integration tests) that back your changes.
- [ ] Your local test passed `./mvnw clean install -Dmaven.javadoc.skip=true`.
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "xcsnx (via GitHub)" <gi...@apache.org>.
xcsnx commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1711584873
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/SelectorController.java:
##########
@@ -111,10 +117,9 @@ public ShenyuAdminResult createSelector(@Valid @RequestBody final SelectorDTO se
* @param selectorDTO selector.
* @return {@linkplain ShenyuAdminResult}
*/
- @PutMapping("/{id}")
Review Comment:
ok
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "xcsnx (via GitHub)" <gi...@apache.org>.
xcsnx commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1712559198
##########
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:
ok
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "moremind (via GitHub)" <gi...@apache.org>.
moremind commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1703278101
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SelectorServiceImpl.java:
##########
@@ -173,10 +175,12 @@ public String registerDefault(final SelectorDTO selectorDTO) {
@Override
public String registerDefault(final MetaDataRegisterDTO dto, final String pluginName, final String selectorHandler) {
String contextPath = ContextPathUtils.buildContextPath(dto.getContextPath(), dto.getAppName());
- SelectorDO selectorDO = findByNameAndPluginName(contextPath, pluginName);
+ // todo:[To be refactored with namespace] Temporarily hardcode
+ SelectorDO selectorDO = findByNameAndPluginName(contextPath, pluginName, SYS_DEFAULT_NAMESPACE_NAMESPACE_ID);
if (Objects.isNull(selectorDO)) {
SelectorDTO selectorDTO = SelectorUtil.buildSelectorDTO(contextPath, pluginMapper.selectByName(pluginName).getId());
selectorDTO.setHandle(selectorHandler);
+ selectorDTO.setNamespaceId(SYS_DEFAULT_NAMESPACE_NAMESPACE_ID);
Review Comment:
shenyu client must add shenyu namespace id, you should get the namespace from shenyu-client, if it's null, you should use the default.
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "moremind (via GitHub)" <gi...@apache.org>.
moremind commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1711447339
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/SelectorController.java:
##########
@@ -111,10 +117,9 @@ public ShenyuAdminResult createSelector(@Valid @RequestBody final SelectorDTO se
* @param selectorDTO selector.
* @return {@linkplain ShenyuAdminResult}
*/
- @PutMapping("/{id}")
Review Comment:
not delete `/`
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "xcsnx (via GitHub)" <gi...@apache.org>.
xcsnx commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1712563058
##########
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:
pluginId
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "moremind (via GitHub)" <gi...@apache.org>.
moremind merged PR #5604:
URL: https://github.com/apache/shenyu/pull/5604
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "xcsnx (via GitHub)" <gi...@apache.org>.
xcsnx commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1704306428
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/model/entity/PluginDO.java:
##########
@@ -222,6 +223,37 @@ public static PluginDO buildPluginDO(final PluginDTO pluginDTO) {
}).orElse(null);
}
+ /**
Review Comment:
buildPluginDO
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "xcsnx (via GitHub)" <gi...@apache.org>.
xcsnx commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1712560710
##########
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:
not complete
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "moremind (via GitHub)" <gi...@apache.org>.
moremind commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1719578230
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/SelectorMapper.java:
##########
@@ -199,7 +214,7 @@ public interface SelectorMapper extends ExistProvider {
/**
* update status.
*
- * @param id id
+ * @param id id
Review Comment:
not modify the style
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/SelectorService.java:
##########
@@ -105,57 +105,63 @@ default int createOrUpdate(SelectorDTO selectorDTO) {
int updateSelective(SelectorDO selectorDO);
/**
- * delete selectors.
+ * delete selectors by ids and namespaceId.
*
- * @param ids primary key.
+ * @param ids primary key.
+ * @param namespaceId namespaceId.
* @return rows int
*/
- int delete(List<String> ids);
+ int deleteByNamespaceId(List<String> ids, String namespaceId);
/**
- * find selector by id.
+ * find selector by id and namespaceId.
*
- * @param id primary key.
+ * @param id primary key.
+ * @param namespaceId namespaceId.
* @return {@linkplain SelectorVO}
*/
- SelectorVO findById(String id);
+ SelectorVO findByIdAndNamespaceId(String id, String namespaceId);
/**
- * find selector by name.
+ * find selector by name and namespaceId.
*
- * @param name the name
+ * @param name the name
+ * @param namespaceId namespaceId.
+ * Therefore, it is recommended to: {@linkplain SelectorService#findListByNameAndNamespaceId(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 findByNameAndNamespaceId(String name, String namespaceId);
/**
- * find selector list by name.
+ * find selector list by name and namespaceId.
*
- * @param name name
+ * @param name name
+ * @param namespaceId namespaceId.
* @return list
*/
- List<SelectorDO> findListByName(String name);
+ List<SelectorDO> findListByNameAndNamespaceId(String name, String namespaceId);
/**
- * Find by name and plugin id selector do.
+ * Find selector do by name and pluginName and namespaceId.
*
- * @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 findByNameAndPluginNameAndNamespaceId(String name, String pluginName, String namespaceId);
/**
- * findByNameAndPluginNameForUpdate.
+ * findByNameAndPluginNameAndNamespaceIdForUpdate.
Review Comment:
use normal comment. `find selector entity by selector name, plugin name and namespace id`
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/SelectorMapper.java:
##########
@@ -69,37 +78,43 @@ public interface SelectorMapper extends ExistProvider {
List<SelectorDO> selectByQuery(SelectorQuery selectorQuery);
/**
- * Find by plugin id list.
+ * Find list by plugin id and namespaceId.
*
- * @param pluginId the plugin id
+ * @param pluginId the plugin id
+ * @param namespaceId namespaceId.
* @return the list
*/
- List<SelectorDO> findByPluginId(String pluginId);
+ List<SelectorDO> findByPluginIdAndNamespaceId(String pluginId, String namespaceId);
/**
- * Find by plugin id list.
+ * Find list by plugin id and namespaceId.
*
- * @param pluginIds the plugin ids
+ * @param pluginIds the plugin ids
+ * @param namespaceId namespaceId.
* @return the list
*/
- List<SelectorDO> findByPluginIds(List<String> pluginIds);
+ List<SelectorDO> findByPluginIdsAndNamespaceId(@Param("list") List<String> pluginIds, String namespaceId);
/**
- * select select by name.
+ * select list by name and namespaceId.
*
- * @param name the name
+ * @param name the name
+ * @param namespaceId namespaceId.
* @return selector do list
*/
- List<SelectorDO> selectByName(String name);
+ List<SelectorDO> selectByNameAndNamespaceId(String name, String namespaceId);
/**
- * Find by name and plugin id selector do.
+ * Find selector do by name and plugin id and namespaceId.
Review Comment:
selector entity
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "moremind (via GitHub)" <gi...@apache.org>.
moremind commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1703273978
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/SelectorController.java:
##########
@@ -111,10 +117,9 @@ public ShenyuAdminResult createSelector(@Valid @RequestBody final SelectorDTO se
* @param selectorDTO selector.
* @return {@linkplain ShenyuAdminResult}
*/
- @PutMapping("/{id}")
+ @PutMapping("/id={id}")
Review Comment:
same question
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/SelectorMapper.java:
##########
@@ -71,35 +80,39 @@ public interface SelectorMapper extends ExistProvider {
/**
* Find by plugin id list.
*
- * @param pluginId the plugin id
+ * @param pluginId the plugin id
+ * @param namespaceId namespaceId.
* @return the list
*/
- List<SelectorDO> findByPluginId(String pluginId);
+ List<SelectorDO> findByPluginId(String pluginId, String namespaceId);
/**
* Find by plugin id list.
*
- * @param pluginIds the plugin ids
+ * @param pluginIds the plugin ids
+ * @param namespaceId namespaceId.
* @return the list
*/
- List<SelectorDO> findByPluginIds(List<String> pluginIds);
+ List<SelectorDO> findByPluginIds(@Param("list") List<String> pluginIds, String namespaceId);
/**
* select select by name.
*
- * @param name the name
+ * @param name the name
+ * @param namespaceId namespaceId.
* @return selector do list
*/
- List<SelectorDO> selectByName(String name);
+ List<SelectorDO> selectByName(String name, String namespaceId);
Review Comment:
same questions
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/model/dto/SelectorDTO.java:
##########
@@ -386,36 +414,30 @@ public void setSelectorRules(final List<RuleDTO> selectorRules) {
public static SelectorDTO.SelectorDTOBuilder builder() {
return new SelectorDTO.SelectorDTOBuilder();
}
-
+
@Override
public boolean equals(final Object o) {
if (this == o) {
return true;
}
- if (!(o instanceof SelectorDTO)) {
+ if (o == null || getClass() != o.getClass()) {
return false;
}
SelectorDTO that = (SelectorDTO) o;
- return Objects.equals(id, that.id)
- && Objects.equals(pluginId, that.pluginId)
- && Objects.equals(name, that.name)
- && Objects.equals(matchMode, that.matchMode)
- && Objects.equals(type, that.type)
- && Objects.equals(sort, that.sort)
- && Objects.equals(enabled, that.enabled)
- && Objects.equals(loged, that.loged)
- && Objects.equals(continued, that.continued)
- && Objects.equals(handle, that.handle)
- && Objects.equals(selectorConditions, that.selectorConditions)
- && Objects.equals(matchRestful, that.matchRestful);
+ return Objects.equals(id, that.id) && Objects.equals(pluginId, that.pluginId) && Objects.equals(name, that.name)
+ && Objects.equals(matchMode, that.matchMode) && Objects.equals(type, that.type) && Objects.equals(sort, that.sort)
+ && Objects.equals(enabled, that.enabled) && Objects.equals(loged, that.loged) && Objects.equals(continued, that.continued)
+ && Objects.equals(handle, that.handle) && Objects.equals(selectorConditions, that.selectorConditions)
+ && Objects.equals(matchRestful, that.matchRestful) && Objects.equals(selectorRules, that.selectorRules)
+ && Objects.equals(namespaceId, that.namespaceId);
Review Comment:
not change the style
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ApiServiceImpl.java:
##########
@@ -186,7 +188,7 @@ private void removeRegister(final ApiDO apiDO) {
}
});
if (CollectionUtils.isNotEmpty(selectorIds)) {
- selectorService.delete(selectorIds);
+ selectorService.delete(selectorIds, SYS_DEFAULT_NAMESPACE_NAMESPACE_ID);
Review Comment:
need judge logic?
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/SelectorController.java:
##########
@@ -66,29 +64,37 @@ public SelectorController(final SelectorService selectorService) {
* @param name selector name.
* @param currentPage current page.
* @param pageSize page size.
+ * @param namespaceId namespaceId.
* @return {@linkplain ShenyuAdminResult}
*/
@GetMapping("")
public AdminResult<CommonPager<SelectorVO>> querySelectors(final String pluginId, final String name,
- @RequestParam @NotNull final Integer currentPage,
- @RequestParam @NotNull final Integer pageSize) {
+ @RequestParam @NotNull final Integer currentPage,
+ @RequestParam @NotNull final Integer pageSize,
+ @Valid @Existed(message = "namespaceId is not existed",
+ provider = NamespaceMapper.class) final String namespaceId
+ ) {
final SelectorQueryCondition condition = new SelectorQueryCondition();
condition.setUserId(SessionUtil.visitor().getUserId());
condition.setPlugin(ListUtil.of(pluginId));
condition.setKeyword(name);
+ condition.setNamespaceId(namespaceId);
return searchAdaptor(new PageCondition<>(currentPage, pageSize, condition));
}
/**
* detail selector.
*
* @param id selector id.
+ * @param namespaceId namespaceId.
* @return {@linkplain ShenyuAdminResult}
*/
- @GetMapping("/{id}")
+ @GetMapping("/id={id}&namespaceId={namespaceId}")
public ShenyuAdminResult detailSelector(@PathVariable("id") @Valid
Review Comment:
this is path variable not path param, the url shoud be `@GetMapping("{id}/{namespaceId}"`
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/NamespacePluginRelMapper.java:
##########
@@ -65,11 +65,11 @@ 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 selectById(String pluginId, String namespaceId);
Review Comment:
why pluginId?
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/model/dto/SelectorDTO.java:
##########
@@ -386,36 +414,30 @@ public void setSelectorRules(final List<RuleDTO> selectorRules) {
public static SelectorDTO.SelectorDTOBuilder builder() {
return new SelectorDTO.SelectorDTOBuilder();
}
-
+
@Override
public boolean equals(final Object o) {
if (this == o) {
return true;
}
- if (!(o instanceof SelectorDTO)) {
+ if (o == null || getClass() != o.getClass()) {
return false;
}
SelectorDTO that = (SelectorDTO) o;
- return Objects.equals(id, that.id)
- && Objects.equals(pluginId, that.pluginId)
- && Objects.equals(name, that.name)
- && Objects.equals(matchMode, that.matchMode)
- && Objects.equals(type, that.type)
- && Objects.equals(sort, that.sort)
- && Objects.equals(enabled, that.enabled)
- && Objects.equals(loged, that.loged)
- && Objects.equals(continued, that.continued)
- && Objects.equals(handle, that.handle)
- && Objects.equals(selectorConditions, that.selectorConditions)
- && Objects.equals(matchRestful, that.matchRestful);
+ return Objects.equals(id, that.id) && Objects.equals(pluginId, that.pluginId) && Objects.equals(name, that.name)
+ && Objects.equals(matchMode, that.matchMode) && Objects.equals(type, that.type) && Objects.equals(sort, that.sort)
+ && Objects.equals(enabled, that.enabled) && Objects.equals(loged, that.loged) && Objects.equals(continued, that.continued)
+ && Objects.equals(handle, that.handle) && Objects.equals(selectorConditions, that.selectorConditions)
+ && Objects.equals(matchRestful, that.matchRestful) && Objects.equals(selectorRules, that.selectorRules)
+ && Objects.equals(namespaceId, that.namespaceId);
}
-
+
@Override
public int hashCode() {
- return Objects.hash(id, pluginId, name, matchMode, type, sort, enabled, loged, continued, handle,
- selectorConditions, matchRestful);
+ return Objects.hash(id, pluginId, name, matchMode, type, sort, enabled, loged, continued, handle, selectorConditions,
+ matchRestful, selectorRules, namespaceId);
Review Comment:
not change the style
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/SelectorMapper.java:
##########
@@ -71,35 +80,39 @@ public interface SelectorMapper extends ExistProvider {
/**
* Find by plugin id list.
*
- * @param pluginId the plugin id
+ * @param pluginId the plugin id
+ * @param namespaceId namespaceId.
* @return the list
*/
- List<SelectorDO> findByPluginId(String pluginId);
+ List<SelectorDO> findByPluginId(String pluginId, String namespaceId);
/**
* Find by plugin id list.
*
- * @param pluginIds the plugin ids
+ * @param pluginIds the plugin ids
+ * @param namespaceId namespaceId.
* @return the list
*/
- List<SelectorDO> findByPluginIds(List<String> pluginIds);
+ List<SelectorDO> findByPluginIds(@Param("list") List<String> pluginIds, String namespaceId);
Review Comment:
add new method
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/model/entity/SelectorDO.java:
##########
@@ -314,21 +340,15 @@ public boolean equals(final Object o) {
return false;
}
SelectorDO that = (SelectorDO) o;
- return Objects.equals(pluginId, that.pluginId)
- && Objects.equals(name, that.name)
- && Objects.equals(matchMode, that.matchMode)
- && Objects.equals(type, that.type)
- && Objects.equals(sort, that.sort)
- && Objects.equals(enabled, that.enabled)
- && Objects.equals(loged, that.loged)
- && Objects.equals(continued, that.continued)
- && Objects.equals(handle, that.handle)
- && Objects.equals(matchRestful, that.matchRestful);
+ return Objects.equals(pluginId, that.pluginId) && Objects.equals(name, that.name) && Objects.equals(matchMode, that.matchMode)
+ && Objects.equals(type, that.type) && Objects.equals(sort, that.sort) && Objects.equals(enabled, that.enabled)
Review Comment:
not change style
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/model/entity/PluginDO.java:
##########
@@ -222,6 +223,37 @@ public static PluginDO buildPluginDO(final PluginDTO pluginDTO) {
}).orElse(null);
}
+ /**
Review Comment:
shoud add buildPluginDO or buildNamespacePluginDO?
##########
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:
same questions for current service.
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/NamespacePluginRelMapper.java:
##########
@@ -65,11 +65,11 @@ 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 selectById(String pluginId, String namespaceId);
Review Comment:
if select by plugin id, shoud be selectByPluginIdAndNamespaceId(params...)
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "moremind (via GitHub)" <gi...@apache.org>.
moremind commented on code in PR #5604:
URL: https://github.com/apache/shenyu/pull/5604#discussion_r1717004954
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/NamespacePluginRelMapper.java:
##########
@@ -124,22 +123,22 @@ public interface NamespacePluginRelMapper extends ExistProvider {
* @param namespaceId namespace id.
* @return rows int
*/
- int deleteByIds(List<String> ids, String namespaceId);
+ int deleteByIdsAndNamespaceId(List<String> ids, String namespaceId);
/**
* select all.
Review Comment:
change the comment
##########
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.
Review Comment:
change the comment
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/SelectorMapper.java:
##########
@@ -44,6 +44,15 @@ public interface SelectorMapper extends ExistProvider {
@Override
Boolean existed(@Param("id") Serializable id);
+ /**
+ * select selector by id.
Review Comment:
change the comment
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/NamespacePluginRelMapper.java:
##########
@@ -124,22 +123,22 @@ public interface NamespacePluginRelMapper extends ExistProvider {
* @param namespaceId namespace id.
* @return rows int
*/
- int deleteByIds(List<String> ids, String namespaceId);
+ int deleteByIdsAndNamespaceId(List<String> ids, String namespaceId);
/**
* select all.
*
* @param namespaceId namespace id.
Review Comment:
same question
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/DiscoveryUpstreamMapper.java:
##########
@@ -38,15 +38,15 @@ public interface DiscoveryUpstreamMapper extends ExistProvider {
Boolean existed(@Param("id") Serializable key);
/**
- * selectByIds.
+ * selectByIdsAndNamespaceId.
Review Comment:
why change comment?
--
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [type:feat]Selector module adapts to namespace transformation [shenyu]
Posted by "moremind (via GitHub)" <gi...@apache.org>.
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-unsubscribe@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org