From 4eb975610531c9c474ff7676fa2b5a30ac4c48e6 Mon Sep 17 00:00:00 2001 From: George MacRorie Date: Thu, 13 Feb 2020 09:04:59 +0000 Subject: [PATCH] Revert "fix(kv): push down org ID to skip in delete URM (#16841)" This reverts commit a5f508de771c6383729baa9f634b5518fedf2ed7. --- kv/urm.go | 64 +++++++---------------------------------- testing/user_service.go | 6 ++-- 2 files changed, 13 insertions(+), 57 deletions(-) diff --git a/kv/urm.go b/kv/urm.go index 124e4aec7a8..358c37a394d 100644 --- a/kv/urm.go +++ b/kv/urm.go @@ -126,39 +126,14 @@ func userResourceMappingPredicate(filter influxdb.UserResourceMappingFilter) Cur } } -type urmFindOptions struct { - skipKeys map[string]struct{} -} - -type urmFindOption func(*urmFindOptions) - -func (f *urmFindOptions) skip(key string) (skip bool) { - if f.skipKeys == nil { - return false - } - - _, skip = f.skipKeys[key] - return -} - -func withSkipKey(key string) urmFindOption { - return func(o *urmFindOptions) { - if o.skipKeys == nil { - o.skipKeys = map[string]struct{}{} - } - - o.skipKeys[key] = struct{}{} - } -} - -func (s *Service) findUserResourceMappings(ctx context.Context, tx Tx, filter influxdb.UserResourceMappingFilter, opts ...urmFindOption) ([]*influxdb.UserResourceMapping, error) { +func (s *Service) findUserResourceMappings(ctx context.Context, tx Tx, filter influxdb.UserResourceMappingFilter) ([]*influxdb.UserResourceMapping, error) { ms := []*influxdb.UserResourceMapping{} pred := userResourceMappingPredicate(filter) filterFn := filterMappingsFn(filter) // if we are given a user id we should try finding by index if filter.UserID.Valid() { var err error - ms, err = s.findUserResourceMappingsByIndex(ctx, tx, filter, opts...) + ms, err = s.findUserResourceMappingsByIndex(ctx, tx, filter) if err != nil { return nil, err } @@ -191,16 +166,9 @@ func (s *Service) findUserResourceMappings(ctx context.Context, tx Tx, filter in return ms, err } -func (s *Service) findUserResourceMappingsByIndex(ctx context.Context, tx Tx, filter influxdb.UserResourceMappingFilter, opts ...urmFindOption) ([]*influxdb.UserResourceMapping, error) { - var ( - ms = []*influxdb.UserResourceMapping{} - filterFn = filterMappingsFn(filter) - options = urmFindOptions{} - ) - - for _, opt := range opts { - opt(&options) - } +func (s *Service) findUserResourceMappingsByIndex(ctx context.Context, tx Tx, filter influxdb.UserResourceMappingFilter) ([]*influxdb.UserResourceMapping, error) { + ms := []*influxdb.UserResourceMapping{} + filterFn := filterMappingsFn(filter) bkt, err := tx.Bucket(urmBucket) if err != nil { @@ -227,11 +195,6 @@ func (s *Service) findUserResourceMappingsByIndex(ctx context.Context, tx Tx, fi } for k, v := cursor.Next(); k != nil && v != nil; k, v = cursor.Next() { - // step over skip keys - if options.skip(string(v)) { - continue - } - nv, err := bkt.Get(v) if err != nil { return nil, &influxdb.Error{ @@ -463,24 +426,17 @@ func (s *Service) DeleteUserResourceMapping(ctx context.Context, resourceID infl } if m.ResourceType == influxdb.OrgsResourceType { - key, err := userResourceKey(m) - if err != nil { - // I'm not super concerned that we will get here. We know this is a valid resource - // because we've just found it above. Me of the future... if this was a problem, - // sorry. - return err - } - return s.deleteOrgDependentMappings(ctx, tx, m, withSkipKey(string(key))) + return s.deleteOrgDependentMappings(ctx, tx, m) } return nil }) } -func (s *Service) deleteUserResourceMapping(ctx context.Context, tx Tx, filter influxdb.UserResourceMappingFilter, opts ...urmFindOption) error { +func (s *Service) deleteUserResourceMapping(ctx context.Context, tx Tx, filter influxdb.UserResourceMappingFilter) error { // TODO(goller): do we really need to find here? Seems like a Get is // good enough. - ms, err := s.findUserResourceMappings(ctx, tx, filter, opts...) + ms, err := s.findUserResourceMappings(ctx, tx, filter) if err != nil { return err } @@ -571,7 +527,7 @@ func (s *Service) deleteUserResourceMappings(ctx context.Context, tx Tx, filter } // This method deletes the user/resource mappings for resources that belong to an organization. -func (s *Service) deleteOrgDependentMappings(ctx context.Context, tx Tx, m *influxdb.UserResourceMapping, opts ...urmFindOption) error { +func (s *Service) deleteOrgDependentMappings(ctx context.Context, tx Tx, m *influxdb.UserResourceMapping) error { bf := influxdb.BucketFilter{OrganizationID: &m.ResourceID} bs, err := s.findBuckets(ctx, tx, bf) if err != nil { @@ -584,7 +540,7 @@ func (s *Service) deleteOrgDependentMappings(ctx context.Context, tx Tx, m *infl UserID: m.UserID, } - if err := s.deleteUserResourceMapping(ctx, tx, filter, opts...); err != nil { + if err := s.deleteUserResourceMapping(ctx, tx, filter); err != nil { if influxdb.ErrorCode(err) == influxdb.ENotFound { s.log.Info("URM bucket is missing", zap.Stringer("orgID", m.ResourceID)) continue diff --git a/testing/user_service.go b/testing/user_service.go index 1e4403772b2..1bee2286b6c 100644 --- a/testing/user_service.go +++ b/testing/user_service.go @@ -12,9 +12,9 @@ import ( ) const ( - userOneID = "020f755c3c084000" - userTwoID = "020f755c3c084001" - userThreeID = "020f755c3c084002" + userOneID = "020f755c3c082000" + userTwoID = "020f755c3c082001" + userThreeID = "020f755c3c082002" ) var userCmpOptions = cmp.Options{