drop unnecessary SignupService methods (#480)

pkg/application/service/services.go

@@ -9,10 +9,8 @@ import (
 
 type SignupService interface {
 	Signup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, error)
-	GetSignup(ctx *gin.Context, userID, username string) (*signup.Signup, error)
-	GetSignupFromInformer(ctx *gin.Context, userID, username string, checkUserSignupCompleted bool) (*signup.Signup, error)
+	GetSignup(ctx *gin.Context, userID, username string, checkUserSignupCompleted bool) (*signup.Signup, error)
 	GetUserSignupFromIdentifier(userID, username string) (*toolchainv1alpha1.UserSignup, error)
-	UpdateUserSignup(userSignup *toolchainv1alpha1.UserSignup) (*toolchainv1alpha1.UserSignup, error)
 	PhoneNumberAlreadyInUse(userID, username, phoneNumberOrHash string) error
 }
 

pkg/controller/signup.go

@@ -111,7 +111,7 @@ func (s *Signup) GetHandler(ctx *gin.Context) {
 	// Get the UserSignup resource from the service by the userID
 	userID := ctx.GetString(context.SubKey)
 	username := ctx.GetString(context.UsernameKey)
-	signupResource, err := s.app.SignupService().GetSignup(ctx, userID, username)
+	signupResource, err := s.app.SignupService().GetSignup(ctx, userID, username, true)
 	if err != nil {
 		log.Error(ctx, err, "error getting UserSignup resource")
 		crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "error getting UserSignup resource")

pkg/controller/signup_test.go

@@ -207,7 +207,7 @@ func (s *TestSignupSuite) TestSignupGetHandler() {
 				Reason: "Provisioning",
 			},
 		}
-		svc.MockGetSignup = func(_ *gin.Context, id, _ string) (*signup.Signup, error) {
+		svc.MockGetSignup = func(_ *gin.Context, id, _ string, _ bool) (*signup.Signup, error) {
 			if id == userID {
 				return expected, nil
 			}
@@ -234,7 +234,7 @@ func (s *TestSignupSuite) TestSignupGetHandler() {
 		ctx.Request = req
 		ctx.Set(context.SubKey, userID)
 
-		svc.MockGetSignup = func(_ *gin.Context, _, _ string) (*signup.Signup, error) {
+		svc.MockGetSignup = func(_ *gin.Context, _, _ string, _ bool) (*signup.Signup, error) {
 			return nil, nil
 		}
 
@@ -251,7 +251,7 @@ func (s *TestSignupSuite) TestSignupGetHandler() {
 		ctx.Request = req
 		ctx.Set(context.SubKey, userID)
 
-		svc.MockGetSignup = func(_ *gin.Context, _, _ string) (*signup.Signup, error) {
+		svc.MockGetSignup = func(_ *gin.Context, _, _ string, _ bool) (*signup.Signup, error) {
 			return nil, errors.New("oopsie woopsie")
 		}
 
@@ -439,9 +439,6 @@ func (s *TestSignupSuite) TestInitVerificationHandler() {
 				states.SetVerificationRequired(&us, true)
 				return &us, nil
 			},
-			MockUpdateUserSignup: func(userSignup *crtapi.UserSignup) (userSignup2 *crtapi.UserSignup, e error) {
-				return userSignup, nil
-			},
 			MockPhoneNumberAlreadyInUse: func(_, _, _ string) error {
 				return nil
 			},
@@ -863,20 +860,14 @@ func initActivationCodeVerification(t *testing.T, handler gin.HandlerFunc, usern
 }
 
 type FakeSignupService struct {
-	MockGetSignup                   func(ctx *gin.Context, userID, username string) (*signup.Signup, error)
-	MockGetSignupFromInformer       func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error)
+	MockGetSignup                   func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error)
 	MockSignup                      func(ctx *gin.Context) (*crtapi.UserSignup, error)
 	MockGetUserSignupFromIdentifier func(userID, username string) (*crtapi.UserSignup, error)
-	MockUpdateUserSignup            func(userSignup *crtapi.UserSignup) (*crtapi.UserSignup, error)
 	MockPhoneNumberAlreadyInUse     func(userID, username, value string) error
 }
 
-func (m *FakeSignupService) GetSignup(ctx *gin.Context, userID, username string) (*signup.Signup, error) {
-	return m.MockGetSignup(ctx, userID, username)
-}
-
-func (m *FakeSignupService) GetSignupFromInformer(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) {
-	return m.MockGetSignupFromInformer(ctx, userID, username, checkUserSignupComplete)
+func (m *FakeSignupService) GetSignup(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) {
+	return m.MockGetSignup(ctx, userID, username, checkUserSignupComplete)
 }
 
 func (m *FakeSignupService) Signup(ctx *gin.Context) (*crtapi.UserSignup, error) {
@@ -887,10 +878,6 @@ func (m *FakeSignupService) GetUserSignupFromIdentifier(userID, username string)
 	return m.MockGetUserSignupFromIdentifier(userID, username)
 }
 
-func (m *FakeSignupService) UpdateUserSignup(userSignup *crtapi.UserSignup) (*crtapi.UserSignup, error) {
-	return m.MockUpdateUserSignup(userSignup)
-}
-
 func (m *FakeSignupService) PhoneNumberAlreadyInUse(userID, username, e164phoneNumber string) error {
 	return m.MockPhoneNumberAlreadyInUse(userID, username, e164phoneNumber)
 }

pkg/proxy/handlers/spacelister.go

@@ -35,7 +35,7 @@ type SpaceLister struct {
 func NewSpaceLister(client namespaced.Client, app application.Application, proxyMetrics *metrics.ProxyMetrics) *SpaceLister {
 	return &SpaceLister{
 		Client:        client,
-		GetSignupFunc: app.SignupService().GetSignupFromInformer,
+		GetSignupFunc: app.SignupService().GetSignup,
 		ProxyMetrics:  proxyMetrics,
 	}
 }

pkg/proxy/handlers/spacelister_get_test.go

@@ -548,7 +548,7 @@ func testSpaceListerGet(t *testing.T, publicViewerEnabled bool) {
 					tc.mockFakeClient(fakeClient)
 				}
 
-				signupProvider := fakeSignupService.GetSignupFromInformer
+				signupProvider := fakeSignupService.GetSignup
 				if tc.overrideSignupFunc != nil {
 					signupProvider = tc.overrideSignupFunc
 				}
@@ -724,7 +724,7 @@ func TestGetUserWorkspace(t *testing.T) {
 				tc.mockFakeClient(fakeClient)
 			}
 
-			signupProvider := fakeSignupService.GetSignupFromInformer
+			signupProvider := fakeSignupService.GetSignup
 			if tc.overrideSignupFunc != nil {
 				signupProvider = tc.overrideSignupFunc
 			}
@@ -834,7 +834,7 @@ func TestSpaceListerGetPublicViewerEnabled(t *testing.T) {
 
 		t.Run(k, func(t *testing.T) {
 			// given
-			signupProvider := fakeSignupService.GetSignupFromInformer
+			signupProvider := fakeSignupService.GetSignup
 
 			proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry())
 			s := &handlers.SpaceLister{
@@ -969,7 +969,7 @@ func TestGetUserWorkspaceWithBindingsWithPublicViewerEnabled(t *testing.T) {
 
 		t.Run(k, func(t *testing.T) {
 			// given
-			signupProvider := fakeSignupService.GetSignupFromInformer
+			signupProvider := fakeSignupService.GetSignup
 
 			proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry())
 			s := &handlers.SpaceLister{

pkg/proxy/handlers/spacelister_list_test.go

@@ -71,7 +71,7 @@ func TestListUserWorkspaces(t *testing.T) {
 
 		t.Run(k, func(t *testing.T) {
 			// given
-			signupProvider := fakeSignupService.GetSignupFromInformer
+			signupProvider := fakeSignupService.GetSignup
 
 			proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry())
 
@@ -196,7 +196,7 @@ func TestHandleSpaceListRequest(t *testing.T) {
 						tc.mockFakeClient(fakeClient)
 					}
 
-					signupProvider := fakeSignupService.GetSignupFromInformer
+					signupProvider := fakeSignupService.GetSignup
 					if tc.overrideSignupFunc != nil {
 						signupProvider = tc.overrideSignupFunc
 					}

pkg/proxy/proxy.go

@@ -378,7 +378,7 @@ func (p *Proxy) checkUserIsProvisioned(ctx echo.Context, userID, username string
 	//
 	// UserSignup complete status is not checked, since it might cause the proxy blocking the request
 	// and returning an error when quick transitions from ready to provisioning are happening.
-	userSignup, err := p.app.SignupService().GetSignupFromInformer(nil, userID, username, false)
+	userSignup, err := p.app.SignupService().GetSignup(nil, userID, username, false)
 	if err != nil {
 		return err
 	}
@@ -414,7 +414,7 @@ func (p *Proxy) getClusterAccess(ctx echo.Context, userID, username, proxyPlugin
 // this function returns an error.
 func (p *Proxy) getClusterAccessAsUserOrPublicViewer(ctx echo.Context, userID, username, proxyPluginName string, workspace *toolchainv1alpha1.Workspace) (*access.ClusterAccess, error) {
 	// retrieve the requesting user's UserSignup
-	userSignup, err := p.app.SignupService().GetSignupFromInformer(nil, userID, username, false)
+	userSignup, err := p.app.SignupService().GetSignup(nil, userID, username, false)
 	if err != nil {
 		log.Error(nil, err, fmt.Sprintf("error retrieving user signup for userID '%s' and username '%s'", userID, username))
 		return nil, crterrors.NewInternalError(errs.New("unable to get user info"), "error retrieving user")

pkg/proxy/proxy_community_test.go

@@ -153,7 +153,7 @@ func (s *TestProxySuite) checkProxyCommunityOK(fakeApp *fake.ProxyFakeApp, p *Pr
 		// configure proxy
 		p.spaceLister = &handlers.SpaceLister{
 			Client:        p.Client,
-			GetSignupFunc: fakeApp.SignupServiceMock.GetSignupFromInformer,
+			GetSignupFunc: fakeApp.SignupServiceMock.GetSignup,
 			ProxyMetrics:  p.metrics,
 		}
 

pkg/proxy/proxy_test.go

@@ -852,7 +852,7 @@ func (s *TestProxySuite) checkProxyOK(fakeApp *fake.ProxyFakeApp, p *Proxy) {
 
 									p.spaceLister = &handlers.SpaceLister{
 										Client:        p.Client,
-										GetSignupFunc: fakeApp.SignupServiceMock.GetSignupFromInformer,
+										GetSignupFunc: fakeApp.SignupServiceMock.GetSignup,
 										ProxyMetrics:  p.metrics,
 									}
 									if tc.OverrideGetSignupFunc != nil {

pkg/proxy/service/cluster_service.go

@@ -100,7 +100,7 @@ func (s *ServiceImpl) getClusterAccessForDefaultWorkspace(userID, username, prox
 func (s *ServiceImpl) getSignupFromInformerForProvisionedUser(userID, username string) (*signup.Signup, error) {
 	// don't check for usersignup complete status, since it might cause the proxy blocking the request
 	// and returning an error when quick transitions from ready to provisioning are happening.
-	userSignup, err := s.SignupService.GetSignupFromInformer(nil, userID, username, false)
+	userSignup, err := s.SignupService.GetSignup(nil, userID, username, false)
 	if err != nil {
 		return nil, err
 	}

pkg/signup/service/signup_service.go

@@ -350,16 +350,10 @@ func (s *ServiceImpl) reactivateUserSignup(ctx *gin.Context, existing *toolchain
 
 // 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.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) {
+// 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, checkUserSignupCompleted bool) (*signup.Signup, error) {
 	return s.DoGetSignup(ctx, s.Client, userID, username, checkUserSignupCompleted)
 }
 
@@ -388,10 +382,8 @@ func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, cl namespaced.Client, userID
 		// If there is no need to update the UserSignup then break out of the loop here (by returning nil)
 		// otherwise update the UserSignup
 		if updated {
-			var updateErr error
-			userSignup, updateErr = s.UpdateUserSignup(userSignup)
-			if updateErr != nil {
-				return updateErr
+			if err := s.Update(gocontext.TODO(), userSignup); err != nil {
+				return err
 			}
 		}
 
@@ -582,15 +574,6 @@ func (s *ServiceImpl) DoGetUserSignupFromIdentifier(cl namespaced.Client, userID
 	return userSignup, nil
 }
 
-// UpdateUserSignup is used to update the provided UserSignup resource, and returning the updated resource
-func (s *ServiceImpl) UpdateUserSignup(userSignup *toolchainv1alpha1.UserSignup) (*toolchainv1alpha1.UserSignup, error) {
-	if err := s.Update(gocontext.TODO(), userSignup); err != nil {
-		return nil, err
-	}
-
-	return userSignup, nil
-}
-
 var (
 	md5Matcher = regexp.MustCompile("(?i)[a-f0-9]{32}$")
 )