Revert "fix(kv): push down org ID to skip in delete URM (#16841)"

This reverts commit a5f508d.
influxdata · Feb 13, 2020 · 4eb9756 · 4eb9756
1 parent fc64f0f
commit 4eb9756
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 57 deletions.
diff --git 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
@@ -12,9 +12,9 @@ import (
 )
 
 const (
-	userOneID   = "020f755c3c084000"
-	userTwoID   = "020f755c3c084001"
-	userThreeID = "020f755c3c084002"
+	userOneID   = "020f755c3c082000"
+	userTwoID   = "020f755c3c082001"
+	userThreeID = "020f755c3c082002"
 )
 
 var userCmpOptions = cmp.Options{