This is an automated email from the ASF dual-hosted git repository.
tianxiaoliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-service-center.git
The following commit(s) were added to refs/heads/master by this push:
new fbcd26c SCB-2176 Fix: POST tags should be covered (#828)
fbcd26c is described below
commit fbcd26c7ad739ae71b32373f9ffe051b4a3b137e
Author: little-cui <[email protected]>
AuthorDate: Mon Jan 18 10:25:48 2021 +0800
SCB-2176 Fix: POST tags should be covered (#828)
* SCB-2176 Fix: POST tags should be covered
* SCB-2176 Fix: UT failures
---
datasource/etcd/ms.go | 33 +-----------------------
datasource/etcd/ms_test.go | 39 ----------------------------
datasource/mongo/ms.go | 48 ++++++++---------------------------
datasource/mongo/ms_test.go | 38 ---------------------------
server/plugin/quota/buildin/common.go | 11 ++------
server/service/tag_test.go | 10 +-------
6 files changed, 14 insertions(+), 165 deletions(-)
diff --git a/datasource/etcd/ms.go b/datasource/etcd/ms.go
index ed9981a..b7e70a5 100644
--- a/datasource/etcd/ms.go
+++ b/datasource/etcd/ms.go
@@ -1638,38 +1638,7 @@ func (ds *DataSource) AddTags(ctx context.Context,
request *pb.AddServiceTagsReq
}, nil
}
- addTags := request.Tags
- res := quota.NewApplyQuotaResource(quota.TagQuotaType, domainProject,
request.ServiceId, int64(len(addTags)))
- rst := quota.Apply(ctx, res)
- errQuota := rst.Err
- if errQuota != nil {
- log.Errorf(errQuota, "add service[%s]'s tags %v failed,
operator: %s", request.ServiceId, addTags, remoteIP)
- response := &pb.AddServiceTagsResponse{
- Response: pb.CreateResponseWithSCErr(errQuota),
- }
- if errQuota.InternalError() {
- return response, errQuota
- }
- return response, nil
- }
-
- dataTags, err := serviceUtil.GetTagsUtils(ctx, domainProject,
request.ServiceId)
- if err != nil {
- log.Errorf(err, "add service[%s]'s tags %v failed, get existed
tag failed, operator: %s",
- request.ServiceId, addTags, remoteIP)
- return &pb.AddServiceTagsResponse{
- Response: pb.CreateResponse(pb.ErrInternal,
err.Error()),
- }, err
- }
- for key, value := range dataTags {
- if _, ok := addTags[key]; ok {
- continue
- }
- addTags[key] = value
- }
- dataTags = addTags
-
- checkErr := serviceUtil.AddTagIntoETCD(ctx, domainProject,
request.ServiceId, dataTags)
+ checkErr := serviceUtil.AddTagIntoETCD(ctx, domainProject,
request.ServiceId, request.Tags)
if checkErr != nil {
log.Errorf(checkErr, "add service[%s]'s tags %v failed,
operator: %s", request.ServiceId, request.Tags, remoteIP)
resp := &pb.AddServiceTagsResponse{
diff --git a/datasource/etcd/ms_test.go b/datasource/etcd/ms_test.go
index 9c3e059..8d8c7a6 100644
--- a/datasource/etcd/ms_test.go
+++ b/datasource/etcd/ms_test.go
@@ -3568,7 +3568,6 @@ func TestRule_Permission(t *testing.T) {
func TestTags_Add(t *testing.T) {
var (
serviceId1 string
- serviceId2 string
)
// create service
@@ -3586,20 +3585,6 @@ func TestTags_Add(t *testing.T) {
assert.NoError(t, err)
assert.NotEqual(t, "", resp.ServiceId)
serviceId1 = resp.ServiceId
-
- svc2 := &pb.MicroService{
- AppId: "create_tag_group_ms",
- ServiceName: "create_tag_service_ms",
- Version: "1.0.1",
- Level: "FRONT",
- Status: pb.MS_UP,
- }
- resp, err = datasource.Instance().RegisterService(getContext(),
&pb.CreateServiceRequest{
- Service: svc2,
- })
- assert.NoError(t, err)
- assert.NotNil(t, "", resp.ServiceId)
- serviceId2 = resp.ServiceId
})
t.Run("the request is invalid", func(t *testing.T) {
@@ -3629,30 +3614,6 @@ func TestTags_Add(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, pb.ResponseSuccess, resp.Response.GetCode())
})
-
- t.Run("tag's quota exceeded", func(t *testing.T) {
- log.Info("insufficient tag quota")
- size := quota.DefaultTagQuota / 2
- tags := make(map[string]string, size)
- for i := 0; i < size; i++ {
- s := "tag" + strconv.Itoa(i)
- tags[s] = s
- }
- resp, err := datasource.Instance().AddTags(getContext(),
&pb.AddServiceTagsRequest{
- ServiceId: serviceId2,
- Tags: tags,
- })
- assert.NoError(t, err)
- assert.Equal(t, pb.ResponseSuccess, resp.Response.GetCode())
-
- tags["out"] = "range"
- resp, _ = datasource.Instance().AddTags(getContext(),
&pb.AddServiceTagsRequest{
- ServiceId: serviceId2,
- Tags: tags,
- })
- assert.NoError(t, err)
- assert.Equal(t, pb.ErrNotEnoughQuota, resp.Response.GetCode())
- })
}
func TestTags_Get(t *testing.T) {
diff --git a/datasource/mongo/ms.go b/datasource/mongo/ms.go
index 16eb4ab..7152e78 100644
--- a/datasource/mongo/ms.go
+++ b/datasource/mongo/ms.go
@@ -483,48 +483,20 @@ func (ds *DataSource) GetServicesInfo(ctx
context.Context, request *discovery.Ge
}
func (ds *DataSource) AddTags(ctx context.Context, request
*discovery.AddServiceTagsRequest) (*discovery.AddServiceTagsResponse, error) {
- remoteIP := util.GetIPFromContext(ctx)
- service, err := GetService(ctx, GeneratorServiceFilter(ctx,
request.ServiceId))
- if err != nil {
- if errors.Is(err, datasource.ErrNoData) {
- log.Debug(fmt.Sprintf("service %s not exist in db",
request.ServiceId))
- return &discovery.AddServiceTagsResponse{Response:
discovery.CreateResponse(discovery.ErrServiceNotExists, "Service not exist")},
nil
- }
- log.Error(fmt.Sprintf("failed to add tags for service %s for
get service failed", request.ServiceId), err)
+ err := UpdateService(ctx, GeneratorServiceFilter(ctx,
request.ServiceId), bson.M{"$set": bson.M{ColumnTag: request.Tags}})
+ if err == nil {
return &discovery.AddServiceTagsResponse{
- Response:
discovery.CreateResponse(discovery.ErrInternal, "Failed to check service
exist"),
+ Response:
discovery.CreateResponse(discovery.ResponseSuccess, "Add service tags
successfully."),
}, nil
}
- tags := request.Tags
- res := quota.NewApplyQuotaResource(quota.TagQuotaType,
util.ParseDomainProject(ctx), request.ServiceId, int64(len(tags)))
- rst := quota.Apply(ctx, res)
- errQuota := rst.Err
- if errQuota != nil {
- log.Error(fmt.Sprintf("add service[%s]'s tags %v failed,
operator: %s", request.ServiceId, tags, remoteIP), errQuota)
- response := &discovery.AddServiceTagsResponse{
- Response: discovery.CreateResponseWithSCErr(errQuota),
- }
- if errQuota.InternalError() {
- return response, errQuota
- }
- return response, nil
- }
- dataTags := service.Tags
- for key, value := range dataTags {
- if _, ok := tags[key]; ok {
- continue
- }
- tags[key] = value
- }
- err = UpdateService(ctx, GeneratorServiceFilter(ctx,
request.ServiceId), bson.M{"$set": bson.M{ColumnTag: tags}})
- if err != nil {
- log.Error(fmt.Sprintf("update service %s tags failed.",
request.ServiceId), err)
+ log.Error(fmt.Sprintf("update service %s tags failed.",
request.ServiceId), err)
+ if err == client.ErrNoDocuments {
return &discovery.AddServiceTagsResponse{
- Response:
discovery.CreateResponse(discovery.ErrInternal, err.Error()),
+ Response:
discovery.CreateResponse(discovery.ErrServiceNotExists, err.Error()),
}, nil
}
return &discovery.AddServiceTagsResponse{
- Response: discovery.CreateResponse(discovery.ResponseSuccess,
"Add service tags successfully."),
+ Response: discovery.CreateResponse(discovery.ErrInternal,
err.Error()),
}, nil
}
@@ -2955,9 +2927,9 @@ func GetInstancesByServiceID(ctx context.Context,
serviceID string) ([]*discover
return res, nil
}
-func formatRevision(consumerServiceId string, instances
[]*discovery.MicroServiceInstance) (string, error) {
+func formatRevision(consumerServiceID string, instances
[]*discovery.MicroServiceInstance) (string, error) {
if instances == nil {
- return fmt.Sprintf("%x",
sha1.Sum(util.StringToBytesWithNoCopy(consumerServiceId))), nil
+ return fmt.Sprintf("%x",
sha1.Sum(util.StringToBytesWithNoCopy(consumerServiceID))), nil
}
copyInstance := make([]*discovery.MicroServiceInstance, len(instances))
copy(copyInstance, instances)
@@ -2967,6 +2939,6 @@ func formatRevision(consumerServiceId string, instances
[]*discovery.MicroServic
log.Error("fail to marshal instance json", err)
return "", err
}
- s := fmt.Sprintf("%s.%x", consumerServiceId, sha1.Sum(data))
+ s := fmt.Sprintf("%s.%x", consumerServiceID, sha1.Sum(data))
return fmt.Sprintf("%x", sha1.Sum(util.StringToBytesWithNoCopy(s))), nil
}
diff --git a/datasource/mongo/ms_test.go b/datasource/mongo/ms_test.go
index c9b05fb..b296c7d 100644
--- a/datasource/mongo/ms_test.go
+++ b/datasource/mongo/ms_test.go
@@ -608,7 +608,6 @@ func TestApplication_Get(t *testing.T) {
func TestTags_Add(t *testing.T) {
var (
serviceId1 string
- serviceId2 string
)
// create service
@@ -626,20 +625,6 @@ func TestTags_Add(t *testing.T) {
assert.NoError(t, err)
assert.NotEqual(t, "", resp.ServiceId)
serviceId1 = resp.ServiceId
-
- svc2 := &pb.MicroService{
- AppId: "create_tag_group_ms",
- ServiceName: "create_tag_service_ms",
- Version: "1.0.1",
- Level: "FRONT",
- Status: pb.MS_UP,
- }
- resp, err = datasource.Instance().RegisterService(getContext(),
&pb.CreateServiceRequest{
- Service: svc2,
- })
- assert.NoError(t, err)
- assert.NotNil(t, "", resp.ServiceId)
- serviceId2 = resp.ServiceId
})
t.Run("the request is invalid", func(t *testing.T) {
@@ -667,29 +652,6 @@ func TestTags_Add(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, pb.ResponseSuccess, resp.Response.GetCode())
})
-
- t.Run("tag's quota exceeded", func(t *testing.T) {
- size := quota.DefaultTagQuota / 2
- tags := make(map[string]string, size)
- for i := 0; i < size; i++ {
- s := "tag" + strconv.Itoa(i)
- tags[s] = s
- }
- resp, err := datasource.Instance().AddTags(getContext(),
&pb.AddServiceTagsRequest{
- ServiceId: serviceId2,
- Tags: tags,
- })
- assert.NoError(t, err)
- assert.Equal(t, pb.ResponseSuccess, resp.Response.GetCode())
-
- tags["out"] = "range"
- resp, _ = datasource.Instance().AddTags(getContext(),
&pb.AddServiceTagsRequest{
- ServiceId: serviceId2,
- Tags: tags,
- })
- assert.NoError(t, err)
- assert.Equal(t, pb.ErrNotEnoughQuota, resp.Response.GetCode())
- })
}
func TestTags_Get(t *testing.T) {
diff --git a/server/plugin/quota/buildin/common.go
b/server/plugin/quota/buildin/common.go
index d9656ce..ca69707 100644
--- a/server/plugin/quota/buildin/common.go
+++ b/server/plugin/quota/buildin/common.go
@@ -109,15 +109,8 @@ func resourceLimitHandler(ctx context.Context, res
*quota.ApplyQuotaResource) (i
return int64(len(resp.Schemas)), nil
}
case quota.TagQuotaType:
- {
- resp, err := datasource.Instance().GetTags(ctx,
&pb.GetServiceTagsRequest{
- ServiceId: serviceID,
- })
- if err != nil {
- return 0, err
- }
- return int64(len(resp.Tags)), nil
- }
+ // always re-create the service old tags
+ return 0, nil
default:
return 0, fmt.Errorf("not define quota type '%s'",
res.QuotaType)
}
diff --git a/server/service/tag_test.go b/server/service/tag_test.go
index d32480c..40f5c85 100644
--- a/server/service/tag_test.go
+++ b/server/service/tag_test.go
@@ -137,7 +137,7 @@ var _ = Describe("'Tag' service", func() {
Expect(err).To(BeNil())
Expect(respAddTags.Response.GetCode()).To(Equal(pb.ErrInvalidParams))
- size = quota.DefaultRuleQuota / 2
+ size = quota.DefaultRuleQuota
tags = make(map[string]string, size)
for i := 0; i < size; i++ {
s := "tag" + strconv.Itoa(i)
@@ -149,14 +149,6 @@ var _ = Describe("'Tag' service", func() {
})
Expect(err).To(BeNil())
Expect(respAddTags.Response.GetCode()).To(Equal(pb.ResponseSuccess))
-
- tags["out"] = "range"
- respAddTags, _ =
serviceResource.AddTags(getContext(), &pb.AddServiceTagsRequest{
- ServiceId: serviceId2,
- Tags: tags,
- })
- Expect(err).To(BeNil())
-
Expect(respAddTags.Response.GetCode()).To(Equal(pb.ErrNotEnoughQuota))
})
})
})
