Drop ResourceProvider abstraction (#475)

pkg/application/service/factory/service_factory.go

@@ -111,7 +111,7 @@ func NewServiceFactory(options ...Option) *ServiceFactory {
 
 	if f.signupServiceFunc == nil {
 		f.signupServiceFunc = func(_ ...signupservice.SignupServiceOption) service.SignupService {
-			return signupservice.NewSignupService(f.getContext(), f.signupServiceOptions...)
+			return signupservice.NewSignupService(f.getContext().Client(), f.signupServiceOptions...)
 		}
 	}
 

pkg/application/service/services.go

@@ -11,8 +11,6 @@ import (
 type InformerService interface {
 	GetMasterUserRecord(name string) (*toolchainv1alpha1.MasterUserRecord, error)
 	GetSpace(name string) (*toolchainv1alpha1.Space, error)
-	GetToolchainStatus() (*toolchainv1alpha1.ToolchainStatus, error)
-	GetUserSignup(name string) (*toolchainv1alpha1.UserSignup, error)
 	ListSpaceBindings(reqs ...labels.Requirement) ([]toolchainv1alpha1.SpaceBinding, error)
 	GetProxyPluginConfig(name string) (*toolchainv1alpha1.ProxyPlugin, error)
 	GetNSTemplateTier(name string) (*toolchainv1alpha1.NSTemplateTier, error)

pkg/informers/service/informer_service.go

@@ -50,20 +50,6 @@ func (s *ServiceImpl) GetSpace(name string) (*toolchainv1alpha1.Space, error) {
 	return space, err
 }
 
-func (s *ServiceImpl) GetToolchainStatus() (*toolchainv1alpha1.ToolchainStatus, error) {
-	status := &toolchainv1alpha1.ToolchainStatus{}
-	namespacedName := types.NamespacedName{Name: "toolchain-status", Namespace: s.namespace}
-	err := s.client.Get(context.TODO(), namespacedName, status)
-	return status, err
-}
-
-func (s *ServiceImpl) GetUserSignup(name string) (*toolchainv1alpha1.UserSignup, error) {
-	signup := &toolchainv1alpha1.UserSignup{}
-	namespacedName := types.NamespacedName{Name: name, Namespace: s.namespace}
-	err := s.client.Get(context.TODO(), namespacedName, signup)
-	return signup, err
-}
-
 func (s *ServiceImpl) ListSpaceBindings(reqs ...labels.Requirement) ([]toolchainv1alpha1.SpaceBinding, error) {
 	selector := labels.NewSelector()
 	for i := range reqs {

pkg/informers/service/informer_service_test.go

@@ -157,52 +157,6 @@ func TestInformerService(t *testing.T) {
 		})
 	})
 
-	t.Run("toolchainstatuses", func(t *testing.T) {
-		t.Run("not found", func(t *testing.T) {
-			// given
-			client := test.NewFakeClient(t)
-			svc := service.NewInformerService(client, test.HostOperatorNs)
-
-			// when
-			val, err := svc.GetToolchainStatus()
-
-			// then
-			assert.Empty(t, val)
-			assert.EqualError(t, err, "toolchainstatuses.toolchain.dev.openshift.com \"toolchain-status\" not found")
-		})
-
-		t.Run("found", func(t *testing.T) {
-			// when
-			val, err := svc.GetToolchainStatus()
-
-			// then
-			require.NotNil(t, val)
-			require.NoError(t, err)
-			assert.Equal(t, status.Status, val.Status)
-		})
-	})
-
-	t.Run("usersignups", func(t *testing.T) {
-		t.Run("not found", func(t *testing.T) {
-			// when
-			val, err := svc.GetUserSignup("unknown")
-
-			// then
-			assert.Empty(t, val)
-			assert.EqualError(t, err, "usersignups.toolchain.dev.openshift.com \"unknown\" not found")
-		})
-
-		t.Run("found", func(t *testing.T) {
-			// when
-			val, err := svc.GetUserSignup("johnUserSignup")
-
-			// then
-			require.NotNil(t, val)
-			require.NoError(t, err)
-			assert.Equal(t, signupJohn.Spec, val.Spec)
-		})
-	})
-
 	t.Run("bannedusers", func(t *testing.T) {
 		t.Run("not found", func(t *testing.T) {
 			// when

pkg/signup/service/signup_service.go

@@ -11,12 +11,9 @@ import (
 
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
 	"github.com/codeready-toolchain/registration-service/pkg/application/service"
-	"github.com/codeready-toolchain/registration-service/pkg/application/service/base"
-	servicecontext "github.com/codeready-toolchain/registration-service/pkg/application/service/context"
 	"github.com/codeready-toolchain/registration-service/pkg/configuration"
 	"github.com/codeready-toolchain/registration-service/pkg/context"
 	"github.com/codeready-toolchain/registration-service/pkg/errors"
-	infservice "github.com/codeready-toolchain/registration-service/pkg/informers/service"
 	"github.com/codeready-toolchain/registration-service/pkg/log"
 	"github.com/codeready-toolchain/registration-service/pkg/namespaced"
 	"github.com/codeready-toolchain/registration-service/pkg/signup"
@@ -29,9 +26,7 @@ import (
 	apiv1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/selection"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -49,22 +44,17 @@ var annotationsToRetain = []string{
 
 // ServiceImpl represents the implementation of the signup service.
 type ServiceImpl struct { // nolint:revive
-	base.BaseService
 	namespaced.Client
-	defaultProvider ResourceProvider
-	CaptchaChecker  captcha.Assessor
+	CaptchaChecker captcha.Assessor
 }
 
 type SignupServiceOption func(svc *ServiceImpl)
 
 // NewSignupService creates a service object for performing user signup-related activities.
-func NewSignupService(context servicecontext.ServiceContext, opts ...SignupServiceOption) service.SignupService {
-
+func NewSignupService(client namespaced.Client, opts ...SignupServiceOption) service.SignupService {
 	s := &ServiceImpl{
-		BaseService:     base.NewBaseService(context),
-		defaultProvider: infservice.NewInformerService(context.Client().Client, context.Client().Namespace),
-		CaptchaChecker:  captcha.Helper{},
-		Client:          context.Client(),
+		CaptchaChecker: captcha.Helper{},
+		Client:         client,
 	}
 
 	for _, opt := range opts {
@@ -97,7 +87,7 @@ func (s *ServiceImpl) newUserSignup(ctx *gin.Context) (*toolchainv1alpha1.UserSi
 
 	// Query BannedUsers to check the user has not been banned
 	bannedUsers := &toolchainv1alpha1.BannedUserList{}
-	if err := s.List(gocontext.TODO(), bannedUsers, client.InNamespace(s.Namespace),
+	if err := s.List(ctx, bannedUsers, client.InNamespace(s.Namespace),
 		client.MatchingLabels{toolchainv1alpha1.BannedUserEmailHashLabelKey: hash.EncodeString(userEmail)}); err != nil {
 		return nil, err
 	}
@@ -293,11 +283,11 @@ func (s *ServiceImpl) Signup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, e
 
 	// Retrieve UserSignup resource from the host cluster
 	userSignup := &toolchainv1alpha1.UserSignup{}
-	if err := s.Get(gocontext.TODO(), s.NamespacedName(encodedUserID), userSignup); err != nil {
+	if err := s.Get(ctx, s.NamespacedName(encodedUserID), userSignup); err != nil {
 		if apierrors.IsNotFound(err) {
 			// The UserSignup could not be located by its encoded UserID, attempt to load it using its encoded PreferredUsername instead
 			encodedUsername := EncodeUserIdentifier(ctx.GetString(context.UsernameKey))
-			if err := s.Get(gocontext.TODO(), s.NamespacedName(encodedUsername), userSignup); err != nil {
+			if err := s.Get(ctx, s.NamespacedName(encodedUsername), userSignup); err != nil {
 				if apierrors.IsNotFound(err) {
 					// New Signup
 					log.WithValues(map[string]interface{}{"encoded_user_id": encodedUserID}).Info(ctx, "user not found, creating a new one")
@@ -329,7 +319,7 @@ func (s *ServiceImpl) createUserSignup(ctx *gin.Context) (*toolchainv1alpha1.Use
 		return nil, err
 	}
 
-	return userSignup, s.Create(gocontext.TODO(), userSignup)
+	return userSignup, s.Create(ctx, userSignup)
 }
 
 // reactivateUserSignup reactivates the deactivated UserSignup resource with the specified username and userID
@@ -355,32 +345,31 @@ func (s *ServiceImpl) reactivateUserSignup(ctx *gin.Context, existing *toolchain
 	existing.Labels = newUserSignup.Labels
 	existing.Spec = newUserSignup.Spec
 
-	return existing, s.Update(gocontext.TODO(), existing)
+	return existing, s.Update(ctx, existing)
 }
 
 // GetSignup returns Signup resource which represents the corresponding K8s UserSignup
 // and MasterUserRecord resources in the host cluster.
 // Returns nil, nil if the UserSignup resource is not found or if it's deactivated.
 func (s *ServiceImpl) GetSignup(ctx *gin.Context, userID, username string) (*signup.Signup, error) {
-	return s.DoGetSignup(ctx, s.defaultProvider, userID, username, true)
+	return s.DoGetSignup(ctx, s.Client, userID, username, true)
 }
 
 // GetSignupFromInformer uses the same logic of the 'GetSignup' function, except it uses informers to get resources.
 // This function and the ResourceProvider abstraction can replace the original GetSignup function once it is determined to be stable.
 // The checkUserSignupCompleted was introduced in order to avoid checking the readiness of the complete condition on the UserSignup in certain situations,
 // such as proxy calls for example.
 func (s *ServiceImpl) GetSignupFromInformer(ctx *gin.Context, userID, username string, checkUserSignupCompleted bool) (*signup.Signup, error) {
-	return s.DoGetSignup(ctx, s.Services().InformerService(), userID, username, checkUserSignupCompleted)
+	return s.DoGetSignup(ctx, s.Client, userID, username, checkUserSignupCompleted)
 }
 
-func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, provider ResourceProvider, userID, username string, checkUserSignupCompleted bool) (*signup.Signup, error) {
+func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, cl namespaced.Client, userID, username string, checkUserSignupCompleted bool) (*signup.Signup, error) {
 	var userSignup *toolchainv1alpha1.UserSignup
-	var err error
 
-	err = signup.PollUpdateSignup(ctx, func() error {
+	err := signup.PollUpdateSignup(ctx, func() error {
 		// Retrieve UserSignup resource from the host cluster, using the specified UserID and username
 		var getError error
-		userSignup, getError = s.DoGetUserSignupFromIdentifier(provider, userID, username)
+		userSignup, getError = s.DoGetUserSignupFromIdentifier(cl, userID, username)
 		// If an error was returned, then return here
 		if getError != nil {
 			if apierrors.IsNotFound(getError) {
@@ -471,8 +460,8 @@ func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, provider ResourceProvider, u
 
 	// If UserSignup status is complete as active
 	// Retrieve MasterUserRecord resource from the host cluster and use its status
-	mur, err := provider.GetMasterUserRecord(userSignup.Status.CompliantUsername)
-	if err != nil {
+	mur := &toolchainv1alpha1.MasterUserRecord{}
+	if err := cl.Get(ctx, cl.NamespacedName(userSignup.Status.CompliantUsername), mur); err != nil {
 		return nil, errs.Wrap(err, fmt.Sprintf("error when retrieving MasterUserRecord for completed UserSignup %s", userSignup.GetName()))
 	}
 	murCondition, _ := condition.FindConditionByType(mur.Status.Conditions, toolchainv1alpha1.ConditionReady)
@@ -491,11 +480,12 @@ func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, provider ResourceProvider, u
 		signupResponse.StartDate = mur.Status.ProvisionedTime.UTC().Format(time.RFC3339)
 	}
 
-	memberCluster, defaultNamespace := GetDefaultUserTarget(provider, userSignup.Status.HomeSpace, mur.Name)
+	memberCluster, defaultNamespace := GetDefaultUserTarget(cl, userSignup.Status.HomeSpace, mur.Name)
 	if memberCluster != "" {
 		// Retrieve cluster-specific URLs from the status of the corresponding member cluster
-		status, err := provider.GetToolchainStatus()
-		if err != nil {
+		status := &toolchainv1alpha1.ToolchainStatus{}
+
+		if err := cl.Get(ctx, cl.NamespacedName("toolchain-status"), status); err != nil {
 			return nil, errs.Wrapf(err, "error when retrieving ToolchainStatus to set Che Dashboard for completed UserSignup %s", userSignup.GetName())
 		}
 		signupResponse.ProxyURL = status.Status.HostRoutes.ProxyURL
@@ -568,18 +558,17 @@ func (s *ServiceImpl) auditUserSignupAgainstClaims(ctx *gin.Context, userSignup
 
 // GetUserSignupFromIdentifier is used to return the actual UserSignup resource instance, rather than the Signup DTO
 func (s *ServiceImpl) GetUserSignupFromIdentifier(userID, username string) (*toolchainv1alpha1.UserSignup, error) {
-	return s.DoGetUserSignupFromIdentifier(s.defaultProvider, userID, username)
+	return s.DoGetUserSignupFromIdentifier(s.Client, userID, username)
 }
 
 // GetUserSignupFromIdentifier is used to return the actual UserSignup resource instance, rather than the Signup DTO
-func (s *ServiceImpl) DoGetUserSignupFromIdentifier(provider ResourceProvider, userID, username string) (*toolchainv1alpha1.UserSignup, error) {
+func (s *ServiceImpl) DoGetUserSignupFromIdentifier(cl namespaced.Client, userID, username string) (*toolchainv1alpha1.UserSignup, error) {
 	// Retrieve UserSignup resource from the host cluster
-	userSignup, err := provider.GetUserSignup(EncodeUserIdentifier(username))
-	if err != nil {
+	userSignup := &toolchainv1alpha1.UserSignup{}
+	if err := cl.Get(gocontext.TODO(), cl.NamespacedName(EncodeUserIdentifier(username)), userSignup); err != nil {
 		if apierrors.IsNotFound(err) {
 			// Capture any error here in a separate var, as we need to preserve the original
-			userSignup, err2 := provider.GetUserSignup(EncodeUserIdentifier(userID))
-			if err2 != nil {
+			if err2 := cl.Get(gocontext.TODO(), cl.NamespacedName(EncodeUserIdentifier(userID)), userSignup); err2 != nil {
 				if apierrors.IsNotFound(err2) {
 					return nil, err
 				}
@@ -654,34 +643,27 @@ func (s *ServiceImpl) PhoneNumberAlreadyInUse(userID, username, phoneNumberOrHas
 //  2. the name of the default namespace
 //
 // If the user doesn't have access to any Space, then empty strings are returned
-func GetDefaultUserTarget(provider ResourceProvider, spaceName, murName string) (string, string) {
+func GetDefaultUserTarget(cl namespaced.Client, spaceName, murName string) (string, string) {
 	if spaceName == "" {
-		sbSelector, err := labels.NewRequirement(toolchainv1alpha1.SpaceBindingMasterUserRecordLabelKey, selection.Equals, []string{murName})
-		if err != nil {
-			log.Errorf(nil, err, "unable to create spacebindings selector for MUR %s", murName)
-			return "", ""
-		}
-
-		requirements := []labels.Requirement{*sbSelector}
-
-		sbs, err := provider.ListSpaceBindings(requirements...)
-		if err != nil {
+		sbs := &toolchainv1alpha1.SpaceBindingList{}
+		if err := cl.List(gocontext.TODO(), sbs, client.InNamespace(cl.Namespace),
+			client.MatchingLabels{toolchainv1alpha1.SpaceBindingMasterUserRecordLabelKey: murName}); err != nil {
 			log.Errorf(nil, err, "unable to list spacebindings for MUR %s", murName)
 			return "", ""
 		}
-		if len(sbs) == 0 {
+		if len(sbs.Items) == 0 {
 			return "", ""
 		}
-		spaceNames := make([]string, len(sbs))
-		for i, sb := range sbs {
+		spaceNames := make([]string, len(sbs.Items))
+		for i, sb := range sbs.Items {
 			spaceNames[i] = sb.Spec.Space
 		}
 		sort.Strings(spaceNames)
 		spaceName = spaceNames[0]
 
 	}
-	space, err := provider.GetSpace(spaceName)
-	if err != nil {
+	space := &toolchainv1alpha1.Space{}
+	if err := cl.Get(gocontext.TODO(), cl.NamespacedName(spaceName), space); err != nil {
 		// log error and continue so that the api behaves in a best effort manner
 		// ie. if a space isn't listed something went wrong but we still want to return the other spaces if possible
 		log.Errorf(nil, err, "unable to get space '%s'", spaceName)