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 committed Nov 25, 2019
1 parent 6e45d38 commit 0393f81
Show file tree
Hide file tree
Showing 7 changed files with 221 additions and 10 deletions.
29 changes: 29 additions & 0 deletions pkg/api/controllers/fileshare.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,35 @@ func (f *FileSharePortal) UpdateFileShare() {
return
}

if fshare.Name == "" {
errMsg := fmt.Sprintf("empty fileshare name is not allowed. Please give valid name.")
log.Error(errMsg)
f.ErrorHandle(model.ErrorBadRequest, errMsg)
return
}
if len(fshare.Name) > 255 {
errMsg := fmt.Sprintf("fileshare name length should not be more than 255 characters. input name length is : %d", len(fshare.Name))
log.Error(errMsg)
f.ErrorHandle(model.ErrorBadRequest, errMsg)
return
}
reg, err := utils.Special_Character_Regex_Match_Pattern()
if err != nil {
log.Error(err)
return
}
if reg.MatchString(fshare.Name) {
errMsg := fmt.Sprintf("invalid fileshare name because it has some special characters")
log.Error(errMsg)
f.ErrorHandle(model.ErrorBadRequest, errMsg)
return
}
if reg.MatchString(fshare.Description) {
errMsg := fmt.Sprintf("invalid fileshare description and it has some special characters")
log.Error(errMsg)
f.ErrorHandle(model.ErrorBadRequest, errMsg)
return
}
fshare.Id = id
result, err := db.C.UpdateFileShare(c.GetContext(f.Ctx), &fshare)
if err != nil {
Expand Down
89 changes: 89 additions & 0 deletions pkg/api/controllers/fileshare_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/opensds/opensds/pkg/db"
"github.com/opensds/opensds/pkg/model"
pb "github.com/opensds/opensds/pkg/model/proto"
"github.com/opensds/opensds/pkg/utils"
. "github.com/opensds/opensds/testutils/collection"
ctrtest "github.com/opensds/opensds/testutils/controller/testing"
dbtest "github.com/opensds/opensds/testutils/db/testing"
Expand Down Expand Up @@ -208,6 +209,94 @@ func TestUpdateFileShare(t *testing.T) {
assertTestResult(t, &output, &expected)
})

t.Run("Should return 400 empty fileshare name is not allowed", func(t *testing.T) {
var jsonStr = []byte(`{
"name":"",
"description":"fake fileshare"
}`)
share := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&share)
mockClient := new(dbtest.Client)
mockClient.On("UpdateFileShare", c.NewAdminContext(), share.Id, &share).
Return(&expected, nil)
db.C = mockClient

r, _ := http.NewRequest("PUT", "/v1beta/file/shares/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
w := httptest.NewRecorder()
r.Header.Set("Content-Type", "application/JSON")
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
httpCtx.Input.SetData("context", c.NewAdminContext())
})
beego.BeeApp.Handlers.ServeHTTP(w, r)
assertTestResult(t, w.Code, 400)
})

t.Run("Should return 400 fileshare name length should not be more than 255 characters", func(t *testing.T) {
var jsonStr = []byte(`{
"description":"fake fileshare"
}`)
share := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
share.Name = utils.RandomString(258)
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&share)
mockClient := new(dbtest.Client)
mockClient.On("UpdateFileShare", c.NewAdminContext(), share.Id, &share).
Return(&expected, nil)
db.C = mockClient

r, _ := http.NewRequest("PUT", "/v1beta/file/shares/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
w := httptest.NewRecorder()
r.Header.Set("Content-Type", "application/JSON")
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
httpCtx.Input.SetData("context", c.NewAdminContext())
})
beego.BeeApp.Handlers.ServeHTTP(w, r)
assertTestResult(t, w.Code, 400)
})

t.Run("Should return 400 invalid fileshare name because it has some special characters", func(t *testing.T) {
var jsonStr = []byte(`{
"description":"fake fileshare"
}`)
share := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
share.Name = "#Share !$!test"
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&share)
mockClient := new(dbtest.Client)
mockClient.On("UpdateFileShare", c.NewAdminContext(), share.Id, &share).
Return(&expected, nil)
db.C = mockClient

r, _ := http.NewRequest("PUT", "/v1beta/file/shares/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
w := httptest.NewRecorder()
r.Header.Set("Content-Type", "application/JSON")
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
httpCtx.Input.SetData("context", c.NewAdminContext())
})
beego.BeeApp.Handlers.ServeHTTP(w, r)
assertTestResult(t, w.Code, 400)
})

t.Run("Should return 400 invalid fileshare description and it has some special characters", func(t *testing.T) {
var jsonStr = []byte(`{
"name": "sampletestfileshare01",
"description":"#Share !$!test"
}`)
share := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&share)
mockClient := new(dbtest.Client)
mockClient.On("UpdateFileShare", c.NewAdminContext(), share.Id, &share).
Return(&expected, nil)
db.C = mockClient

r, _ := http.NewRequest("PUT", "/v1beta/file/shares/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
w := httptest.NewRecorder()
r.Header.Set("Content-Type", "application/JSON")
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
httpCtx.Input.SetData("context", c.NewAdminContext())
})
beego.BeeApp.Handlers.ServeHTTP(w, r)
assertTestResult(t, w.Code, 400)
})

