Skip to content

Commit

Permalink
adapt the storagespace methods and tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@micbar
micbar committed Jul 6, 2022
1 parent 31b709b commit db9bd08
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 90 deletions.
61 changes: 28 additions & 33 deletions pkg/storagespace/storagespace.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,26 +42,28 @@ var (
// SplitID splits a storage space ID into a provider ID and a node ID.
// The accepted formats results of the storage space ID and respective results
// are:
// <providerid>$<spaceid>!<nodeid> -> <providerid>$<spaceid>, <nodeid>
// <providerid>$<spaceid> -> <providerid>$<spaceid>, <spaceid>
// <spaceid> -> <spaceid>, <spaceid>
func SplitID(ssid string) (storageid, nodeid string, err error) {
// <storageid>$<spaceid>!<nodeid> -> <storageid>, <spaceid>, <nodeid>
// <storageid>$<spaceid> -> <storageid>, <spaceid>, ""
// <spaceid> -> "", <spaceid>, ""
func SplitID(ssid string) (storageid, spaceid, nodeid string, err error) {
if ssid == "" {
return "", "", errors.Wrap(ErrInvalidSpaceID, "can't split empty storage space ID")
return "", "", "", errors.Wrap(ErrInvalidSpaceID, "can't split empty storage space ID")
}
parts := strings.SplitN(ssid, _idDelimiter, 2)

storageid, snid := SplitStorageID(ssid)
parts := strings.SplitN(snid, _idDelimiter, 2)
if len(parts) == 1 || parts[1] == "" {
_, sid := SplitStorageID(parts[0])
return parts[0], sid, nil
return storageid, parts[0], "", nil
}
return parts[0], parts[1], nil

return storageid, parts[0], parts[1], nil
}

// SplitStorageID splits a storage ID into the provider ID and the spaceID.
// SplitStorageID splits a storage ID into the storage ID and the spaceID.
// The accepted formats are:
// <providerid>$<spaceid> -> <providerid>, <spaceid>
// <storageid>$<spaceid> -> <storageid>, <spaceid>
// <spaceid> -> "", <spaceid>
func SplitStorageID(sid string) (providerID, spaceID string) {
func SplitStorageID(sid string) (storageID, spaceID string) {
parts := strings.SplitN(sid, _storageIDDelimiter, 2)
if len(parts) == 2 {
return parts[0], parts[1]
Expand All @@ -71,20 +73,23 @@ func SplitStorageID(sid string) (providerID, spaceID string) {

// FormatResourceID converts a ResourceId into the string format.
// The result format will look like:
// <storageid>!<opaqueid>
// <storageid>$<spaceid>!<opaqueid>
func FormatResourceID(sid provider.ResourceId) string {
return strings.Join([]string{sid.StorageId, sid.OpaqueId}, _idDelimiter)
if sid.OpaqueId == "" {
return FormatStorageID(sid.StorageId, sid.SpaceId)
}
return strings.Join([]string{FormatStorageID(sid.StorageId, sid.SpaceId), sid.OpaqueId}, _idDelimiter)
}

// FormatStorageID converts the provider ID and space ID into the string format.
// The result format will look like:
// <providerid>$<spaceid> or
// <storageid>$<spaceid> or
// <spaceid> in case the provider ID is empty.
func FormatStorageID(providerID, spaceID string) string {
if providerID == "" {
func FormatStorageID(storageID, spaceID string) string {
if storageID == "" {
return spaceID
}
return strings.Join([]string{providerID, spaceID}, _storageIDDelimiter)
return strings.Join([]string{storageID, spaceID}, _storageIDDelimiter)
}

// ParseID parses a storage space ID and returns a storageprovider ResourceId.
Expand All @@ -93,10 +98,11 @@ func FormatStorageID(providerID, spaceID string) string {
// <providerid>$<spaceid> -> <providerid>$<spaceid>, <spaceid>
// <spaceid> -> <spaceid>, <spaceid>
func ParseID(ssid string) (provider.ResourceId, error) {
sid, nid, err := SplitID(ssid)
sid, spid, oid, err := SplitID(ssid)
return provider.ResourceId{
StorageId: sid,
OpaqueId: nid,
SpaceId: spid,
OpaqueId: oid,
}, err
}

Expand Down Expand Up @@ -131,21 +137,10 @@ func ParseReference(sRef string) (provider.Reference, error) {
// "storage_id/path"
// "storage_id"
func FormatReference(ref *provider.Reference) (string, error) {
if ref == nil || ref.ResourceId == nil || ref.ResourceId.StorageId == "" {
if ref == nil || ref.ResourceId == nil || ref.ResourceId.SpaceId == "" {
return "", ErrInvalidSpaceReference
}
var ssid string
if ref.ResourceId.OpaqueId == "" {

ssid = ref.ResourceId.StorageId
} else {
var sb strings.Builder
// ssid == storage_id!opaque_id
sb.Grow(len(ref.ResourceId.StorageId) + len(ref.ResourceId.OpaqueId) + 1)
sb.WriteString(ref.ResourceId.StorageId)
sb.WriteString(_idDelimiter)
sb.WriteString(ref.ResourceId.OpaqueId)
ssid = sb.String()
}
ssid = FormatResourceID(*ref.ResourceId)
return path.Join(ssid, ref.Path), nil
}
162 changes: 106 additions & 56 deletions pkg/storagespace/storagespace_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,16 @@ func TestSplitStorageID(t *testing.T) {
expected []string
}{
{
"providerid" + _storageIDDelimiter + "storageid",
[]string{"storageid", "providerid"},
"storageid" + _storageIDDelimiter + "spaceid",
[]string{"storageid", "spaceid"},
},
{
"",
nil,
},
{
"storageid",
[]string{"storageid", ""},
"spaceid",
[]string{"", "spaceid"},
},
}

Expand All @@ -54,10 +54,10 @@ func TestSplitStorageID(t *testing.T) {
}
case len(tt.expected) != 2:
t.Error("testcase won't work with len(expected) != 2. Avoiding panic")
case spaceID != tt.expected[0]:
t.Errorf("StorageID doesn't match, expected '%s' got '%s'", tt.expected[0], spaceID)
case storageID != tt.expected[1]:
t.Errorf("ProviderID doesn't match, expected '%s' got '%s'", tt.expected[1], storageID)
case storageID != tt.expected[0]:
t.Errorf("StorageID doesn't match, expected '%s' got '%s'", tt.expected[0], storageID)
case spaceID != tt.expected[1]:
t.Errorf("ProviderID doesn't match, expected '%s' got '%s'", tt.expected[1], spaceID)
}
}

Expand All @@ -71,12 +71,12 @@ func TestParseID(t *testing.T) {
}{
{
"spaceid" + _idDelimiter + "opaqueid",
provider.ResourceId{StorageId: "spaceid", OpaqueId: "opaqueid"},
provider.ResourceId{SpaceId: "spaceid", OpaqueId: "opaqueid"},
nil,
},
{
"providerid" + _storageIDDelimiter + "spaceid" + _idDelimiter + "opaqueid",
provider.ResourceId{StorageId: "providerid$spaceid", OpaqueId: "opaqueid"},
"storageid" + _storageIDDelimiter + "spaceid" + _idDelimiter + "opaqueid",
provider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "opaqueid"},
nil,
},
{
Expand All @@ -86,7 +86,7 @@ func TestParseID(t *testing.T) {
},
{
"spaceid",
provider.ResourceId{StorageId: "spaceid", OpaqueId: "spaceid"},
provider.ResourceId{SpaceId: "spaceid"},
nil,
},
}
Expand All @@ -108,17 +108,17 @@ func TestParseID(t *testing.T) {
}

func TestFormatResourceID(t *testing.T) {
expected := "storageid" + _idDelimiter + "opaqueid"
wrapped := FormatResourceID(provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"})
expected := "spaceid" + _idDelimiter + "opaqueid"
wrapped := FormatResourceID(provider.ResourceId{SpaceId: "spaceid", OpaqueId: "opaqueid"})

if wrapped != expected {
t.Errorf("wrapped id doesn't have the expected format: got %s expected %s", wrapped, expected)
}
}

func TestFormatStorageID(t *testing.T) {
expected := "providerid" + _storageIDDelimiter + "spaceid"
wrapped := FormatStorageID("providerid", "spaceid")
expected := "storageid" + _storageIDDelimiter + "spaceid"
wrapped := FormatStorageID("storageid", "spaceid")

if wrapped != expected {
t.Errorf("wrapped id doesn't have the expected format: got %s expected %s", wrapped, expected)
Expand All @@ -127,38 +127,84 @@ func TestFormatStorageID(t *testing.T) {

func TestParseStorageSpaceReference(t *testing.T) {
tests := []struct {
sRef string
storageID string
nodeID string
relPath string
sRef string
resourceID *provider.ResourceId
relPath string
expectedErr error
}{
{
"1234!abcd/f1/f2",
"1234",
"abcd",
"1234$abcd!5678/f1/f2",
&provider.ResourceId{
StorageId: "1234",
SpaceId: "abcd",
OpaqueId: "5678",
},
"./f1/f2",
nil,
},
{
"1234!abcd",
"1234",
"abcd",
&provider.ResourceId{
SpaceId: "1234",
OpaqueId: "abcd",
},
".",
nil,
},
{
"01234$56789!abcd",
"01234$56789",
"abcd",
&provider.ResourceId{
StorageId: "01234",
SpaceId: "56789",
OpaqueId: "abcd",
},
".",
nil,
},
{
"",
nil,
"",
ErrInvalidSpaceID,
},
{
"01234$abcd",
&provider.ResourceId{
StorageId: "01234",
SpaceId: "abcd",
OpaqueId: "",
},
".",
nil,
},
{
"01234$!5678",
&provider.ResourceId{
StorageId: "01234",
SpaceId: "",
OpaqueId: "5678",
},
".",
nil,
},
{
"/f1/f2",
nil,
"",
ErrInvalidSpaceID,
},
}
for _, tt := range tests {
ref, _ := ParseReference(tt.sRef)
ref, err := ParseReference(tt.sRef)

if ref.ResourceId.StorageId != tt.storageID {
t.Errorf("Expected storageId %s got %s", tt.storageID, ref.ResourceId.StorageId)
if !errors.Is(err, tt.expectedErr) {
t.Errorf("Expected error %s got %s", tt.expectedErr, err)
}
if ref.ResourceId.OpaqueId != tt.nodeID {
t.Errorf("Expected OpaqueId %s got %s", tt.nodeID, ref.ResourceId.OpaqueId)
if ref.ResourceId != nil && !utils.ResourceIDEqual(ref.ResourceId, tt.resourceID) {
t.Errorf("Expected ResourceID %s got %s", tt.resourceID, ref.ResourceId)
}
if ref.ResourceId == nil && tt.resourceID != nil {
t.Errorf("Expected ResourceID to be %s got %s", tt.resourceID, ref.ResourceId)
}
if ref.Path != tt.relPath {
t.Errorf("Expected path %s got %s", tt.relPath, ref.Path)
Expand Down Expand Up @@ -187,24 +233,28 @@ func TestFormatStorageSpaceReference(t *testing.T) {
expectedErr: ErrInvalidSpaceReference,
},
{
ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}, Path: "path"},
expected: "storageid/path",
ref: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid"}, Path: "path"},
expected: "spaceid/path",
},
{
ref: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid", OpaqueId: "opaqueid"}, Path: "path"},
expected: "spaceid!opaqueid/path",
},
{
ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "path"},
expected: "storageid!opaqueid/path",
ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "opaqueid"}, Path: "path"},
expected: "storageid$spaceid!opaqueid/path",
},
{
ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "."},
expected: "storageid!opaqueid",
ref: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid", OpaqueId: "opaqueid"}, Path: "."},
expected: "spaceid!opaqueid",
},
{
ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}, Path: "."},
expected: "storageid",
ref: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid"}, Path: "."},
expected: "spaceid",
},
{
ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}},
expected: "storageid",
ref: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid"}},
expected: "spaceid",
},
}

Expand All @@ -225,32 +275,32 @@ func TestFormatAndParseReference(t *testing.T) {
expected *provider.Reference
}{
{
orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}, Path: "./path"},
expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "storageid"}, Path: "./path"},
orig: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid"}, Path: "./path"},
expected: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid"}, Path: "./path"},
},
{
orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid$spaceid"}, Path: "./path"},
expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid$spaceid", OpaqueId: "spaceid"}, Path: "./path"},
orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", SpaceId: "spaceid"}, Path: "./path"},
expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", SpaceId: "spaceid"}, Path: "./path"},
},
{
orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "./path"},
expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "./path"},
orig: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid", OpaqueId: "opaqueid"}, Path: "./path"},
expected: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid", OpaqueId: "opaqueid"}, Path: "./path"},
},
{
orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid$spaceid", OpaqueId: "opaqueid"}, Path: "./path"},
expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid$spaceid", OpaqueId: "opaqueid"}, Path: "./path"},
orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "opaqueid"}, Path: "./path"},
expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "opaqueid"}, Path: "./path"},
},
{
orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "."},
expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "."},
orig: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid", OpaqueId: "opaqueid"}, Path: "."},
expected: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid", OpaqueId: "opaqueid"}, Path: "."},
},
{
orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}, Path: "."},
expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "storageid"}, Path: "."},
orig: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid"}, Path: "."},
expected: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid"}, Path: "."},
},
{
orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}},
expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "storageid"}, Path: "."},
orig: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid"}},
expected: &provider.Reference{ResourceId: &provider.ResourceId{SpaceId: "spaceid"}, Path: "."},
},
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func GroupEqual(u, v *grouppb.GroupId) bool {

// ResourceIDEqual returns whether two resources have the same field values.
func ResourceIDEqual(u, v *provider.ResourceId) bool {
return u != nil && v != nil && u.StorageId == v.StorageId && u.OpaqueId == v.OpaqueId
return u != nil && v != nil && u.StorageId == v.StorageId && u.OpaqueId == v.OpaqueId && u.SpaceId == v.SpaceId
}

// ResourceEqual returns whether two resources have the same field values.
Expand Down

0 comments on commit db9bd08

Please sign in to comment.