Skip to content

Commit 3e29abd

Browse files
committed
fix(versioning): close gaps surfaced by the smoke drill
Running the §9.4 smoke drill end-to-end uncovered three real defects that the unit suite did not catch: - RuleVersionSubscriber recorded a duplicate UPSTREAM row whenever the registry echoed back a no-op change identical to the latest ledger row (typically right after BOOTSTRAP). Now dedupes upstream events whose content hash already matches the current head, with an explicit test in versioning_test.go. - writeVersioningResp mapped every bizerror to HTTP 200/UnknownError; bizerror.InvalidArgument (eg. empty rollback reason) now returns HTTP 400 with its original code so the frontend can act on it. Covered by a new handler/rule_version_test.go. - The 409 VERSION_CONFLICT toast auto-dismissed after the default duration; users could miss the Reload button entirely. Pinned with duration: 0 so the notification stays until acknowledged.
1 parent e1edfa0 commit 3e29abd

5 files changed

Lines changed: 93 additions & 0 deletions

File tree

pkg/console/handler/rule_version.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ func writeVersioningResp(c *gin.Context, data any, err error) {
151151
return
152152
}
153153
var conflict *versioning.ConflictError
154+
var bizErr bizerror.Error
154155
switch {
155156
case errors.As(err, &conflict):
156157
c.JSON(http.StatusConflict, gin.H{
@@ -164,6 +165,8 @@ func writeVersioningResp(c *gin.Context, data any, err error) {
164165
c.JSON(http.StatusOK, model.NewBizErrorResp(bizerror.New(bizerror.NotFoundError, err.Error())))
165166
case errors.Is(err, versioning.ErrRollbackToDelete):
166167
c.JSON(http.StatusOK, model.NewBizErrorResp(bizerror.New(bizerror.InvalidArgument, err.Error())))
168+
case errors.As(err, &bizErr) && bizErr.Code() == bizerror.InvalidArgument:
169+
c.JSON(http.StatusBadRequest, model.NewBizErrorResp(bizErr))
167170
default:
168171
c.JSON(http.StatusOK, model.NewBizErrorResp(bizerror.New(bizerror.UnknownError, err.Error())))
169172
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package handler
19+
20+
import (
21+
"net/http"
22+
"net/http/httptest"
23+
"testing"
24+
25+
"github.com/gin-gonic/gin"
26+
"github.com/stretchr/testify/require"
27+
28+
"github.com/apache/dubbo-admin/pkg/common/bizerror"
29+
)
30+
31+
func TestWriteVersioningRespMapsInvalidArgumentToBadRequest(t *testing.T) {
32+
gin.SetMode(gin.TestMode)
33+
recorder := httptest.NewRecorder()
34+
c, _ := gin.CreateTestContext(recorder)
35+
36+
writeVersioningResp(c, nil, bizerror.New(bizerror.InvalidArgument, "rollback reason is required"))
37+
38+
require.Equal(t, http.StatusBadRequest, recorder.Code)
39+
require.JSONEq(t, `{"code":"InvalidArgument","message":"rollback reason is required","data":null}`, recorder.Body.String())
40+
}

pkg/core/versioning/subscriber.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,14 @@ func (s *Subscriber) record(event events.Event) error {
160160
author := "system:upstream"
161161
reason := ""
162162
var rolledBackFromID *int64
163+
hinted := false
163164
if ctx := event.Context(); ctx != nil {
164165
if registry := ctx[sourceRegistryContextKey]; registry != "" {
165166
author = "system:" + registry
166167
}
167168
}
168169
if hint, ok := s.hints.Take(res.ResourceKind(), res.ResourceKey(), hash); ok {
170+
hinted = true
169171
source = hint.Source
170172
if source == "" {
171173
source = SourceAdmin
@@ -194,6 +196,15 @@ func (s *Subscriber) record(event events.Event) error {
194196
if author == "" {
195197
author = "system:unknown"
196198
}
199+
if !hinted && source == SourceUpstream && op != OperationDelete {
200+
latest, err := s.store.LatestVersion(ruleKind, resourceKey)
201+
if err != nil {
202+
return err
203+
}
204+
if latest != nil && latest.ContentHash == hash {
205+
return nil
206+
}
207+
}
197208
_, err = s.store.InsertVersion(InsertRequest{
198209
RuleKind: ruleKind,
199210
Mesh: mesh,

pkg/core/versioning/versioning_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,44 @@ func TestSubscriberRespectsRegistrySourceContext(t *testing.T) {
229229
require.Equal(t, "system:zookeeper", items[0].Author)
230230
}
231231

232+
func TestSubscriberSkipsNoopUpstreamEchoAfterBootstrap(t *testing.T) {
233+
store := NewMemoryStore()
234+
hints := NewAdminHintRegistry()
235+
sub := NewSubscriber(meshresource.ConditionRouteKind, store, hints, 5, 0)
236+
237+
original := meshresource.NewConditionRouteResourceWithAttributes("demo.condition-router", "mesh")
238+
original.Spec = &meshproto.ConditionRoute{Key: "demo", Priority: 1}
239+
require.NoError(t, RecordBootstrap(store, 5, original))
240+
require.NoError(t, sub.ProcessEvent(events.NewResourceChangedEventWithContext(
241+
cache.Updated,
242+
nil,
243+
original,
244+
map[string]string{"source-registry": "zookeeper"},
245+
)))
246+
247+
items, err := store.ListVersions(meshresource.ConditionRouteKind, original.ResourceKey())
248+
require.NoError(t, err)
249+
require.Len(t, items, 1)
250+
require.Equal(t, SourceBootstrap, items[0].Source)
251+
require.True(t, items[0].IsCurrent)
252+
253+
changed := meshresource.NewConditionRouteResourceWithAttributes("demo.condition-router", "mesh")
254+
changed.Spec = &meshproto.ConditionRoute{Key: "demo", Priority: 2}
255+
require.NoError(t, sub.ProcessEvent(events.NewResourceChangedEventWithContext(
256+
cache.Updated,
257+
original,
258+
changed,
259+
map[string]string{"source-registry": "zookeeper"},
260+
)))
261+
262+
items, err = store.ListVersions(meshresource.ConditionRouteKind, original.ResourceKey())
263+
require.NoError(t, err)
264+
require.Len(t, items, 2)
265+
require.Equal(t, SourceUpstream, items[0].Source)
266+
require.Equal(t, "system:zookeeper", items[0].Author)
267+
require.True(t, items[0].IsCurrent)
268+
}
269+
232270
func TestSubscriberRecordsDeleteWithAdminHintSnapshot(t *testing.T) {
233271
store := NewMemoryStore()
234272
hints := NewAdminHintRegistry()

ui-vue3/src/views/traffic/_shared/ruleVersion.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export const notifyVersionConflict = (
7777
if (isVersionConflict(e)) {
7878
notification.warning({
7979
key: 'rule-version-conflict',
80+
duration: 0,
8081
message: '版本冲突',
8182
description: '规则已被其他操作更新,请重新加载当前版本后再提交。',
8283
btn: options?.reload

0 commit comments

Comments
 (0)