AsperforMias commented on code in PR #1554: URL: https://github.com/apache/dubbo-admin/pull/1554#discussion_r4245476358
########## pkg/console/model/router_rule.go: ########## @@ -0,0 +1,99 @@ +/* + * 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 model + +import meshproto "github.com/apache/dubbo-admin/api/mesh/v1alpha1" + +// RouterRuleSearchResp is intentionally body-free; script text is returned +// only by the detail endpoint. +type RouterRuleSearchResp struct { + CreateTime string `json:"createTime"` + Enabled bool `json:"enabled"` + RuleName string `json:"ruleName"` + Scope string `json:"scope"` +} + +// AffinityRuleInput is the public Console representation. affinityAware is +// mapped to the internal protobuf field named affinity. +type AffinityRuleInput struct { + ConfigVersion string `json:"configVersion"` + Scope string `json:"scope"` + Key string `json:"key"` + Runtime bool `json:"runtime"` + Enabled bool `json:"enabled"` + AffinityAware *meshproto.AffinityAware `json:"affinityAware"` +} + +func (i *AffinityRuleInput) ToProto() *meshproto.AffinityRoute { + return &meshproto.AffinityRoute{ + ConfigVersion: i.ConfigVersion, + Scope: i.Scope, + Key: i.Key, + Runtime: i.Runtime, + Enabled: i.Enabled, + Affinity: i.AffinityAware, + } +} + +func GenAffinityRuleResp(data *meshproto.AffinityRoute) *CommonResp { + if data == nil { + return NewSuccessResp(nil) + } + return NewSuccessResp(AffinityRuleInput{ + ConfigVersion: data.ConfigVersion, + Scope: data.Scope, + Key: data.Key, + Runtime: data.Runtime, + Enabled: data.Enabled, + AffinityAware: data.Affinity, + }) +} + +type ScriptRuleInput struct { + ConfigVersion string `json:"configVersion"` + Scope string `json:"scope"` + Key string `json:"key"` + Enabled bool `json:"enabled"` + Type string `json:"type"` + Script string `json:"script"` +} + +func (i *ScriptRuleInput) ToProto() *meshproto.ScriptRoute { + return &meshproto.ScriptRoute{ + ConfigVersion: i.ConfigVersion, + Scope: i.Scope, + Key: i.Key, + Enabled: i.Enabled, + Type: i.Type, + Script: i.Script, + } +} + +func GenScriptRuleResp(data *meshproto.ScriptRoute) *CommonResp { + if data == nil { + return NewSuccessResp(nil) + } + return NewSuccessResp(ScriptRuleInput{ + ConfigVersion: data.ConfigVersion, + Scope: data.Scope, + Key: data.Key, + Enabled: data.Enabled, Review Comment: [P1] Preserve enabled-field presence through the detail/edit API With the latest codec from #1552 applied, a Script YAML document that omits `enabled` remains default-enabled in Dubbo-Go, while its internal scalar `Spec.Enabled` is false and an annotation records the omission. This response receives only the Spec and emits `enabled: false`. I reproduced `DecodeRule -> GenScriptRuleResp -> JSON -> ScriptRuleInput.ToProto -> EncodeRule`: sending the unchanged detail data back through the update path writes `enabled: false`, because the handler constructs a new resource without the presence annotation. In the current Go router, the same rule changed from selecting only provider 20881 to returning both 20880 and 20881, removing its routing constraint. Please preserve absent versus explicit-false values across both the DTO and mutation path, and add a read/edit/save regression test. The Affinity enabled/runtime fields need the same presence treatment, preserving each runtime's existing defaults. ########## pkg/core/versioning/normalize.go: ########## @@ -115,6 +115,22 @@ func ResourceFromSpecJSON(kind coremodel.ResourceKind, mesh, ruleName, specJSON } res.Spec = &spec return res, nil + case meshresource.AffinityRouteKind: + res := meshresource.NewAffinityRouteResourceWithAttributes(ruleName, mesh) + var spec meshproto.AffinityRoute + if err := unmarshalSpec(specJSON, &spec); err != nil { + return nil, err + } + res.Spec = &spec + return res, nil + case meshresource.ScriptRouteKind: + res := meshresource.NewScriptRouteResourceWithAttributes(ruleName, mesh) + var spec meshproto.ScriptRoute + if err := unmarshalSpec(specJSON, &spec); err != nil { + return nil, err + } + res.Spec = &spec Review Comment: [P1] Preserve omitted rule flags in history snapshots and rollback The latest #1552 codec keeps omitted `enabled`/`runtime` in resource annotations, but `NormalizeResource` still serializes only the scalar protobuf Spec. Those annotations never reach the snapshot, and this reconstruction creates a resource without them. I reproduced the history round trip with the latest #1552 codec applied to this branch: a default-enabled Script rule with no `enabled` key becomes `enabled: false` after `NormalizeResource -> ResourceFromSpecJSON -> EncodeRule`. Rolling back an earlier routing policy therefore disables it instead of restoring it. Omitted enabled and explicit false also produce identical hashes, so the rollback-to-current check can reject a rollback even though runtime behavior differs. Please preserve field presence in the snapshot/hash and restore it when rebuilding the resource. Add regressions for omitted versus explicit-false flags, distinct hashes for different runtime behavior, and rollback retaining the original routing result. Affinity runtime/enabled presence needs the same coverage. -- 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]
