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-unsubscr...@shenyu.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
