Skip to content

Commit

Permalink
File share name fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@Shruthi-1MN
Shruthi-1MN authored and Shruthi-1MN committed Jan 3, 2020
1 parent f8581c3 commit d5091b2
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 125 deletions.
32 changes: 23 additions & 9 deletions openapi-spec/swagger.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -2676,8 +2671,8 @@ components:
name:
type: string
minLength: 1
maxLength: 255
pattern: '[A-Za-z0-9_-]'
maxLength: 103
pattern: '^[\w\- ]+$'
description:
type: string
size:
Expand All @@ -2689,7 +2684,26 @@ components:
default: default
profileId:
type: string

FileShareUpdateSpec:
type: object
properties:
name:
type: string
minLength: 1
maxLength: 103
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
Expand Down
12 changes: 12 additions & 0 deletions pkg/api/controllers/fileshare.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/controllers/fileshare_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
36 changes: 15 additions & 21 deletions pkg/api/util/db.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"errors"
"fmt"
"net"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -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
Expand Down
109 changes: 14 additions & 95 deletions pkg/api/util/db_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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]
Expand All @@ -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)
Expand All @@ -415,117 +414,37 @@ 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)
expectedError := "profile id can not be empty when creating fileshare in db!"
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)
})
}
Expand Down Expand Up @@ -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)
})
}
}

0 comments on commit d5091b2

Please sign in to comment.