t.Run("Should return 500 if update file share with bad request", func(t *testing.T) {
fileshare := model.FileShareSpec{BaseModel: &model.BaseModel{}}
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&fileshare)
Expand Down
27 changes: 24 additions & 3 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 @@ -182,10 +181,32 @@ func CreateFileShareDBEntry(ctx *c.Context, in *model.FileShareSpec) (*model.Fil
return nil, errors.New(errMsg)
}

shares, err := db.C.ListFileShares(ctx)
if err != nil {
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)
}
}
}

// validate the description
reg, err := regexp.Compile("[^a-zA-Z0-9 ]+")
reg, err := utils.Special_Character_Regex_Match_Pattern()
if err != nil {
errMsg := fmt.Sprintf("regex compilation for file share description validation failed")
log.Error(err)
return nil, err
}
if reg.MatchString(in.Name) {
errMsg := fmt.Sprintf("invalid fileshare name because it has some special characters")
log.Error(errMsg)
return nil, errors.New(errMsg)
}
Expand Down
54 changes: 52 additions & 2 deletions pkg/api/util/db_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ func TestCreateFileShareSnapshotDBEntry(t *testing.T) {
func TestCreateFileShareDBEntry(t *testing.T) {
var in = &model.FileShareSpec{
BaseModel: &model.BaseModel{},
Name: "sample-fileshare-01",
Name: "samplefileshare01",
Description: "This is a sample fileshare for testing",
Size: int64(1),
ProfileId: "b3585ebe-c42c-120g-b28e-f373746a71ca",
Expand All @@ -390,6 +390,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 +405,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,6 +417,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)
Expand All @@ -426,6 +429,7 @@ func TestCreateFileShareDBEntry(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)
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
db.C = mockClient

_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
Expand All @@ -438,6 +442,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
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)
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
db.C = mockClient

_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
Expand All @@ -450,6 +455,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
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)
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
db.C = mockClient

var expected = &SampleFileShares[0]
Expand All @@ -465,6 +471,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
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)
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
db.C = mockClient

var expected = &SampleFileShares[0]
Expand All @@ -480,6 +487,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
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)
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
db.C = mockClient

var expected = &SampleFileShares[0]
Expand All @@ -495,6 +503,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
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)
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
db.C = mockClient

var expected = &SampleFileShares[0]
Expand All @@ -510,6 +519,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
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)
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
db.C = mockClient

_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
Expand All @@ -522,6 +532,19 @@ func TestCreateFileShareDBEntry(t *testing.T) {
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)
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, 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))
assertTestResult(t, err.Error(), expectedError)
})

t.Run("Special characters in file share name are not allowed", func(t *testing.T) {
in.Size, in.Name, in.ProfileId, in.Description = int64(1), "#Share !$!test", "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(nil, nil)
db.C = mockClient

_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
Expand All @@ -530,15 +553,42 @@ func TestCreateFileShareDBEntry(t *testing.T) {
})

t.Run("Special characters in file share description are not allowed", func(t *testing.T) {
in.Size, in.Name, in.ProfileId, in.Description = int64(1), "sample-fileshare-01", "b3585ebe-c42c-120g-b28e-f373746a71ca", "#FileShare Code!$! test"
in.Size, in.Name, in.ProfileId, in.Description = int64(1), "test tmp", "b3585ebe-c42c-120g-b28e-f373746a71ca", "#Share !$!test"
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 := "invalid fileshare description and it has some special characters"
assertTestResult(t, err.Error(), expectedError)
})

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), "samplefileshare01", "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 := "file share name already exists"
assertTestResult(t, err.Error(), expectedError)
})

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), "samplefileshare05", "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 := "file share id already exists"
assertTestResult(t, err.Error(), expectedError)
})
}

func TestDeleteFileShareDBEntry(t *testing.T) {
Expand Down
22 changes: 22 additions & 0 deletions pkg/utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"reflect"
"sort"
"strings"
"regexp"
"time"

"github.com/opensds/opensds/pkg/utils/constants"
Expand Down Expand Up @@ -327,3 +328,24 @@ func WaitForCondition(f func() (bool, error), interval, timeout time.Duration) e
}
return fmt.Errorf("wait for condition timeout")
}

func RandomString(n int) string {
var letter = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789")

b := make([]rune, n)
for i := range b {
b[i] = letter[rand.Intn(len(letter))]
}
return string(b)
}

func Special_Character_Regex_Match_Pattern() (*regexp.Regexp, error) {
reg, err := regexp.Compile(`[^a-zA-Z0-9 ]+`)
if err != nil {
errMsg := fmt.Sprintf("regex compilation validation failed")
log.Error(errMsg)
return nil, errors.New(errMsg)
} else {
return reg, nil
}
}
6 changes: 3 additions & 3 deletions test/e2e/e2e_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ func TestUpdateVolumeGroup(t *testing.T) {
func TestCreateFileShare(t *testing.T) {
t.Log("Start creating file share...")
var body = &model.FileShareSpec{
Name: "share_test",
Name: "sharetest",
Description: "This is a test",
Size: int64(1),
}
Expand Down Expand Up @@ -743,7 +743,7 @@ func prepareGetFileShare(t *testing.T) (string, error) {
}

for _, item := range filesharelst {
if item.Name == "share_test" {
if item.Name == "sharetest" {
fileshare_id = item.Id
}
}
Expand Down Expand Up @@ -792,7 +792,7 @@ func prepareGetShare(t *testing.T) (string, error) {
}

for _, item := range filesharelst {
if item.Name == "share_test" {
if item.Name == "sharetest" {
fileshare_id = item.Id
}
}
Expand Down

0 comments on commit 0393f81

Please sign in to comment.