From d901f6661456061ea213053cedcdd0940e66264f Mon Sep 17 00:00:00 2001 From: David Christofas Date: Wed, 8 Sep 2021 11:29:00 +0200 Subject: [PATCH] enhance filter handling in the sql share drivers --- pkg/cbox/share/sql/sql.go | 117 ++++++++++++++++++++++++++--- pkg/share/manager/json/json.go | 11 ++- pkg/share/manager/memory/memory.go | 12 ++- pkg/share/manager/sql/sql.go | 114 +++++++++++++++++++++++++--- pkg/share/share.go | 3 + 5 files changed, 227 insertions(+), 30 deletions(-) diff --git a/pkg/cbox/share/sql/sql.go b/pkg/cbox/share/sql/sql.go index 675f40ba0a9..f5d27f04bff 100644 --- a/pkg/cbox/share/sql/sql.go +++ b/pkg/cbox/share/sql/sql.go @@ -43,6 +43,11 @@ import ( _ "github.com/go-sql-driver/mysql" ) +const ( + shareTypeUser = 0 + shareTypeGroup = 1 +) + func init() { registry.Register("sql", New) } @@ -276,19 +281,30 @@ func (m *mgr) UpdateShare(ctx context.Context, ref *collaboration.ShareReference func (m *mgr) ListShares(ctx context.Context, filters []*collaboration.Filter) ([]*collaboration.Share, error) { uid := conversions.FormatUserID(ctxpkg.ContextMustGetUser(ctx).Id) - query := `SELECT coalesce(uid_owner, '') as uid_owner, coalesce(uid_initiator, '') as uid_initiator, coalesce(share_with, '') as share_with, - coalesce(fileid_prefix, '') as fileid_prefix, coalesce(item_source, '') as item_source, id, stime, permissions, share_type + query := `select coalesce(uid_owner, '') as uid_owner, coalesce(uid_initiator, '') as uid_initiator, coalesce(share_with, '') as share_with, + coalesce(fileid_prefix, '') as fileid_prefix, coalesce(item_source, '') as item_source, id, stime, permissions, share_type FROM oc_share - WHERE (orphan = 0 or orphan IS NULL) AND (uid_owner=? or uid_initiator=?) AND (share_type=? OR share_type=?)` - params := []interface{}{uid, uid, 0, 1} - for _, f := range filters { - if f.Type == collaboration.Filter_TYPE_RESOURCE_ID { - query += " AND (fileid_prefix=? AND item_source=?)" - params = append(params, f.GetResourceId().StorageId, f.GetResourceId().OpaqueId) - } else if f.Type == collaboration.Filter_TYPE_EXCLUDE_DENIALS { - // TODO this may change once the mapping of permission to share types is completed (cf. pkg/cbox/utils/conversions.go) - query += " AND (permissions > 0)" + WHERE (orphan = 0 or orphan IS NULL) AND (uid_owner=? or uid_initiator=?)` + params := []interface{}{uid, uid} + var ( + filterQuery string + filterParams []interface{} + err error + ) + if len(filters) == 0 { + filterQuery += "(share_type=? OR share_type=?)" + params = append(params, shareTypeUser) + params = append(params, shareTypeGroup) + } else { + filterQuery, filterParams, err = translateFilters(filters) + if err != nil { + return nil, err } + params = append(params, filterParams...) + } + + if filterQuery != "" { + query = fmt.Sprintf("%s AND (%s)", query, filterQuery) } rows, err := m.db.Query(query, params...) @@ -342,6 +358,16 @@ func (m *mgr) ListReceivedShares(ctx context.Context, filters []*collaboration.F } } + filterQuery, filterParams, err := translateFilters(filters) + if err != nil { + return nil, err + } + params = append(params, filterParams...) + + if filterQuery != "" { + query = fmt.Sprintf("%s AND (%s)", query, filterQuery) + } + rows, err := m.db.Query(query, params...) if err != nil { return nil, err @@ -476,3 +502,72 @@ func (m *mgr) UpdateReceivedShare(ctx context.Context, ref *collaboration.ShareR rs.State = f.GetState() 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: + return shareTypeUser + case provider.GranteeType_GRANTEE_TYPE_GROUP: + return shareTypeGroup + } + return -1 +} + +// translateFilters translates the filters to sql queries +func translateFilters(filters []*collaboration.Filter) (string, []interface{}, error) { + var ( + filterQuery string + params []interface{} + ) + + groupedFilters := 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. + // Combine the different filter types using `AND` + var filterCounter = 0 + for filterType, filters := range groupedFilters { + switch filterType { + case collaboration.Filter_TYPE_RESOURCE_ID: + filterQuery += "(" + for i, f := range filters { + filterQuery += "item_source=?" + params = append(params, f.GetResourceId().OpaqueId) + + if i != len(filters)-1 { + filterQuery += " OR " + } + } + filterQuery += ")" + case collaboration.Filter_TYPE_GRANTEE_TYPE: + filterQuery += "(" + for i, f := range filters { + filterQuery += "share_type=?" + params = append(params, granteeTypeToShareType(f.GetGranteeType())) + + if i != len(filters)-1 { + filterQuery += " OR " + } + } + filterQuery += ")" + case collaboration.Filter_TYPE_EXCLUDE_DENIALS: + // TODO this may change once the mapping of permission to share types is completed (cf. pkg/cbox/utils/conversions.go) + filterQuery += "permissions > 0" + default: + return "", nil, fmt.Errorf("filter type is not supported") + } + if filterCounter != len(groupedFilters)-1 { + filterQuery += " AND " + } + filterCounter++ + } + return filterQuery, params, nil +} diff --git a/pkg/share/manager/json/json.go b/pkg/share/manager/json/json.go index bcac6785c67..afe158e63e5 100644 --- a/pkg/share/manager/json/json.go +++ b/pkg/share/manager/json/json.go @@ -389,12 +389,17 @@ func (m *mgr) ListReceivedShares(ctx context.Context, filters []*collaboration.F continue } + allFiltersMatch := true for _, f := range filters { - if share.MatchesFilter(s, f) { - rs := m.convert(ctx, s) - rss = append(rss, rs) + if !share.MatchesFilter(s, f) { + allFiltersMatch = false + break } } + if allFiltersMatch { + rs := m.convert(ctx, s) + rss = append(rss, rs) + } } return rss, nil } diff --git a/pkg/share/manager/memory/memory.go b/pkg/share/manager/memory/memory.go index 796d979657f..7b1a1998b9d 100644 --- a/pkg/share/manager/memory/memory.go +++ b/pkg/share/manager/memory/memory.go @@ -256,13 +256,17 @@ 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) { - rs := m.convert(ctx, s) - rss = append(rss, rs) + if !share.MatchesFilter(s, f) { + allFiltersMatch = false + break } } + if allFiltersMatch { + rs := m.convert(ctx, s) + rss = append(rss, rs) + } } return rss, nil } diff --git a/pkg/share/manager/sql/sql.go b/pkg/share/manager/sql/sql.go index c43ef2b3778..d935dd796c1 100644 --- a/pkg/share/manager/sql/sql.go +++ b/pkg/share/manager/sql/sql.go @@ -42,6 +42,11 @@ import ( _ "github.com/go-sql-driver/mysql" ) +const ( + shareTypeUser = 0 + shareTypeGroup = 1 +) + func init() { registry.Register("oc10-sql", NewMysql) } @@ -271,20 +276,26 @@ func (m *mgr) UpdateShare(ctx context.Context, ref *collaboration.ShareReference func (m *mgr) ListShares(ctx context.Context, filters []*collaboration.Filter) ([]*collaboration.Share, error) { uid := ctxpkg.ContextMustGetUser(ctx).Username - query := "select coalesce(uid_owner, '') as uid_owner, coalesce(uid_initiator, '') as uid_initiator, coalesce(share_with, '') as share_with, coalesce(item_source, '') as item_source, id, stime, permissions, share_type FROM oc_share WHERE (uid_owner=? or uid_initiator=?) AND (share_type=? OR share_type=?)" - var filterQuery string - params := []interface{}{uid, uid, 0, 1} - for i, f := range filters { - if f.Type == collaboration.Filter_TYPE_RESOURCE_ID { - filterQuery += "(item_source=?)" - if i != len(filters)-1 { - filterQuery += " AND " - } - params = append(params, f.GetResourceId().OpaqueId) - } else { - return nil, fmt.Errorf("filter type is not supported") + query := "select coalesce(uid_owner, '') as uid_owner, coalesce(uid_initiator, '') as uid_initiator, coalesce(share_with, '') as share_with, coalesce(item_source, '') as item_source, id, stime, permissions, share_type FROM oc_share WHERE (uid_owner=? or uid_initiator=?)" + params := []interface{}{uid, uid} + + var ( + filterQuery string + filterParams []interface{} + err error + ) + if len(filters) == 0 { + filterQuery += "(share_type=? OR share_type=?)" + params = append(params, shareTypeUser) + params = append(params, shareTypeGroup) + } else { + filterQuery, filterParams, err = translateFilters(filters) + if err != nil { + return nil, err } + params = append(params, filterParams...) } + if filterQuery != "" { query = fmt.Sprintf("%s AND (%s)", query, filterQuery) } @@ -337,6 +348,16 @@ func (m *mgr) ListReceivedShares(ctx context.Context, filters []*collaboration.F query += "AND (share_with=?)" } + filterQuery, filterParams, err := translateFilters(filters) + if err != nil { + return nil, err + } + params = append(params, filterParams...) + + if filterQuery != "" { + query = fmt.Sprintf("%s AND (%s)", query, filterQuery) + } + rows, err := m.db.Query(query, params...) if err != nil { return nil, err @@ -500,3 +521,72 @@ 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: + return shareTypeUser + case provider.GranteeType_GRANTEE_TYPE_GROUP: + return shareTypeGroup + } + return -1 +} + +// translateFilters translates the filters to sql queries +func translateFilters(filters []*collaboration.Filter) (string, []interface{}, error) { + var ( + filterQuery string + params []interface{} + ) + + groupedFilters := 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. + // Combine the different filter types using `AND` + var filterCounter = 0 + for filterType, filters := range groupedFilters { + switch filterType { + case collaboration.Filter_TYPE_RESOURCE_ID: + filterQuery += "(" + for i, f := range filters { + filterQuery += "item_source=?" + params = append(params, f.GetResourceId().OpaqueId) + + if i != len(filters)-1 { + filterQuery += " OR " + } + } + filterQuery += ")" + case collaboration.Filter_TYPE_GRANTEE_TYPE: + filterQuery += "(" + for i, f := range filters { + filterQuery += "share_type=?" + params = append(params, granteeTypeToShareType(f.GetGranteeType())) + + if i != len(filters)-1 { + filterQuery += " OR " + } + } + filterQuery += ")" + case collaboration.Filter_TYPE_EXCLUDE_DENIALS: + // TODO this may change once the mapping of permission to share types is completed (cf. pkg/cbox/utils/conversions.go) + filterQuery += "permissions > 0" + default: + return "", nil, fmt.Errorf("filter type is not supported") + } + if filterCounter != len(groupedFilters)-1 { + filterQuery += " AND " + } + filterCounter++ + } + return filterQuery, params, nil +} diff --git a/pkg/share/share.go b/pkg/share/share.go index 53eb398e76b..f3020cc7f14 100644 --- a/pkg/share/share.go +++ b/pkg/share/share.go @@ -113,6 +113,9 @@ func MatchesFilter(share *collaboration.Share, filter *collaboration.Filter) boo return utils.ResourceIDEqual(share.ResourceId, filter.GetResourceId()) case collaboration.Filter_TYPE_GRANTEE_TYPE: return share.Grantee.Type == filter.GetGranteeType() + case collaboration.Filter_TYPE_EXCLUDE_DENIALS: + // TODO filter deny shares. + return true default: return false }