diff --git a/changelog/unreleased/fix-tags-pkg.md b/changelog/unreleased/fix-tags-pkg.md new file mode 100644 index 0000000000..af1946bfd5 --- /dev/null +++ b/changelog/unreleased/fix-tags-pkg.md @@ -0,0 +1,5 @@ +Enhancement: Fix tag pkg + +`tags` pkg needed an option to build the tags struct from a slice of tags. Here it is + +https://github.com/cs3org/reva/pull/3564 diff --git a/pkg/tags/tags.go b/pkg/tags/tags.go index c6bb765d5c..5113b3f8cf 100644 --- a/pkg/tags/tags.go +++ b/pkg/tags/tags.go @@ -39,37 +39,38 @@ type Tags struct { numtags int } -// FromList creates a Tags struct from a list of tags -func FromList(s string) *Tags { - t := &Tags{sep: _tagsep, maxtags: _maxtags, exists: make(map[string]bool)} - t.t = t.addTags(s) +// New creates a Tag struct from a slice of tags, e.g. ["tag1", "tag2"] or a list of tags, e.g. "tag1,tag2" +func New(ts ...string) *Tags { + t := &Tags{sep: _tagsep, maxtags: _maxtags, exists: make(map[string]bool), t: make([]string, 0)} + t.addTags(ts) return t } -// AddList appends a list of new tags and returns true if at least one was appended -func (t *Tags) AddList(s string) bool { - tags := t.addTags(s) - t.t = append(tags, t.t...) - return len(tags) > 0 +// Add appends a list of new tags and returns true if at least one was appended +func (t *Tags) Add(ts ...string) bool { + return len(t.addTags(ts)) > 0 } -// RemoveList removes a list of tags and returns true if at least one was removed -func (t *Tags) RemoveList(s string) bool { +// Remove removes a list of tags and returns true if at least one was removed +func (t *Tags) Remove(s ...string) bool { var removed bool - for _, tag := range strings.Split(s, t.sep) { - if !t.exists[tag] { - continue - } - for i, tt := range t.t { - if tt == tag { - t.t = append(t.t[:i], t.t[i+1:]...) - break + for _, tt := range s { + for _, tag := range strings.Split(tt, t.sep) { + if !t.exists[tag] { + continue } - } - delete(t.exists, tag) - removed = true + for i, tt := range t.t { + if tt == tag { + t.t = append(t.t[:i], t.t[i+1:]...) + break + } + } + + delete(t.exists, tag) + removed = true + } } return removed } @@ -85,28 +86,31 @@ func (t *Tags) AsSlice() []string { } // adds the tags and returns a list of added tags -func (t *Tags) addTags(s string) []string { +func (t *Tags) addTags(s []string) []string { added := make([]string, 0) - for _, tag := range strings.Split(s, t.sep) { - if tag == "" { - // ignore empty tags - continue - } + for _, tt := range s { + for _, tag := range strings.Split(tt, t.sep) { + if tag == "" { + // ignore empty tags + continue + } - if t.exists[tag] { - // tag is already existing - continue - } + if t.exists[tag] { + // tag is already existing + continue + } - if t.numtags >= t.maxtags { - // max number of tags reached. We return silently without warning anyone - break - } + if t.numtags >= t.maxtags { + // max number of tags reached. We return silently without warning anyone + break + } - added = append(added, tag) - t.exists[tag] = true - t.numtags++ + added = append(added, tag) + t.exists[tag] = true + t.numtags++ + } } + t.t = append(added, t.t...) return added } diff --git a/pkg/tags/tags_test.go b/pkg/tags/tags_test.go index 0e91eef485..e80acd2663 100644 --- a/pkg/tags/tags_test.go +++ b/pkg/tags/tags_test.go @@ -105,10 +105,95 @@ func TestAddTags(t *testing.T) { } for _, tc := range testCases { - ts := FromList(tc.InitialTags) + ts := New(tc.InitialTags) ts.maxtags = 10 - added := ts.AddList(tc.TagsToAdd) + added := ts.Add(tc.TagsToAdd) + require.Equal(t, tc.ExpectNoTagsAdded, !added, tc.Alias) + require.Equal(t, tc.ExpectedTags, ts.AsList(), tc.Alias) + require.Equal(t, strings.Split(tc.ExpectedTags, ","), ts.AsSlice(), tc.Alias) + } + +} + +func TestAddTagsSlices(t *testing.T) { + testCases := []struct { + Alias string + InitialTags []string + TagsToAdd []string + ExpectedTags string + ExpectNoTagsAdded bool + }{ + { + Alias: "add one tag", + InitialTags: []string{}, + TagsToAdd: []string{"tag1"}, + ExpectedTags: "tag1", + }, + { + Alias: "add multiple tags", + InitialTags: []string{"a", "b", "c"}, + TagsToAdd: []string{"c,d,e"}, + ExpectedTags: "d,e,a,b,c", + }, + { + Alias: "no new tags", + InitialTags: []string{"a", "b", "c", "d", "e", "f"}, + TagsToAdd: []string{"c", "d", "e"}, + ExpectedTags: "a,b,c,d,e,f", + ExpectNoTagsAdded: true, + }, + { + Alias: "ignore duplicate tags", + InitialTags: []string{"a"}, + TagsToAdd: []string{"b", "b", "b", "b", "a", "a", "a", "a", "a"}, + ExpectedTags: "b,a", + }, + { + Alias: "stop adding when maximum is reached", + InitialTags: []string{"a", "b", "c", "d", "e", "f", "g", "h", "i"}, + TagsToAdd: []string{"j", "k", "l", "m"}, + ExpectedTags: "j,a,b,c,d,e,f,g,h,i", + }, + { + Alias: "don't do anything when already maxed", + InitialTags: []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"}, + TagsToAdd: []string{"k", "l", "m", "n", "o", "p"}, + ExpectedTags: "a,b,c,d,e,f,g,h,i,j", + ExpectNoTagsAdded: true, + }, + { + Alias: "special characters are allowed", + InitialTags: []string{"old tag"}, + TagsToAdd: []string{"new tag", "bettertag!"}, + ExpectedTags: "new tag,bettertag!,old tag", + }, + { + Alias: "empty tags are ignored", + InitialTags: []string{"tag1"}, + TagsToAdd: []string{"tag2", "", "tag3"}, + ExpectedTags: "tag2,tag3,tag1", + }, + { + Alias: "empty tags are not ignored if there are no new tags", + InitialTags: []string{"tag1"}, + TagsToAdd: []string{"", "", "", "tag1", "", ""}, + ExpectedTags: "tag1", + ExpectNoTagsAdded: true, + }, + { + Alias: "condition hold for initial tags too", + InitialTags: []string{"tag1", "tag1", "", "tag3", ""}, + TagsToAdd: []string{"tag2"}, + ExpectedTags: "tag2,tag1,tag3", + }, + } + + for _, tc := range testCases { + ts := New(tc.InitialTags...) + ts.maxtags = 10 + + added := ts.Add(tc.TagsToAdd...) require.Equal(t, tc.ExpectNoTagsAdded, !added, tc.Alias) require.Equal(t, tc.ExpectedTags, ts.AsList(), tc.Alias) require.Equal(t, strings.Split(tc.ExpectedTags, ","), ts.AsSlice(), tc.Alias) @@ -171,11 +256,102 @@ func TestRemoveTags(t *testing.T) { } for _, tc := range testCases { - ts := FromList(tc.InitialTags) + ts := New(tc.InitialTags) - removed := ts.RemoveList(tc.TagsToRemove) + removed := ts.Remove(tc.TagsToRemove) require.Equal(t, tc.ExpectNoTagsRemoved, !removed, tc.Alias) require.Equal(t, tc.ExpectedTags, ts.AsList(), tc.Alias) } } + +func TestRemoveTagsSlices(t *testing.T) { + testCases := []struct { + Alias string + InitialTags []string + TagsToRemove []string + ExpectedTags string + ExpectNoTagsRemoved bool + }{ + { + Alias: "simple", + InitialTags: []string{"a", "b", "c"}, + TagsToRemove: []string{"a", "b"}, + ExpectedTags: "c", + }, + { + Alias: "remove all tags", + InitialTags: []string{"a", "b", "c", "d", "e", "f"}, + TagsToRemove: []string{"f", "c", "a", "d", "e", "b"}, + ExpectedTags: "", + }, + { + Alias: "ignore duplicate tags", + InitialTags: []string{"a", "b"}, + TagsToRemove: []string{"b", "b", "b", "b"}, + ExpectedTags: "a", + }, + { + Alias: "order of tags is preserved", + InitialTags: []string{"a", "b", "c", "d"}, + TagsToRemove: []string{"a", "c"}, + ExpectedTags: "b,d", + }, + { + Alias: "special characters are allowed", + InitialTags: []string{"anothertag", "btag!!", "c#", "distro 66"}, + TagsToRemove: []string{"distro 66", "btag!!"}, + ExpectedTags: "anothertag,c#", + }, + { + Alias: "empty list errors", + InitialTags: []string{"tag1", "tag2"}, + TagsToRemove: []string{"", "", "", "", "", ""}, + ExpectedTags: "tag1,tag2", + ExpectNoTagsRemoved: true, + }, + { + Alias: "unknown tag errors", + InitialTags: []string{"tag1", "tag2"}, + TagsToRemove: []string{"tag3"}, + ExpectedTags: "tag1,tag2", + ExpectNoTagsRemoved: true, + }, + } + + for _, tc := range testCases { + ts := New(tc.InitialTags...) + + removed := ts.Remove(tc.TagsToRemove...) + require.Equal(t, tc.ExpectNoTagsRemoved, !removed, tc.Alias) + require.Equal(t, tc.ExpectedTags, ts.AsList(), tc.Alias) + } + +} + +func TestBuilders(t *testing.T) { + testCases := []struct { + Alias string + List string + Slice []string + }{ + { + Alias: "simple", + List: "a,b,c", + Slice: []string{"a", "b", "c"}, + }, + { + Alias: "zero values", + List: "", + Slice: nil, + }, + } + + for _, tc := range testCases { + list := New(tc.List) + slice := New(tc.Slice...) + + require.Equal(t, list.AsSlice(), slice.AsSlice()) + require.Equal(t, list.AsList(), slice.AsList()) + } +}