From 6055098ff2f596bd2574683695bd83ffc2e09573 Mon Sep 17 00:00:00 2001 From: Shruthi-1MN Date: Wed, 20 Nov 2019 16:40:47 +0530 Subject: [PATCH] File share name fixes --- openapi-spec/swagger.yaml | 32 +++++--- pkg/api/controllers/fileshare.go | 12 +++ pkg/api/controllers/fileshare_test.go | 2 + pkg/api/util/db.go | 36 ++++----- pkg/api/util/db_test.go | 109 ++++---------------------- 5 files changed, 66 insertions(+), 125 deletions(-) diff --git a/openapi-spec/swagger.yaml b/openapi-spec/swagger.yaml index de628705e..9ef0d352e 100755 --- a/openapi-spec/swagger.yaml +++ b/openapi-spec/swagger.yaml @@ -2294,12 +2294,7 @@ components: content: application/json: schema: - type: object - properties: - name: - type: string - description: - type: string + $ref: '#/components/schemas/FileShareUpdateSpec' FileShareSnapshotCreateSpec: content: application/json: @@ -2676,8 +2671,8 @@ components: name: type: string minLength: 1 - maxLength: 255 - pattern: '[A-Za-z0-9_-]' + maxLength: 102 + pattern: '^[\w\- ]+$' description: type: string size: @@ -2689,7 +2684,26 @@ components: default: default profileId: type: string - + FileShareUpdateSpec: + type: object + properties: + name: + type: string + minLength: 1 + maxLength: 102 + pattern: '^[\w\- ]+$' + description: + type: string + pattern: '^[\w\- ]+$' + size: + type: integer + format: int64 + example: 2 + availabilityZone: + type: string + default: default + profileId: + type: string FileShareRespSpec: description: >- FileShare is a file system created by nfs southbound driver, it can be diff --git a/pkg/api/controllers/fileshare.go b/pkg/api/controllers/fileshare.go index f55843ef2..a4d11cee5 100644 --- a/pkg/api/controllers/fileshare.go +++ b/pkg/api/controllers/fileshare.go @@ -370,6 +370,18 @@ func (f *FileSharePortal) UpdateFileShare() { f.ErrorHandle(model.ErrorBadRequest, errMsg) return } + + fshare1, err := db.C.GetFileShare(c.GetContext(f.Ctx), id) + if err != nil { + errMsg := fmt.Sprintf("fileshare %s not found: %s", id, err.Error()) + f.ErrorHandle(model.ErrorNotFound, errMsg) + return + } + if fshare1.Status == "error" { + errMsg := fmt.Sprintf("can't update file share %s because it's status is error", fshare1.Id) + f.ErrorHandle(model.ErrorBadRequest, errMsg) + return + } fshare.Id = id result, err := db.C.UpdateFileShare(c.GetContext(f.Ctx), &fshare) diff --git a/pkg/api/controllers/fileshare_test.go b/pkg/api/controllers/fileshare_test.go index 03cd8fbf6..00bdcf4a3 100644 --- a/pkg/api/controllers/fileshare_test.go +++ b/pkg/api/controllers/fileshare_test.go @@ -192,6 +192,7 @@ func TestUpdateFileShare(t *testing.T) { fileshare := model.FileShareSpec{BaseModel: &model.BaseModel{}} json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&fileshare) mockClient := new(dbtest.Client) + mockClient.On("GetFileShare", c.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(&expected, nil) mockClient.On("UpdateFileShare", c.NewAdminContext(), &fileshare).Return(&expected, nil) db.C = mockClient @@ -212,6 +213,7 @@ func TestUpdateFileShare(t *testing.T) { fileshare := model.FileShareSpec{BaseModel: &model.BaseModel{}} json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&fileshare) mockClient := new(dbtest.Client) + mockClient.On("GetFileShare", c.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(&expected, nil) mockClient.On("UpdateFileShare", c.NewAdminContext(), &fileshare).Return(nil, errors.New("db error")) db.C = mockClient diff --git a/pkg/api/util/db.go b/pkg/api/util/db.go index 74ce0bee8..d2bae9586 100644 --- a/pkg/api/util/db.go +++ b/pkg/api/util/db.go @@ -22,7 +22,6 @@ import ( "errors" "fmt" "net" - "regexp" "strings" "time" @@ -170,28 +169,23 @@ func CreateFileShareDBEntry(ctx *c.Context, in *model.FileShareSpec) (*model.Fil if in.UpdatedAt == "" { in.UpdatedAt = time.Now().Format(constants.TimeFormat) } - //validate the name - if in.Name == "" { - errMsg := fmt.Sprintf("empty fileshare name is not allowed. Please give valid name.") - log.Error(errMsg) - return nil, errors.New(errMsg) - } - if len(in.Name) > 255 { - errMsg := fmt.Sprintf("fileshare name length should not be more than 255 characters. input name length is : %d", len(in.Name)) - log.Error(errMsg) - return nil, errors.New(errMsg) - } - reg, err := regexp.Compile("^[a-zA-Z0-9_-]+$") + shares, err := db.C.ListFileShares(ctx) if err != nil { - errMsg := fmt.Sprintf("regex compilation for file name validation failed") - log.Error(errMsg) - return nil, errors.New(errMsg) - } - if reg.MatchString(in.Name) == false { - errMsg := fmt.Sprintf("invalid fileshare name it only contain english char and number : %v", in.Name) - log.Error(errMsg) - return nil, errors.New(errMsg) + return nil, err + } else { + for _, fshare := range shares { + if fshare.Name == in.Name { + errMsg := fmt.Sprintf("file share name already exists") + log.Error(errMsg) + return nil, errors.New(errMsg) + } + if fshare.Id == in.Id { + errMsg := fmt.Sprintf("file share id already exists") + log.Error(errMsg) + return nil, errors.New(errMsg) + } + } } in.UserId = ctx.UserId diff --git a/pkg/api/util/db_test.go b/pkg/api/util/db_test.go index 7a095c80b..97d2400bb 100644 --- a/pkg/api/util/db_test.go +++ b/pkg/api/util/db_test.go @@ -17,11 +17,8 @@ package util import ( "fmt" "reflect" - "strconv" "testing" - "github.com/opensds/opensds/pkg/utils" - "github.com/opensds/opensds/pkg/context" "github.com/opensds/opensds/pkg/db" "github.com/opensds/opensds/pkg/model" @@ -390,6 +387,7 @@ func TestCreateFileShareDBEntry(t *testing.T) { t.Run("Everything should work well", func(t *testing.T) { mockClient := new(dbtest.Client) mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) + mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil) db.C = mockClient var expected = &SampleFileShares[0] @@ -404,6 +402,7 @@ func TestCreateFileShareDBEntry(t *testing.T) { in.Size = int64(-2) mockClient := new(dbtest.Client) mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) + mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil) db.C = mockClient _, err := CreateFileShareDBEntry(context.NewAdminContext(), in) @@ -415,6 +414,7 @@ func TestCreateFileShareDBEntry(t *testing.T) { in.ProfileId = "" mockClient := new(dbtest.Client) mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) + mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil) db.C = mockClient _, err := CreateFileShareDBEntry(context.NewAdminContext(), in) @@ -422,110 +422,29 @@ func TestCreateFileShareDBEntry(t *testing.T) { assertTestResult(t, err.Error(), expectedError) }) - t.Run("Empty file share name is allowed", func(t *testing.T) { - in.Size, in.Name, in.ProfileId = int64(1), "", "b3585ebe-c42c-120g-b28e-f373746a71ca" - mockClient := new(dbtest.Client) - mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) - db.C = mockClient - - _, err := CreateFileShareDBEntry(context.NewAdminContext(), in) - expectedError := "empty fileshare name is not allowed. Please give valid name." - assertTestResult(t, err.Error(), expectedError) - }) - - t.Run("File share name length equal to 0 character are not allowed", func(t *testing.T) { - in.Name = utils.RandSeqWithAlnum(0) - in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca" - mockClient := new(dbtest.Client) - mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) - db.C = mockClient - - _, err := CreateFileShareDBEntry(context.NewAdminContext(), in) - expectedError := "empty fileshare name is not allowed. Please give valid name." - assertTestResult(t, err.Error(), expectedError) - }) - - t.Run("File share name length equal to 1 character are allowed", func(t *testing.T) { - in.Name = utils.RandSeqWithAlnum(1) - in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca" - mockClient := new(dbtest.Client) - mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) - db.C = mockClient - - var expected = &SampleFileShares[0] - result, err := CreateFileShareDBEntry(context.NewAdminContext(), in) - if err != nil { - t.Errorf("failed to create fileshare err is %v\n", err) - } - assertTestResult(t, result, expected) - }) - - t.Run("File share name length equal to 10 characters are allowed", func(t *testing.T) { - in.Name = utils.RandSeqWithAlnum(10) - in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca" - mockClient := new(dbtest.Client) - mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) - db.C = mockClient - - var expected = &SampleFileShares[0] - result, err := CreateFileShareDBEntry(context.NewAdminContext(), in) - if err != nil { - t.Errorf("failed to create fileshare err is %v\n", err) - } - assertTestResult(t, result, expected) - }) - - t.Run("File share name length equal to 254 characters are allowed", func(t *testing.T) { - in.Name = utils.RandSeqWithAlnum(254) - in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca" - mockClient := new(dbtest.Client) - mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) - db.C = mockClient - - var expected = &SampleFileShares[0] - result, err := CreateFileShareDBEntry(context.NewAdminContext(), in) - if err != nil { - t.Errorf("failed to create fileshare err is %v\n", err) - } - assertTestResult(t, result, expected) - }) - - t.Run("File share name length equal to 255 characters are allowed", func(t *testing.T) { - in.Name = utils.RandSeqWithAlnum(255) - in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca" - mockClient := new(dbtest.Client) - mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) - db.C = mockClient - - var expected = &SampleFileShares[0] - result, err := CreateFileShareDBEntry(context.NewAdminContext(), in) - if err != nil { - t.Errorf("failed to create fileshare err is %v\n", err) - } - assertTestResult(t, result, expected) - }) - - t.Run("File share name length more than 255 characters are not allowed", func(t *testing.T) { - in.Name = utils.RandSeqWithAlnum(256) - in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca" + t.Run("Duplicate file share name are not allowed", func(t *testing.T) { + var sampleshares = []*model.FileShareSpec{&SampleFileShares[0]} + in.Size, in.Name, in.ProfileId, in.Description = int64(1), "sample-fileshare-01", "b3585ebe-c42c-120g-b28e-f373746a71ca", "File share test" mockClient := new(dbtest.Client) mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) + mockClient.On("ListFileShares", context.NewAdminContext()).Return(sampleshares, nil) db.C = mockClient _, err := CreateFileShareDBEntry(context.NewAdminContext(), in) - expectedError := "fileshare name length should not be more than 255 characters. input name length is : " + strconv.Itoa(len(in.Name)) + expectedError := "file share name already exists" assertTestResult(t, err.Error(), expectedError) }) - t.Run("File share name length more than 255 characters are not allowed", func(t *testing.T) { - in.Name = utils.RandSeqWithAlnum(257) - in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca" + t.Run("Overwritting existing file share resource are not allowed", func(t *testing.T) { + var sampleshares = []*model.FileShareSpec{&SampleFileShares[0]} + in.Size, in.Name, in.ProfileId, in.Description, in.Id = int64(1), "sample-fileshare-05", "b3585ebe-c42c-120g-b28e-f373746a71ca", "File share test", "d2975ebe-d82c-430f-b28e-f373746a71ca" mockClient := new(dbtest.Client) mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil) + mockClient.On("ListFileShares", context.NewAdminContext()).Return(sampleshares, nil) db.C = mockClient _, err := CreateFileShareDBEntry(context.NewAdminContext(), in) - expectedError := "fileshare name length should not be more than 255 characters. input name length is : " + strconv.Itoa(len(in.Name)) + expectedError := "file share id already exists" assertTestResult(t, err.Error(), expectedError) }) } @@ -811,4 +730,4 @@ func TestDeleteFileShareSnapshotDBEntry(t *testing.T) { expectedError := fmt.Sprintf("only the fileshare snapshot with the status available, error, error_deleting can be deleted, the fileshare status is %s", in.Status) assertTestResult(t, err.Error(), expectedError) }) -} +} \ No newline at end of file