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 2020/05/06 03:52:43 UTC

[servicecomb-kie] branch master updated: API get/list Use validate pkg to check param (#140)

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


The following commit(s) were added to refs/heads/master by this push:
     new 167443b  API get/list Use validate pkg to check param (#140)
167443b is described below

commit 167443bcbc1122cfc592420362dd54b72a95d9d7
Author: humingcheng <hu...@163.com>
AuthorDate: Tue May 5 22:52:36 2020 -0500

    API get/list Use validate pkg to check param (#140)
---
 pkg/model/db_schema.go                 | 18 +++++++++
 pkg/validate/instance.go               |  7 ++--
 pkg/validate/validator.go              |  5 +--
 server/resource/v1/common.go           | 31 +++++-----------
 server/resource/v1/kv_resource.go      | 67 ++++++++++++++++------------------
 server/resource/v1/kv_resource_test.go |  1 +
 server/service/mongo/kv/kv_service.go  | 18 ++++-----
 server/service/service.go              |  2 +-
 8 files changed, 73 insertions(+), 76 deletions(-)

diff --git a/pkg/model/db_schema.go b/pkg/model/db_schema.go
index cdcf8ee..4abf22b 100644
--- a/pkg/model/db_schema.go
+++ b/pkg/model/db_schema.go
@@ -77,3 +77,21 @@ type UpdateKVRequest struct {
 	Domain  string `json:"domain,omitempty" yaml:"domain,omitempty" validate:"commonName"` //redundant
 	Status  string `json:"status,omitempty" yaml:"status,omitempty" validate:"kvStatus"`
 }
+
+// GetKVRequest contains kv get request params
+type GetKVRequest struct {
+	Project string `json:"project,omitempty" yaml:"project,omitempty" validate:"commonName"`
+	Domain  string `json:"domain,omitempty" yaml:"domain,omitempty" validate:"commonName"` //redundant
+	ID      string `json:"id,omitempty" bson:"id,omitempty" yaml:"id,omitempty" swag:"string" validate:"uuid"`
+}
+
+// ListKVRequest contains kv list request params
+type ListKVRequest struct {
+	Project string            `json:"project,omitempty" yaml:"project,omitempty" validate:"commonName"`
+	Domain  string            `json:"domain,omitempty" yaml:"domain,omitempty" validate:"commonName"` //redundant
+	Key     string            `json:"key" yaml:"key" validate:"getKey"`
+	Labels  map[string]string `json:"labels,omitempty" yaml:"labels,omitempty" validate:"max=8,dive,keys,lableKV,endkeys,lableKV"` //redundant
+	Offset  int64             `validate:"min=0"`
+	Limit   int64             `validate:"min=0,max=100"`
+	Status  string            `json:"status,omitempty" yaml:"status,omitempty" validate:"kvStatus"`
+}
diff --git a/pkg/validate/instance.go b/pkg/validate/instance.go
index 9c8387d..90d3c8d 100644
--- a/pkg/validate/instance.go
+++ b/pkg/validate/instance.go
@@ -11,10 +11,11 @@ const (
 // custom validate rules
 // please use different tag names from third party tags
 var customRules = []*RegexValidateRule{
-	NewRule(key, commonNameRegexString, &Option{Min: 1, Max: 1024}), //2K
+	NewRule(key, commonNameRegexString, &Option{Min: 1, Max: 1024}), //1K
+	NewRule("getKey", commonNameRegexString, &Option{Max: 1024}),    //1K
 	NewRule("commonName", commonNameRegexString, &Option{Min: 1, Max: 256}),
-	NewRule("valueType", `^(ini|json|text|yaml|properties){0,1}$`, nil),
-	NewRule("kvStatus", `^(enabled|disabled){0,1}$`, nil),
+	NewRule("valueType", `^$|^(ini|json|text|yaml|properties)$`, nil),
+	NewRule("kvStatus", `^$|^(enabled|disabled)$`, nil),
 	NewRule("value", asciiRegexString, &Option{Max: 2097152}), //ASCII, 2M
 	NewRule("lableKV", commonNameRegexString, &Option{Max: 32}),
 	NewRule("check", asciiRegexString, &Option{Max: 1048576}), //ASCII, 1M
diff --git a/pkg/validate/validator.go b/pkg/validate/validator.go
index aa2b6b1..59fdf26 100644
--- a/pkg/validate/validator.go
+++ b/pkg/validate/validator.go
@@ -52,10 +52,7 @@ func (v *Validator) RegisterRule(r *RegexValidateRule) error {
 	if err := v.valid.RegisterValidation(r.tag, r.validateFL); err != nil {
 		return err
 	}
-	if err := v.AddErrorTranslation4Tag(r.tag); err != nil {
-		return err
-	}
-	return nil
+	return v.AddErrorTranslation4Tag(r.tag)
 }
 
 // translates raw errors to easy-to-understand messages
diff --git a/server/resource/v1/common.go b/server/resource/v1/common.go
index 91987ea..d9d3530 100644
--- a/server/resource/v1/common.go
+++ b/server/resource/v1/common.go
@@ -218,10 +218,6 @@ func validateGet(domain, project, kvID string) error {
 	return checkDomainAndProject(domain, project)
 }
 
-func validateList(domain, project string) error {
-	return checkDomainAndProject(domain, project)
-}
-
 func validateDelete(domain, project, kvID string) error {
 	return validateGet(domain, project, kvID)
 }
@@ -240,35 +236,26 @@ func checkDomainAndProject(domain, project string) error {
 	return nil
 }
 
-func checkStatus(status string) (string, error) {
-	if status != "" {
-		if status != common.StatusEnabled && status != common.StatusDisabled {
-			return "", errors.New("invalid status string")
-		}
-	}
-	return status, nil
-}
-
-func queryAndResponse(rctx *restful.Context, doc *model.KVDoc, offset, limit int64) {
+func queryAndResponse(rctx *restful.Context, request *model.ListKVRequest) {
 	m := getMatchPattern(rctx)
 	opts := []service.FindOption{
-		service.WithKey(doc.Key),
-		service.WithLabels(doc.Labels),
-		service.WithOffset(offset),
-		service.WithLimit(limit),
+		service.WithKey(request.Key),
+		service.WithLabels(request.Labels),
+		service.WithOffset(request.Offset),
+		service.WithLimit(request.Limit),
 	}
 	if m == common.PatternExact {
 		opts = append(opts, service.WithExactLabels())
 	}
-	if doc.Status != "" {
-		opts = append(opts, service.WithStatus(doc.Status))
+	if request.Status != "" {
+		opts = append(opts, service.WithStatus(request.Status))
 	}
-	rev, err := service.RevisionService.GetRevision(rctx.Ctx, doc.Domain)
+	rev, err := service.RevisionService.GetRevision(rctx.Ctx, request.Domain)
 	if err != nil {
 		WriteErrResponse(rctx, http.StatusInternalServerError, err.Error())
 		return
 	}
-	kv, err := service.KVService.List(rctx.Ctx, doc.Domain, doc.Project, opts...)
+	kv, err := service.KVService.List(rctx.Ctx, request.Domain, request.Project, opts...)
 	if err != nil {
 		openlogging.Error("common: " + err.Error())
 		WriteErrResponse(rctx, http.StatusInternalServerError, common.MsgDBError)
diff --git a/server/resource/v1/kv_resource.go b/server/resource/v1/kv_resource.go
index 7f9a4ab..cbe335c 100644
--- a/server/resource/v1/kv_resource.go
+++ b/server/resource/v1/kv_resource.go
@@ -22,7 +22,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"github.com/go-chassis/go-chassis/pkg/backends/quota"
 	"net/http"
 
 	"github.com/apache/servicecomb-kie/pkg/common"
@@ -31,7 +30,9 @@ import (
 	"github.com/apache/servicecomb-kie/server/pubsub"
 	"github.com/apache/servicecomb-kie/server/service"
 	"github.com/apache/servicecomb-kie/server/service/mongo/session"
+
 	goRestful "github.com/emicklei/go-restful"
+	"github.com/go-chassis/go-chassis/pkg/backends/quota"
 	"github.com/go-chassis/go-chassis/server/restful"
 	"github.com/go-mesh/openlogging"
 )
@@ -146,15 +147,17 @@ func (r *KVResource) Put(rctx *restful.Context) {
 
 //Get search key by kv id
 func (r *KVResource) Get(rctx *restful.Context) {
-	project := rctx.ReadPathParameter(common.PathParameterProject)
-	domain := ReadDomain(rctx).(string)
-	kvID := rctx.ReadPathParameter(common.PathParamKVID)
-	err := validateGet(domain, project, kvID)
+	request := &model.GetKVRequest{
+		Project: rctx.ReadPathParameter(common.PathParameterProject),
+		Domain:  ReadDomain(rctx).(string),
+		ID:      rctx.ReadPathParameter(common.PathParamKVID),
+	}
+	err := validate.Validate(request)
 	if err != nil {
 		WriteErrResponse(rctx, http.StatusBadRequest, err.Error())
 		return
 	}
-	kv, err := service.KVService.Get(rctx.Ctx, domain, project, kvID)
+	kv, err := service.KVService.Get(rctx.Ctx, request)
 	if err != nil {
 		openlogging.Error("kv_resource: " + err.Error())
 		if err == service.ErrKeyNotExists {
@@ -176,20 +179,18 @@ func (r *KVResource) Get(rctx *restful.Context) {
 //List response kv list
 func (r *KVResource) List(rctx *restful.Context) {
 	var err error
-	key := rctx.ReadQueryParameter(common.QueryParamKey)
-	project := rctx.ReadPathParameter(common.PathParameterProject)
-	domain := ReadDomain(rctx).(string)
-	err = validateList(domain, project)
-	if err != nil {
-		WriteErrResponse(rctx, http.StatusBadRequest, err.Error())
-		return
+	request := &model.ListKVRequest{
+		Project: rctx.ReadPathParameter(common.PathParameterProject),
+		Domain:  ReadDomain(rctx).(string),
+		Key:     rctx.ReadQueryParameter(common.QueryParamKey),
+		Status:  rctx.ReadQueryParameter(common.QueryParamStatus),
 	}
 	labels, err := getLabels(rctx)
 	if err != nil {
 		WriteErrResponse(rctx, http.StatusBadRequest, common.MsgIllegalLabels)
 		return
 	}
-
+	request.Labels = labels
 	offsetStr := rctx.ReadQueryParameter(common.QueryParamOffset)
 	limitStr := rctx.ReadQueryParameter(common.QueryParamLimit)
 	offset, limit, err := checkPagination(offsetStr, limitStr)
@@ -197,47 +198,41 @@ func (r *KVResource) List(rctx *restful.Context) {
 		WriteErrResponse(rctx, http.StatusBadRequest, err.Error())
 		return
 	}
-	sessionID := rctx.ReadHeader(HeaderSessionID)
-	statusStr := rctx.ReadQueryParameter(common.QueryParamStatus)
-	status, err := checkStatus(statusStr)
+	request.Offset = offset
+	request.Limit = limit
+	err = validate.Validate(request)
 	if err != nil {
 		WriteErrResponse(rctx, http.StatusBadRequest, err.Error())
 		return
 	}
-	returnData(rctx, &model.KVDoc{
-		Domain:  domain,
-		Project: project,
-		Key:     key,
-		Labels:  labels,
-		Status:  status,
-	}, offset, limit, sessionID)
+	returnData(rctx, request)
 }
 
-func returnData(rctx *restful.Context, doc *model.KVDoc, offset, limit int64, sessionID string) {
+func returnData(rctx *restful.Context, request *model.ListKVRequest) {
 	revStr := rctx.ReadQueryParameter(common.QueryParamRev)
 	wait := rctx.ReadQueryParameter(common.QueryParamWait)
 	if revStr == "" {
 		if wait == "" {
-			queryAndResponse(rctx, doc, offset, limit)
+			queryAndResponse(rctx, request)
 			return
 		}
 		changed, err := eventHappened(rctx, wait, &pubsub.Topic{
-			Labels:    doc.Labels,
-			Project:   doc.Project,
+			Labels:    request.Labels,
+			Project:   request.Project,
 			MatchType: getMatchPattern(rctx),
-			DomainID:  doc.Domain,
+			DomainID:  request.Domain,
 		})
 		if err != nil {
 			WriteErrResponse(rctx, http.StatusBadRequest, err.Error())
 			return
 		}
 		if changed {
-			queryAndResponse(rctx, doc, offset, limit)
+			queryAndResponse(rctx, request)
 			return
 		}
 		rctx.WriteHeader(http.StatusNotModified)
 	} else {
-		revised, err := isRevised(rctx.Ctx, revStr, doc.Domain)
+		revised, err := isRevised(rctx.Ctx, revStr, request.Domain)
 		if err != nil {
 			if err == ErrInvalidRev {
 				WriteErrResponse(rctx, http.StatusBadRequest, err.Error())
@@ -247,21 +242,21 @@ func returnData(rctx *restful.Context, doc *model.KVDoc, offset, limit int64, se
 			return
 		}
 		if revised {
-			queryAndResponse(rctx, doc, offset, limit)
+			queryAndResponse(rctx, request)
 			return
 		} else if wait != "" {
 			changed, err := eventHappened(rctx, wait, &pubsub.Topic{
-				Labels:    doc.Labels,
-				Project:   doc.Project,
+				Labels:    request.Labels,
+				Project:   request.Project,
 				MatchType: getMatchPattern(rctx),
-				DomainID:  doc.Domain,
+				DomainID:  request.Domain,
 			})
 			if err != nil {
 				WriteErrResponse(rctx, http.StatusBadRequest, err.Error())
 				return
 			}
 			if changed {
-				queryAndResponse(rctx, doc, offset, limit)
+				queryAndResponse(rctx, request)
 				return
 			}
 			rctx.WriteHeader(http.StatusNotModified)
diff --git a/server/resource/v1/kv_resource_test.go b/server/resource/v1/kv_resource_test.go
index 8d229ef..a7d81a0 100644
--- a/server/resource/v1/kv_resource_test.go
+++ b/server/resource/v1/kv_resource_test.go
@@ -174,6 +174,7 @@ func TestKVResource_List(t *testing.T) {
 		c.ServeHTTP(resp, r)
 		body, err := ioutil.ReadAll(resp.Body)
 		assert.NoError(t, err)
+		assert.Equal(t, http.StatusOK, resp.Code, string(body))
 		result := &model.KVResponse{}
 		err = json.Unmarshal(body, result)
 		assert.NoError(t, err)
diff --git a/server/service/mongo/kv/kv_service.go b/server/service/mongo/kv/kv_service.go
index 97d3c20..3a11214 100644
--- a/server/service/mongo/kv/kv_service.go
+++ b/server/service/mongo/kv/kv_service.go
@@ -73,7 +73,12 @@ func (s *Service) Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, er
 //Update will update a key value record
 func (s *Service) Update(ctx context.Context, kv *model.UpdateKVRequest) (*model.KVDoc, error) {
 	ctx, _ = context.WithTimeout(ctx, session.Timeout)
-	oldKV, err := s.Get(ctx, kv.Domain, kv.Project, kv.ID)
+	getRequest := &model.GetKVRequest{
+		Domain:  kv.Domain,
+		Project: kv.Project,
+		ID:      kv.ID,
+	}
+	oldKV, err := s.Get(ctx, getRequest)
 	if err != nil {
 		return nil, err
 	}
@@ -171,15 +176,8 @@ func (s *Service) List(ctx context.Context, domain, project string, options ...s
 }
 
 //Get get kvs by id
-func (s *Service) Get(ctx context.Context, domain, project, id string, options ...service.FindOption) (*model.KVDoc, error) {
-	opts := service.FindOptions{}
-	for _, o := range options {
-		o(&opts)
-	}
-	if opts.Timeout == 0 {
-		opts.Timeout = session.DefaultTimeout
-	}
-	return findKVDocByID(ctx, domain, project, id)
+func (s *Service) Get(ctx context.Context, request *model.GetKVRequest) (*model.KVDoc, error) {
+	return findKVDocByID(ctx, request.Domain, request.Project, request.ID)
 }
 
 //Total return kv record number
diff --git a/server/service/service.go b/server/service/service.go
index be19bff..dd949bc 100644
--- a/server/service/service.go
+++ b/server/service/service.go
@@ -50,7 +50,7 @@ type KV interface {
 	//FindManyAndDelete deletes multiple kvs and return the deleted kv list as these appeared before deletion
 	FindManyAndDelete(ctx context.Context, kvIDs []string, domain, project string) ([]*model.KVDoc, error)
 	//Get return kv by id
-	Get(ctx context.Context, domain, project, id string, options ...FindOption) (*model.KVDoc, error)
+	Get(ctx context.Context, request *model.GetKVRequest) (*model.KVDoc, error)
 	//KV is a resource of kie, this api should return kv resource number by domain id
 	Total(ctx context.Context, domain string) (int64, error)
 }