You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by ti...@apache.org on 2019/07/02 08:55:38 UTC
[servicecomb-kie] 32/34: Refactoring the delete function
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-kie.git
commit 196d7dd8e181a5315462277b5b1d0417254eedee
Author: wangqijun <wa...@sohu.com>
AuthorDate: Tue Jul 2 09:14:14 2019 +0800
Refactoring the delete function
---
client/client.go | 54 ++++++++++++++++++++++++
client/client_test.go | 34 +++++++++++++--
server/dao/kie_api.go | 88 +++++++++++++++++++--------------------
server/dao/kv.go | 34 +++++++++++----
server/dao/kv_test.go | 35 ++++++++++------
server/dao/label_history.go | 32 ++++++++------
server/dao/mongodb_operator.go | 4 +-
server/resource/v1/common.go | 4 +-
server/resource/v1/doc_struct.go | 5 +++
server/resource/v1/kv_resource.go | 32 +++++++-------
10 files changed, 219 insertions(+), 103 deletions(-)
diff --git a/client/client.go b/client/client.go
index 58a3c49..34fc9b7 100644
--- a/client/client.go
+++ b/client/client.go
@@ -79,6 +79,39 @@ func New(config Config) (*Client, error) {
}, nil
}
+//Put create value of a key
+func (c *Client) Put(ctx context.Context, kv model.KVDoc) (*model.KVDoc, error) {
+ url := fmt.Sprintf("%s/%s/%s", c.opts.Endpoint, APIPathKV, kv.Key)
+ h := http.Header{}
+ h.Set("Content-Type", "application/json")
+ h.Set("domain", "test")
+ body, _ := json.Marshal(kv)
+ resp, err := c.c.HTTPDoWithContext(ctx, "PUT", url, h, body)
+ if err != nil {
+ return nil, err
+ }
+ b := httputil.ReadBody(resp)
+ if resp.StatusCode != http.StatusOK {
+ if resp.StatusCode == http.StatusNotFound {
+ return nil, ErrKeyNotExist
+ }
+ openlogging.Error("get failed", openlogging.WithTags(openlogging.Tags{
+ "k": kv.Key,
+ "status": resp.Status,
+ "body": b,
+ }))
+ return nil, fmt.Errorf("get %s failed,http status [%s], body [%s]", kv.Key, resp.Status, b)
+ }
+
+ kvs := &model.KVDoc{}
+ err = json.Unmarshal(b, kvs)
+ if err != nil {
+ openlogging.Error("unmarshal kv failed:" + err.Error())
+ return nil, err
+ }
+ return kvs, nil
+}
+
//Get get value of a key
func (c *Client) Get(ctx context.Context, key string, opts ...GetOption) ([]*model.KVDoc, error) {
options := GetOptions{}
@@ -112,3 +145,24 @@ func (c *Client) Get(ctx context.Context, key string, opts ...GetOption) ([]*mod
}
return kvs, nil
}
+
+//Delete remove kv
+func (c *Client) Delete(ctx context.Context, kvID, labelID string) error {
+ url := fmt.Sprintf("%s/%s/%s", c.opts.Endpoint, APIPathKV, kvID)
+ if labelID != "" {
+ url = fmt.Sprintf("%s?labelID=%s", url, labelID)
+ }
+ h := http.Header{}
+ h.Set("Content-Type", "application/json")
+ h.Set("domain", "test")
+
+ resp, err := c.c.HTTPDoWithContext(ctx, "DELETE", url, h, nil)
+ if err != nil {
+ return err
+ }
+ b := httputil.ReadBody(resp)
+ if resp.StatusCode != http.StatusNoContent {
+ return fmt.Errorf("delete %s failed,http status [%s], body [%s]", kvID, resp.Status, b)
+ }
+ return nil
+}
diff --git a/client/client_test.go b/client/client_test.go
index 937b6c1..bf125fd 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -18,11 +18,11 @@
package client_test
import (
- . "github.com/onsi/ginkgo"
- . "github.com/onsi/gomega"
-
"context"
. "github.com/apache/servicecomb-kie/client"
+ "github.com/apache/servicecomb-kie/pkg/model"
+ . "github.com/onsi/ginkgo"
+ . "github.com/onsi/gomega"
"os"
)
@@ -62,4 +62,32 @@ var _ = Describe("Client", func() {
})
})
+
+ Describe("DELETE /v1/kv/", func() {
+ Context("by kvID", func() {
+ client2, err := New(Config{
+ Endpoint: "http://127.0.0.1:30110",
+ })
+
+ kvBody := model.KVDoc{}
+ kvBody.Key = "time"
+ kvBody.Value = "100s"
+ kvBody.ValueType = "string"
+ kvBody.Labels = make(map[string]string)
+ kvBody.Labels["evn"] = "test"
+ kv, err := client2.Put(context.TODO(), kvBody)
+ It("should be not error", func() {
+ Ω(err).ShouldNot(HaveOccurred())
+ Expect(kv.Key).To(Equal(kvBody.Key))
+ })
+ client3, err := New(Config{
+ Endpoint: "http://127.0.0.1:30110",
+ })
+ It("should be 204", func() {
+ err := client3.Delete(context.TODO(), kv.ID.Hex(), "")
+ Ω(err).ShouldNot(HaveOccurred())
+ })
+ })
+ })
+
})
diff --git a/server/dao/kie_api.go b/server/dao/kie_api.go
index 5e641aa..de65dcd 100644
--- a/server/dao/kie_api.go
+++ b/server/dao/kie_api.go
@@ -193,7 +193,8 @@ func (s *MongodbService) FindKVByLabelID(ctx context.Context, domain, labelID, k
ctx, _ = context.WithTimeout(context.Background(), DefaultTimeout)
filter := bson.M{"label_id": labelID, "domain": domain}
if key != "" {
- return s.findOneKey(ctx, filter, key)
+ filter["key"] = key
+ return s.findOneKey(ctx, filter)
}
return s.findKeys(ctx, filter, true)
@@ -279,63 +280,58 @@ func (s *MongodbService) FindKV(ctx context.Context, domain string, options ...F
}
-//DeleteByID delete a key value by collection ID
-func (s *MongodbService) DeleteByID(id string) error {
- collection := s.c.Database(DB).Collection(CollectionKV)
- hex, err := primitive.ObjectIDFromHex(id)
- if err != nil {
- openlogging.Error(fmt.Sprintf("convert %s ,err:%s", id, err))
- return err
- }
- ctx, _ := context.WithTimeout(context.Background(), DefaultTimeout)
- dr, err := collection.DeleteOne(ctx, bson.M{"_id": hex})
- if err != nil {
- openlogging.Error(fmt.Sprintf("delete [%s] failed: %s", hex, err))
- }
- if dr.DeletedCount != 1 {
- openlogging.Warn(fmt.Sprintf("delete [%s], but it is not exist", hex))
- }
- return nil
-}
-
-//Delete remove a list of key values for a tenant
+//Delete delete kv,If the labelID is "", query the collection kv to get it
//domain=tenant
-func (s *MongodbService) Delete(ids []string, domain string) error {
- if len(ids) == 0 {
- openlogging.Warn("delete error,ids is blank")
- return nil
- }
+//1.delete kv;2.add history
+func (s *MongodbService) Delete(kvID string, labelID string, domain string) error {
+ ctx, _ := context.WithTimeout(context.Background(), DefaultTimeout)
if domain == "" {
return ErrMissingDomain
}
- collection := s.c.Database(DB).Collection(CollectionKV)
- //transfer id
- var oid []primitive.ObjectID
- for _, v := range ids {
- if v == "" {
- openlogging.Warn("ids contains continuous ','")
- continue
- }
- hex, err := primitive.ObjectIDFromHex(v)
+ hex, err := primitive.ObjectIDFromHex(kvID)
+ if err != nil {
+ return err
+ }
+ //if labelID == "",get labelID by kvID
+ var kv *model.KVDoc
+ if labelID == "" {
+ kvArray, err := s.findOneKey(ctx, bson.M{"_id": hex})
if err != nil {
- openlogging.Error(fmt.Sprintf("convert %s ,err:%s", v, err))
return err
}
- oid = append(oid, hex)
+ if len(kvArray) > 0 {
+ kv = kvArray[0]
+ labelID = kv.LabelID
+ }
}
- //use in filter
- filter := &bson.M{"_id": &bson.M{"$in": oid}, "domain": domain}
- ctx, _ := context.WithTimeout(context.Background(), DefaultTimeout)
- dr, err := collection.DeleteMany(ctx, filter)
- //check error and delete number
+ //get Label and check labelID
+ r, err := s.getLatestLabel(ctx, labelID)
+ if err != nil {
+ if err == ErrRevisionNotExist {
+ openlogging.Warn(fmt.Sprintf("failed,kvID and labelID do not match"))
+ return ErrKvIDAndLabelIDNotMatch
+ }
+ return err
+ }
+ //delete kv
+ err = s.DeleteKV(ctx, hex)
if err != nil {
- openlogging.Error(fmt.Sprintf("delete [%v] failed : [%s]", filter, err))
return err
}
- if dr.DeletedCount != int64(len(oid)) {
- openlogging.Warn(fmt.Sprintf(" The actual number of deletions[%d] is not equal to the parameters[%d].", dr.DeletedCount, len(oid)))
+ //Labels will not be empty when deleted
+ revision, err := s.addHistory(ctx, r, labelID)
+ if err != nil {
+ openlogging.Warn("add history failed ,", openlogging.WithTags(openlogging.Tags{
+ "kvID": kvID,
+ "labelID": labelID,
+ "error": err.Error(),
+ }))
} else {
- openlogging.Info(fmt.Sprintf("delete success,count=%d", dr.DeletedCount))
+ openlogging.Info("add history success,", openlogging.WithTags(openlogging.Tags{
+ "kvID": kvID,
+ "labelID": labelID,
+ "revision": revision,
+ }))
}
return nil
}
diff --git a/server/dao/kv.go b/server/dao/kv.go
index 41aba48..e3dcad1 100644
--- a/server/dao/kv.go
+++ b/server/dao/kv.go
@@ -34,12 +34,14 @@ import (
//db errors
var (
- ErrMissingDomain = errors.New("domain info missing, illegal access")
- ErrKeyNotExists = errors.New("key with labels does not exits")
- ErrLabelNotExists = errors.New("labels does not exits")
- ErrTooMany = errors.New("key with labels should be only one")
- ErrKeyMustNotEmpty = errors.New("must supply key if you want to get exact one result")
- ErrRevisionNotExist = errors.New("label revision not exist")
+ ErrMissingDomain = errors.New("domain info missing, illegal access")
+ ErrKeyNotExists = errors.New("key with labels does not exits")
+ ErrLabelNotExists = errors.New("labels does not exits")
+ ErrTooMany = errors.New("key with labels should be only one")
+ ErrKeyMustNotEmpty = errors.New("must supply key if you want to get exact one result")
+ ErrRevisionNotExist = errors.New("label revision not exist")
+ ErrKVIDIsNil = errors.New("kvID id is nil")
+ ErrKvIDAndLabelIDNotMatch = errors.New("kvID and labelID do not match")
)
//Options mongodb options
@@ -64,9 +66,8 @@ func NewKVService() (*MongodbService, error) {
}
return NewMongoService(opts)
}
-func (s *MongodbService) findOneKey(ctx context.Context, filter bson.M, key string) ([]*model.KVDoc, error) {
+func (s *MongodbService) findOneKey(ctx context.Context, filter bson.M) ([]*model.KVDoc, error) {
collection := s.c.Database(DB).Collection(CollectionKV)
- filter["key"] = key
sr := collection.FindOne(ctx, filter)
if sr.Err() != nil {
return nil, sr.Err()
@@ -128,3 +129,20 @@ func (s *MongodbService) findKV(ctx context.Context, domain string, opts FindOpt
}
return cur, err
}
+
+//DeleteKV by kvID
+func (s *MongodbService) DeleteKV(ctx context.Context, hexID primitive.ObjectID) error {
+ collection := s.c.Database(DB).Collection(CollectionKV)
+ dr, err := collection.DeleteOne(ctx, bson.M{"_id": hexID})
+ //check error and delete number
+ if err != nil {
+ openlogging.Error(fmt.Sprintf("delete [%s] failed : [%s]", hexID, err))
+ return err
+ }
+ if dr.DeletedCount != 1 {
+ openlogging.Warn(fmt.Sprintf("Failed,May have been deleted,kvID=%s", hexID))
+ } else {
+ openlogging.Info(fmt.Sprintf("delete success,kvID=%s", hexID))
+ }
+ return err
+}
diff --git a/server/dao/kv_test.go b/server/dao/kv_test.go
index db92857..0de63a3 100644
--- a/server/dao/kv_test.go
+++ b/server/dao/kv_test.go
@@ -197,7 +197,7 @@ var _ = Describe("Kv mongodb service", func() {
})
Describe("delete key", func() {
- Context("delete key by id,seperated by ',' ", func() {
+ Context("delete key by kvID", func() {
kv1, err := s.CreateOrUpdate(context.Background(), "default", &model.KVDoc{
Key: "timeout",
Value: "20s",
@@ -208,11 +208,20 @@ var _ = Describe("Kv mongodb service", func() {
It("should not return err", func() {
Expect(err).Should(BeNil())
})
+ It("should not return err", func() {
+ Expect(err).Should(BeNil())
+ })
- kv2, err := s.CreateOrUpdate(context.Background(), "default", &model.KVDoc{
- Key: "times",
- Value: "3",
- Domain: "default",
+ err = s.Delete(kv1.ID.Hex(), "", "default")
+ It("should not return err", func() {
+ Expect(err).Should(BeNil())
+ })
+
+ })
+ Context("delete key by kvID and labelID", func() {
+ kv1, err := s.CreateOrUpdate(context.Background(), "default", &model.KVDoc{
+ Key: "timeout",
+ Value: "20s",
Labels: map[string]string{
"env": "test",
},
@@ -220,28 +229,30 @@ var _ = Describe("Kv mongodb service", func() {
It("should not return err", func() {
Expect(err).Should(BeNil())
})
+ It("should not return err", func() {
+ Expect(err).Should(BeNil())
+ })
- ids := []string{kv1.ID.Hex(), kv2.ID.Hex()}
- err = s.Delete(ids, "default")
+ err = s.Delete(kv1.ID.Hex(), kv1.LabelID, "default")
It("should not return err", func() {
Expect(err).Should(BeNil())
})
})
- Context("test miss ids, no panic", func() {
- err := s.Delete(nil, "default")
+ Context("test miss kvID, no panic", func() {
+ err := s.Delete("", "", "default")
It("should not return err", func() {
- Expect(err).Should(BeNil())
+ Expect(err).Should(HaveOccurred())
})
})
Context("Test encode error ", func() {
- err := s.Delete([]string{"12312312321"}, "default")
+ err := s.Delete("12312312321", "", "default")
It("should return err", func() {
Expect(err).To(HaveOccurred())
})
})
Context("Test miss domain error ", func() {
- err := s.Delete([]string{"5ce3602381fc6e33708b9621"}, "")
+ err := s.Delete("12312312321", "", "")
It("should return err", func() {
Expect(err).Should(Equal(dao.ErrMissingDomain))
})
diff --git a/server/dao/label_history.go b/server/dao/label_history.go
index a3809d6..c796e7b 100644
--- a/server/dao/label_history.go
+++ b/server/dao/label_history.go
@@ -19,7 +19,6 @@ package dao
import (
"context"
-
"fmt"
"github.com/apache/servicecomb-kie/pkg/model"
"github.com/go-mesh/openlogging"
@@ -57,8 +56,8 @@ func (s *MongodbService) getLatestLabel(ctx context.Context, labelID string) (*m
return h, nil
}
-//AddHistory get latest labels revision and plus 1 and save current label stats to history, then update current revision to db
-func (s *MongodbService) AddHistory(ctx context.Context, labelID string, labels map[string]string, domain string) (int, error) {
+//getAndAddHistory get latest labels revision and call addHistory
+func (s *MongodbService) getAndAddHistory(ctx context.Context, labelID string, labels map[string]string, domain string) (int, error) {
r, err := s.getLatestLabel(ctx, labelID)
if err != nil {
if err == ErrRevisionNotExist {
@@ -76,23 +75,31 @@ func (s *MongodbService) AddHistory(ctx context.Context, labelID string, labels
}
}
- r.Revision = r.Revision + 1
+ r.Revision, err = s.addHistory(ctx, r, labelID)
+ if err != nil {
+ return 0, err
+ }
+ return r.Revision, nil
+}
+//addHistory labels revision plus 1 and save current label stats to history, then update current revision to db
+func (s *MongodbService) addHistory(ctx context.Context, labelRevision *model.LabelRevisionDoc, labelID string) (int, error) {
+ labelRevision.Revision = labelRevision.Revision + 1
kvs, err := s.findKeys(ctx, bson.M{"label_id": labelID}, true)
- if err != nil {
+ //Key may be empty When delete
+ if err != nil && err != ErrKeyNotExists {
return 0, err
}
//save current kv states
- r.KVs = kvs
+ labelRevision.KVs = kvs
//clear prev id
- r.ID = primitive.NilObjectID
+ labelRevision.ID = primitive.NilObjectID
collection := s.c.Database(DB).Collection(CollectionLabelRevision)
- _, err = collection.InsertOne(ctx, r)
+ _, err = collection.InsertOne(ctx, labelRevision)
if err != nil {
openlogging.Error(err.Error())
return 0, err
}
-
hex, err := primitive.ObjectIDFromHex(labelID)
if err != nil {
openlogging.Error(fmt.Sprintf("convert %s,err:%s", labelID, err))
@@ -101,13 +108,12 @@ func (s *MongodbService) AddHistory(ctx context.Context, labelID string, labels
labelCollection := s.c.Database(DB).Collection(CollectionLabel)
_, err = labelCollection.UpdateOne(ctx, bson.M{"_id": hex}, bson.D{
{"$set", bson.D{
- {"revision", r.Revision},
+ {"revision", labelRevision.Revision},
}},
})
if err != nil {
return 0, err
}
- openlogging.Debug(fmt.Sprintf("update revision to %d", r.Revision))
-
- return r.Revision, nil
+ openlogging.Debug(fmt.Sprintf("update revision to %d", labelRevision.Revision))
+ return labelRevision.Revision, nil
}
diff --git a/server/dao/mongodb_operator.go b/server/dao/mongodb_operator.go
index ce672d1..e588f5a 100644
--- a/server/dao/mongodb_operator.go
+++ b/server/dao/mongodb_operator.go
@@ -52,7 +52,7 @@ func (s *MongodbService) createKey(ctx context.Context, kv *model.KVDoc) (*model
}
objectID, _ := res.InsertedID.(primitive.ObjectID)
kv.ID = objectID
- revision, err := s.AddHistory(ctx, kv.LabelID, kv.Labels, kv.Domain)
+ revision, err := s.getAndAddHistory(ctx, kv.LabelID, kv.Labels, kv.Domain)
if err != nil {
openlogging.Warn(
fmt.Sprintf("can not updateKey version for [%s] [%s] in [%s]",
@@ -81,7 +81,7 @@ func (s *MongodbService) updateKey(ctx context.Context, kv *model.KVDoc) (int, e
openlogging.Debug(
fmt.Sprintf("updateKey %s with labels %s value [%s] %d ",
kv.Key, kv.Labels, kv.Value, ur.ModifiedCount))
- revision, err := s.AddHistory(ctx, kv.LabelID, kv.Labels, kv.Domain)
+ revision, err := s.getAndAddHistory(ctx, kv.LabelID, kv.Labels, kv.Domain)
if err != nil {
openlogging.Warn(
fmt.Sprintf("can not label revision for [%s] [%s] in [%s],err: %s",
diff --git a/server/resource/v1/common.go b/server/resource/v1/common.go
index 9b01260..f96c11c 100644
--- a/server/resource/v1/common.go
+++ b/server/resource/v1/common.go
@@ -35,8 +35,8 @@ const (
MsgDomainMustNotBeEmpty = "domain must not be empty"
MsgIllegalLabels = "label's value can not be empty, " +
"label can not be duplicated, please check your query parameters"
- MsgIllegalDepth = "X-Depth must be number"
- ErrIDMustNotEmpty = "must supply id if you want to remove key"
+ MsgIllegalDepth = "X-Depth must be number"
+ ErrKvIDMustNotEmpty = "must supply kv id if you want to remove key"
)
//ReadDomain get domain info from attribute
diff --git a/server/resource/v1/doc_struct.go b/server/resource/v1/doc_struct.go
index 33f0eb7..d39fc7e 100644
--- a/server/resource/v1/doc_struct.go
+++ b/server/resource/v1/doc_struct.go
@@ -44,6 +44,11 @@ var (
Name: "key",
ParamType: goRestful.PathParameterKind,
}
+ labelIDParameters = &restful.Parameters{
+ DataType: "string",
+ Name: "key",
+ ParamType: goRestful.PathParameterKind,
+ }
)
//KVBody is open api doc
diff --git a/server/resource/v1/kv_resource.go b/server/resource/v1/kv_resource.go
index 832dae7..d8b2d50 100644
--- a/server/resource/v1/kv_resource.go
+++ b/server/resource/v1/kv_resource.go
@@ -20,14 +20,12 @@ package v1
import (
"encoding/json"
- "fmt"
"github.com/apache/servicecomb-kie/pkg/model"
"github.com/apache/servicecomb-kie/server/dao"
goRestful "github.com/emicklei/go-restful"
"github.com/go-chassis/go-chassis/server/restful"
"github.com/go-mesh/openlogging"
"net/http"
- "strings"
)
//KVResource has API about kv operations
@@ -163,20 +161,24 @@ func (r *KVResource) Delete(context *restful.Context) {
if domain == nil {
WriteErrResponse(context, http.StatusInternalServerError, MsgDomainMustNotBeEmpty)
}
- ids := context.ReadPathParameter("ids")
- if ids == "" {
- WriteErrResponse(context, http.StatusBadRequest, ErrIDMustNotEmpty)
+ kvID := context.ReadPathParameter("kvID")
+ if kvID == "" {
+ WriteErrResponse(context, http.StatusBadRequest, ErrKvIDMustNotEmpty)
return
}
- idArray := strings.Split(ids, ",")
+ labelID := context.ReadQueryParameter("labelID")
s, err := dao.NewKVService()
if err != nil {
WriteErrResponse(context, http.StatusInternalServerError, err.Error())
return
}
- err = s.Delete(idArray, domain.(string))
+ err = s.Delete(kvID, labelID, domain.(string))
if err != nil {
- openlogging.Error(fmt.Sprintf("delete ids=%s,err=%s", ids, err.Error()))
+ openlogging.Error("delete failed ,", openlogging.WithTags(openlogging.Tags{
+ "kvID": kvID,
+ "labelID": labelID,
+ "error": err.Error(),
+ }))
WriteErrResponse(context, http.StatusInternalServerError, err.Error())
return
}
@@ -245,16 +247,12 @@ func (r *KVResource) URLPatterns() []restful.Route {
Produces: []string{goRestful.MIME_JSON},
}, {
Method: http.MethodDelete,
- Path: "/v1/kv/{ids}",
+ Path: "/v1/kv/{kvID}",
ResourceFuncName: "Delete",
- FuncDesc: "delete key by id,separated by ','",
- Parameters: []*restful.Parameters{{
- DataType: "string",
- Name: "ids",
- ParamType: goRestful.PathParameterKind,
- Desc: "The id strings to be removed are separated by ',',If the actual number of deletions " +
- "and the number of parameters are not equal, no error will be returned and only warn log will be printed.",
- },
+ FuncDesc: "Delete key by kvID and labelID,If the labelID is nil, query the collection kv to get it." +
+ "It means if only get kvID, it can also delete normally.But if you want better performance, you need to pass the labelID",
+ Parameters: []*restful.Parameters{
+ labelIDParameters,
},
Returns: []*restful.Returns{
{