correct and simplify filter checks

pkg/cbox/share/sql/sql.go

@@ -494,14 +494,6 @@ func (m *mgr) UpdateReceivedShare(ctx context.Context, ref *collaboration.ShareR
 	return rs, nil
 }
 
-func groupFiltersByType(filters []*collaboration.Filter) map[collaboration.Filter_Type][]*collaboration.Filter {
-	grouped := make(map[collaboration.Filter_Type][]*collaboration.Filter)
-	for _, f := range filters {
-		grouped[f.Type] = append(grouped[f.Type], f)
-	}
-	return grouped
-}
-
 func granteeTypeToShareType(granteeType provider.GranteeType) int {
 	switch granteeType {
 	case provider.GranteeType_GRANTEE_TYPE_USER:
@@ -519,7 +511,7 @@ func translateFilters(filters []*collaboration.Filter) (string, []interface{}, e
 		params      []interface{}
 	)
 
-	groupedFilters := groupFiltersByType(filters)
+	groupedFilters := share.GroupFiltersByType(filters)
 	// If multiple filters of the same type are passed to this function, they need to be combined with the `OR` operator.
 	// That is why the filters got grouped by type.
 	// For every given filter type, iterate over the filters and if there are more than one combine them.

pkg/share/manager/json/json.go

@@ -361,14 +361,7 @@ func (m *mgr) ListShares(ctx context.Context, filters []*collaboration.Filter) (
 				continue
 			}
 			// check filters
-			allFiltersMatch := true
-			for _, f := range filters {
-				if !share.MatchesFilter(s, f) {
-					allFiltersMatch = false
-					break
-				}
-			}
-			if allFiltersMatch {
+			if share.MatchesFilters(s, filters) {
 				ss = append(ss, s)
 			}
 		}
@@ -394,14 +387,7 @@ func (m *mgr) ListReceivedShares(ctx context.Context, filters []*collaboration.F
 			continue
 		}
 
-		allFiltersMatch := true
-		for _, f := range filters {
-			if !share.MatchesFilter(s, f) {
-				allFiltersMatch = false
-				break
-			}
-		}
-		if allFiltersMatch {
+		if share.MatchesFilters(s, filters) {
 			rs := m.convert(ctx, s)
 			rss = append(rss, rs)
 		}

pkg/share/manager/memory/memory.go

@@ -228,14 +228,7 @@ func (m *manager) ListShares(ctx context.Context, filters []*collaboration.Filte
 				continue
 			}
 			// check filters
-			allFiltersMatch := true
-			for _, f := range filters {
-				if !share.MatchesFilter(s, f) {
-					allFiltersMatch = false
-					break
-				}
-			}
-			if allFiltersMatch {
+			if share.MatchesFilters(s, filters) {
 				ss = append(ss, s)
 			}
 		}
@@ -260,14 +253,8 @@ func (m *manager) ListReceivedShares(ctx context.Context, filters []*collaborati
 			rss = append(rss, rs)
 			continue
 		}
-		allFiltersMatch := true
-		for _, f := range filters {
-			if !share.MatchesFilter(s, f) {
-				allFiltersMatch = false
-				break
-			}
-		}
-		if allFiltersMatch {
+
+		if share.MatchesFilters(s, filters) {
 			rs := m.convert(ctx, s)
 			rss = append(rss, rs)
 		}

pkg/share/manager/sql/sql.go

@@ -522,14 +522,6 @@ func (m *mgr) getReceivedByKey(ctx context.Context, key *collaboration.ShareKey)
 	return m.convertToCS3ReceivedShare(ctx, s, m.storageMountID)
 }
 
-func groupFiltersByType(filters []*collaboration.Filter) map[collaboration.Filter_Type][]*collaboration.Filter {
-	grouped := make(map[collaboration.Filter_Type][]*collaboration.Filter)
-	for _, f := range filters {
-		grouped[f.Type] = append(grouped[f.Type], f)
-	}
-	return grouped
-}
-
 func granteeTypeToShareType(granteeType provider.GranteeType) int {
 	switch granteeType {
 	case provider.GranteeType_GRANTEE_TYPE_USER:
@@ -547,7 +539,7 @@ func translateFilters(filters []*collaboration.Filter) (string, []interface{}, e
 		params      []interface{}
 	)
 
-	groupedFilters := groupFiltersByType(filters)
+	groupedFilters := share.GroupFiltersByType(filters)
 	// If multiple filters of the same type are passed to this function, they need to be combined with the `OR` operator.
 	// That is why the filters got grouped by type.
 	// For every given filter type, iterate over the filters and if there are more than one combine them.

pkg/share/share.go

@@ -122,3 +122,36 @@ func MatchesFilter(share *collaboration.Share, filter *collaboration.Filter) boo
 		return false
 	}
 }
+
+// MatchesAnyFilter checks if the share passes at least one of the given filters.
+func MatchesAnyFilter(share *collaboration.Share, filters []*collaboration.Filter) bool {
+	for _, f := range filters {
+		if MatchesFilter(share, f) {
+			return true
+		}
+	}
+	return false
+}
+
+// MatchesFilters checks if the share passes the given filters.
+// Filters of the same type form a disjuntion, a logical OR. Filters of separate type form a conjunction, a logical AND.
+// Here is an example:
+// (resource_id=1 OR resource_id=2) AND (grantee_type=USER OR grantee_type=GROUP)
+func MatchesFilters(share *collaboration.Share, filters []*collaboration.Filter) bool {
+	grouped := GroupFiltersByType(filters)
+	for _, f := range grouped {
+		if !MatchesAnyFilter(share, f) {
+			return false
+		}
+	}
+	return true
+}
+
+// GroupFiltersByType groups the given filters and returns a map using the filter type as the key.
+func GroupFiltersByType(filters []*collaboration.Filter) map[collaboration.Filter_Type][]*collaboration.Filter {
+	grouped := make(map[collaboration.Filter_Type][]*collaboration.Filter)
+	for _, f := range filters {
+		grouped[f.Type] = append(grouped[f.Type], f)
+	}
+	return grouped
+}

pkg/share/share_test.go

@@ -23,8 +23,8 @@ import (
 
 	groupv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
 	userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
-	collaborationv1beta1 "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
-	providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
+	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 )
 
 func TestIsCreatedByUser(t *testing.T) {
@@ -35,21 +35,21 @@ func TestIsCreatedByUser(t *testing.T) {
 		},
 	}
 
-	s1 := collaborationv1beta1.Share{
+	s1 := collaboration.Share{
 		Owner: &userv1beta1.UserId{
 			Idp:      "sampleidp",
 			OpaqueId: "user",
 		},
 	}
 
-	s2 := collaborationv1beta1.Share{
+	s2 := collaboration.Share{
 		Creator: &userv1beta1.UserId{
 			Idp:      "sampleidp",
 			OpaqueId: "user",
 		},
 	}
 
-	s3 := collaborationv1beta1.Share{
+	s3 := collaboration.Share{
 		Owner: &userv1beta1.UserId{
 			Idp:      "sampleidp",
 			OpaqueId: "user",
@@ -85,10 +85,10 @@ func TestIsGrantedToUser(t *testing.T) {
 		Groups: []string{"groupid"},
 	}
 
-	s1 := collaborationv1beta1.Share{
-		Grantee: &providerv1beta1.Grantee{
-			Type: providerv1beta1.GranteeType_GRANTEE_TYPE_USER,
-			Id: &providerv1beta1.Grantee_UserId{
+	s1 := collaboration.Share{
+		Grantee: &provider.Grantee{
+			Type: provider.GranteeType_GRANTEE_TYPE_USER,
+			Id: &provider.Grantee_UserId{
 				UserId: &userv1beta1.UserId{
 					Idp:      "sampleidp",
 					OpaqueId: "another",
@@ -97,10 +97,10 @@ func TestIsGrantedToUser(t *testing.T) {
 		},
 	}
 
-	s2 := collaborationv1beta1.Share{
-		Grantee: &providerv1beta1.Grantee{
-			Type: providerv1beta1.GranteeType_GRANTEE_TYPE_GROUP,
-			Id: &providerv1beta1.Grantee_GroupId{
+	s2 := collaboration.Share{
+		Grantee: &provider.Grantee{
+			Type: provider.GranteeType_GRANTEE_TYPE_GROUP,
+			Id: &provider.Grantee_GroupId{
 				GroupId: &groupv1beta1.GroupId{OpaqueId: "groupid"}},
 		},
 	}
@@ -109,11 +109,105 @@ func TestIsGrantedToUser(t *testing.T) {
 		t.Error("Expected the share to be granted to user")
 	}
 
-	s3 := collaborationv1beta1.Share{
-		Grantee: &providerv1beta1.Grantee{},
+	s3 := collaboration.Share{
+		Grantee: &provider.Grantee{},
 	}
 
 	if IsGrantedToUser(&s3, &user) {
 		t.Error("Expecte the share not to be granted to user")
 	}
 }
+
+func TestMatchesFilter(t *testing.T) {
+	id := &provider.ResourceId{StorageId: "storage", OpaqueId: "opaque"}
+	share := &collaboration.Share{
+		ResourceId: id,
+		Grantee: &provider.Grantee{
+			Type: provider.GranteeType_GRANTEE_TYPE_USER,
+		},
+		Permissions: &collaboration.SharePermissions{Permissions: &provider.ResourcePermissions{}},
+	}
+
+	if !MatchesFilter(share, ResourceIDFilter(id)) {
+		t.Errorf("Expected share to pass the id filter. Share: %v", share)
+	}
+	if MatchesFilter(share, GroupGranteeFilter()) {
+		t.Errorf("Expected share to not pass the grantee type filter. Share: %v", share)
+	}
+	if MatchesFilter(share, &collaboration.Filter{Type: collaboration.Filter_TYPE_EXCLUDE_DENIALS}) {
+		t.Errorf("Expected share to not pass the exclude denials filter. Share: %v", share)
+	}
+	if MatchesFilter(share, &collaboration.Filter{Type: collaboration.Filter_TYPE_INVALID}) {
+		t.Errorf("Expected share to not pass an invalid filter. Share: %v", share)
+	}
+}
+
+func TestMatchesAnyFilter(t *testing.T) {
+	id := &provider.ResourceId{StorageId: "storage", OpaqueId: "opaque"}
+	share := &collaboration.Share{
+		ResourceId: id,
+		Grantee: &provider.Grantee{
+			Type: provider.GranteeType_GRANTEE_TYPE_USER,
+		},
+		Permissions: &collaboration.SharePermissions{Permissions: &provider.ResourcePermissions{}},
+	}
+
+	f1 := []*collaboration.Filter{UserGranteeFilter(), GroupGranteeFilter()}
+	if !MatchesAnyFilter(share, f1) {
+		t.Errorf("Expected share to match any of the given filters. Share: %v, Filters: %v", share, f1)
+	}
+
+	f2 := []*collaboration.Filter{ResourceIDFilter(&provider.ResourceId{StorageId: "something", OpaqueId: "different"}), GroupGranteeFilter()}
+	if MatchesAnyFilter(share, f2) {
+		t.Errorf("Expected share to not match any of the given filters. Share: %v, Filters: %v", share, f2)
+	}
+}
+
+func TestMatchesFilters(t *testing.T) {
+	id := &provider.ResourceId{StorageId: "storage", OpaqueId: "opaque"}
+	share := &collaboration.Share{
+		ResourceId: id,
+		Grantee: &provider.Grantee{
+			Type: provider.GranteeType_GRANTEE_TYPE_USER,
+		},
+		Permissions: &collaboration.SharePermissions{Permissions: &provider.ResourcePermissions{}},
+	}
+
+	f1 := []*collaboration.Filter{ResourceIDFilter(id), GroupGranteeFilter()}
+	if MatchesFilters(share, f1) {
+		t.Errorf("Expected share to not match the filters. Share %v, Filters %v", share, f1)
+	}
+
+	f2 := []*collaboration.Filter{ResourceIDFilter(id), UserGranteeFilter(), GroupGranteeFilter()}
+	if !MatchesFilters(share, f2) {
+		t.Errorf("Expected share to match the filters. Share %v, Filters %v", share, f2)
+	}
+}
+
+func TestGroupFiltersByType(t *testing.T) {
+	id := &provider.ResourceId{StorageId: "storage", OpaqueId: "opaque"}
+	filters := []*collaboration.Filter{UserGranteeFilter(), GroupGranteeFilter(), ResourceIDFilter(id)}
+
+	grouped := GroupFiltersByType(filters)
+
+	for fType, f := range grouped {
+		switch fType {
+		case collaboration.Filter_TYPE_GRANTEE_TYPE:
+			if len(f) != 2 {
+				t.Errorf("Expected 2 grantee type filters got %d", len(f))
+			}
+			for i := range f {
+				if f[i].Type != fType {
+					t.Errorf("Filter %v doesn't belong to this type %v", f[i], t)
+				}
+			}
+		case collaboration.Filter_TYPE_RESOURCE_ID:
+			if len(f) != 1 {
+				t.Errorf("Expected 1 resource id filter got %d", len(f))
+			}
+			if f[0].Type != fType {
+				t.Errorf("Filter %v doesn't belong to this type %v", f[0], t)
+			}
+		}
+	}
+}