Copilot commented on code in PR #1325:
URL: https://github.com/apache/dubbo-admin/pull/1325#discussion_r2365899998
##########
pkg/core/store/index/service_consumer_metadata.go:
##########
@@ -15,11 +15,27 @@
* limitations under the License.
*/
-package v1alpha1
+package index
-func (x *Zone) IsEnabled() bool {
- if x.Enabled == nil {
- return true
+import (
+ meshresource
"github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1"
+ "k8s.io/client-go/tools/cache"
+)
+
+const (
+ ByServiceConsumerAppName = "idx_service_consumer_app_name"
+)
+
+func init() {
+ RegisterIndexers(meshresource.ServiceConsumerMetadataKind,
map[string]cache.IndexFunc{
+ ByServiceProviderAppName: byServiceConsumerAppName,
Review Comment:
The index constant name doesn't match the function being registered. Should
use `ByServiceConsumerAppName` instead of `ByServiceProviderAppName` for
consistency.
```suggestion
ByServiceConsumerAppName: byServiceConsumerAppName,
```
##########
pkg/console/service/affinity_rule.go:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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 service
+
+import (
+ consolectx "github.com/apache/dubbo-admin/pkg/console/context"
+ "github.com/apache/dubbo-admin/pkg/core/logger"
+ "github.com/apache/dubbo-admin/pkg/core/manager"
+ meshresource
"github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1"
+ coremodel "github.com/apache/dubbo-admin/pkg/core/resource/model"
+)
+
+func GetAffinityRule(ctx consolectx.Context, name string, mesh string)
(*meshresource.AffinityRouteResource, error) {
+ res, _, err := manager.GetByKey[*meshresource.AffinityRouteResource](
+ ctx.ResourceManager(),
+ meshresource.AffinityRouteKind,
+ coremodel.BuildResourceKey(mesh, name))
+ if err != nil {
+ logger.Warnf("get affinity rule %s error: %v", name, err)
+ return nil, err
+ }
+ return res, nil
+}
+
+func UpdateAffinityRule(ctx consolectx.Context, res
*meshresource.AffinityRouteResource) error {
+ if err := ctx.ResourceManager().Update(res); err != nil {
+ logger.Warnf("update %s affinity rule failed with error: %s",
res.Name, err.Error())
+ return err
+ }
+ return nil
+}
+
+func CreateAffinityRule(ctx consolectx.Context, res
*meshresource.AffinityRouteResource) error {
+ if err := ctx.ResourceManager().Add(res); err != nil {
+ logger.Warnf("create %s condition failed with error: %s",
res.Name, err.Error())
+ return err
+ }
+ return nil
+}
+
+func DeleteAffinityRule(ctx consolectx.Context, name string, mesh string)
error {
+ if err := ctx.ResourceManager().DeleteByKey(
+ meshresource.ConditionRouteKind,
Review Comment:
Wrong resource kind used for deleting affinity rules. Should use
`meshresource.AffinityRouteKind` instead of `meshresource.ConditionRouteKind`.
```suggestion
meshresource.AffinityRouteKind,
```
##########
pkg/console/handler/condition_rule.go:
##########
@@ -54,13 +55,14 @@ func GetConditionRuleWithRuleName(cs consolectx.Context)
gin.HandlerFunc {
return func(c *gin.Context) {
var name string
ruleName := c.Param("ruleName")
+ mesh := c.Param("mesh")
if strings.HasSuffix(ruleName, consts.ConditionRuleSuffix) {
name =
ruleName[:len(ruleName)-len(consts.ConditionRuleSuffix)]
} else {
c.JSON(http.StatusBadRequest,
model.NewErrorResp(fmt.Sprintf("ruleName must end with %s",
consts.ConditionRuleSuffix)))
return
}
- if res, err := service.GetConditionRule(cs, name); err != nil {
+ if res, err := service.GetConditionRule(cs, mesh, name); err !=
nil {
Review Comment:
Parameter order is incorrect. Based on the function signature in the service
layer, it should be `service.GetConditionRule(cs, name, mesh)` not
`service.GetConditionRule(cs, mesh, name)`.
```suggestion
if res, err := service.GetConditionRule(cs, name, mesh); err !=
nil {
```
##########
pkg/core/store/index/instance.go:
##########
@@ -0,0 +1,58 @@
+/*
+ * 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 index
+
+import (
+ "github.com/apache/dubbo-admin/pkg/common/errors"
+ meshresource
"github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1"
+ "k8s.io/client-go/tools/cache"
+)
+
+const (
+ ByInstanceAppNameIndex = "idx_instance_app_name"
+ ByInstanceIpIndex = "idx_instance_ip"
+)
+
+func init() {
+ RegisterIndexers(meshresource.InstanceKind, map[string]cache.IndexFunc{
+ ByInstanceAppNameIndex: byInstanceAppName,
+ ByInstanceIpIndex: byIp,
+ })
+}
+
+func byInstanceAppName(obj interface{}) ([]string, error) {
+ instance, ok := obj.(meshresource.InstanceResource)
+ if !ok {
+ return nil,
errors.NewAssertionError(string(meshresource.InstanceKind),
string(instance.ResourceKind()))
Review Comment:
Type assertion error will always fail because `instance` is declared but not
properly initialized when the type assertion fails. Should pass the actual
object type instead of accessing `instance.ResourceKind()` on a zero value.
##########
pkg/console/handler/configurator_rule.go:
##########
@@ -40,45 +42,61 @@ func ConfiguratorSearch(ctx consolectx.Context)
gin.HandlerFunc {
c.JSON(http.StatusBadRequest,
model.NewErrorResp(err.Error()))
return
}
- ruleList := &mesh.DynamicConfigResourceList{}
- var respList []model.ConfiguratorSearchResp
- if req.Keywords == "" {
- if err := ctx.ResourceManager().List(ctx.AppContext(),
ruleList, store.ListByPage(req.PageSize, strconv.Itoa(req.PageOffset))); err !=
nil {
- c.JSON(http.StatusBadRequest,
model.NewErrorResp(err.Error()))
- return
- }
+ var pageData
*coremodel.PageData[*meshresource.DynamicConfigResource]
+ var err error
+ if strutil.IsBlank(req.Keywords) {
+ pageData, err =
manager.PageListByIndexes[*meshresource.DynamicConfigResource](
+ ctx.ResourceManager(),
+ meshresource.DynamicConfigKind,
+ map[string]interface{}{
+ index.ByMeshIndex: req.Mesh,
+ },
+ req.PageReq,
+ )
+
} else {
- if err := ctx.ResourceManager().List(ctx.AppContext(),
ruleList, store.ListByNameContains(req.Keywords),
store.ListByPage(req.PageSize, strconv.Itoa(req.PageOffset))); err != nil {
- c.JSON(http.StatusBadRequest,
model.NewErrorResp(err.Error()))
- return
- }
+ pageData, err =
manager.PageSearchResourceByConditions[*meshresource.DynamicConfigResource](
+ ctx.ResourceManager(),
+ meshresource.DynamicConfigKind,
+ []string{"name=" + req.Keywords},
+ req.PageReq,
+ )
+
+ }
+ if err != nil {
+ c.JSON(http.StatusInternalServerError,
model.NewErrorResp(err.Error()))
Review Comment:
Missing return statement after error response. The function will continue
executing and potentially send another response, causing an error.
```suggestion
c.JSON(http.StatusInternalServerError,
model.NewErrorResp(err.Error()))
return
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
