File share name fixes

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

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)

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
 

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

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,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)
 	})
 }
@@ -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)
 	})
-}
+}