Skip to content

Commit

Permalink
chore: remove unused args in functions (#1594)
Browse files Browse the repository at this point in the history
## What kind of change does this PR introduce?
* clean up unused args in various functions
  • Loading branch information
kangmingtay authored May 30, 2024
1 parent b954a48 commit 93f52fc
Show file tree
Hide file tree
Showing 10 changed files with 20 additions and 25 deletions.
4 changes: 2 additions & 2 deletions internal/api/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (a *API) runPostgresHook(ctx context.Context, tx *storage.Connection, hookC
return response, nil
}

func (a *API) runHTTPHook(r *http.Request, hookConfig conf.ExtensibilityPointConfiguration, input, output any) ([]byte, error) {
func (a *API) runHTTPHook(r *http.Request, hookConfig conf.ExtensibilityPointConfiguration, input any) ([]byte, error) {
ctx := r.Context()
client := http.Client{
Timeout: DefaultHTTPHookTimeout,
Expand Down Expand Up @@ -351,7 +351,7 @@ func (a *API) runHook(r *http.Request, conn *storage.Connection, hookConfig conf
var err error
switch strings.ToLower(scheme) {
case "http", "https":
response, err = a.runHTTPHook(r, hookConfig, input, output)
response, err = a.runHTTPHook(r, hookConfig, input)
case "pg-functions":
response, err = a.runPostgresHook(ctx, conn, hookConfig, input, output)
default:
Expand Down
16 changes: 7 additions & 9 deletions internal/api/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ import (
"testing"

"errors"
"net/http/httptest"

"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/supabase/auth/internal/conf"
"github.com/supabase/auth/internal/hooks"
"github.com/supabase/auth/internal/models"
"github.com/supabase/auth/internal/storage"
"net/http/httptest"

"gopkg.in/h2non/gock.v1"
)
Expand Down Expand Up @@ -105,13 +106,13 @@ func (ts *HooksTestSuite) TestRunHTTPHook() {

}

var output hooks.SendSMSOutput
req, _ := http.NewRequest("POST", ts.Config.Hook.SendSMS.URI, nil)
body, err := ts.API.runHTTPHook(req, ts.Config.Hook.SendSMS, &input, &output)
body, err := ts.API.runHTTPHook(req, ts.Config.Hook.SendSMS, &input)

if !tc.expectError {
require.NoError(ts.T(), err)
if body != nil {
var output hooks.SendSMSOutput
require.NoError(ts.T(), json.Unmarshal(body, &output))
require.True(ts.T(), output.Success)
}
Expand Down Expand Up @@ -149,15 +150,14 @@ func (ts *HooksTestSuite) TestShouldRetryWithRetryAfterHeader() {
Reply(http.StatusOK).
JSON(successOutput).SetHeader("content-type", "application/json")

var output hooks.SendSMSOutput

// Simulate the original HTTP request which triggered the hook
req, err := http.NewRequest("POST", "http://localhost:9998/otp", nil)
require.NoError(ts.T(), err)

body, err := ts.API.runHTTPHook(req, ts.Config.Hook.SendSMS, &input, &output)
body, err := ts.API.runHTTPHook(req, ts.Config.Hook.SendSMS, &input)
require.NoError(ts.T(), err)

var output hooks.SendSMSOutput
err = json.Unmarshal(body, &output)
require.NoError(ts.T(), err, "Unmarshal should not fail")
require.True(ts.T(), output.Success, "Expected success on retry")
Expand All @@ -184,12 +184,10 @@ func (ts *HooksTestSuite) TestShouldReturnErrorForNonJSONContentType() {
Reply(http.StatusOK).
SetHeader("content-type", "text/plain")

var output hooks.SendSMSOutput

req, err := http.NewRequest("POST", "http://localhost:9999/otp", nil)
require.NoError(ts.T(), err)

_, err = ts.API.runHTTPHook(req, ts.Config.Hook.SendSMS, &input, &output)
_, err = ts.API.runHTTPHook(req, ts.Config.Hook.SendSMS, &input)
require.Error(ts.T(), err, "Expected an error due to wrong content type")
require.Contains(ts.T(), err.Error(), "Invalid JSON response.")

Expand Down
6 changes: 3 additions & 3 deletions internal/api/identity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (ts *IdentityTestSuite) TestUnlinkIdentityError() {

for _, c := range cases {
ts.Run(c.desc, func() {
token := ts.generateAccessTokenAndSession(context.Background(), c.user, models.PasswordGrant)
token := ts.generateAccessTokenAndSession(c.user)
req, err := http.NewRequest(http.MethodDelete, fmt.Sprintf("/user/identities/%s", c.identityId), nil)
require.NoError(ts.T(), err)
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
Expand Down Expand Up @@ -183,7 +183,7 @@ func (ts *IdentityTestSuite) TestUnlinkIdentity() {
identity, err := models.FindIdentityByIdAndProvider(ts.API.db, u.ID.String(), c.provider)
require.NoError(ts.T(), err)

token := ts.generateAccessTokenAndSession(context.Background(), u, models.PasswordGrant)
token := ts.generateAccessTokenAndSession(u)
req, err := http.NewRequest(http.MethodDelete, fmt.Sprintf("/user/identities/%s", identity.ID), nil)
require.NoError(ts.T(), err)
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
Expand Down Expand Up @@ -214,7 +214,7 @@ func (ts *IdentityTestSuite) TestUnlinkIdentity() {

}

func (ts *IdentityTestSuite) generateAccessTokenAndSession(ctx context.Context, u *models.User, authenticationMethod models.AuthenticationMethod) string {
func (ts *IdentityTestSuite) generateAccessTokenAndSession(u *models.User) string {
s, err := models.NewSession(u.ID, nil)
require.NoError(ts.T(), err)
require.NoError(ts.T(), ts.API.db.Create(s))
Expand Down
2 changes: 1 addition & 1 deletion internal/api/otp.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func (a *API) SmsOtp(w http.ResponseWriter, r *http.Request) error {
if terr != nil {
return internalServerError("Unable to get SMS provider").WithInternalError(err)
}
mID, serr := a.sendPhoneConfirmation(ctx, r, tx, user, params.Phone, phoneConfirmationOtp, smsProvider, params.Channel)
mID, serr := a.sendPhoneConfirmation(r, tx, user, params.Phone, phoneConfirmationOtp, smsProvider, params.Channel)
if serr != nil {
return badRequestError(ErrorCodeSMSSendFailed, "Error sending sms OTP: %v", serr).WithInternalError(serr)
}
Expand Down
3 changes: 1 addition & 2 deletions internal/api/phone.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package api

import (
"bytes"
"context"
"net/http"
"regexp"
"strings"
Expand Down Expand Up @@ -44,7 +43,7 @@ func formatPhoneNumber(phone string) string {
}

// sendPhoneConfirmation sends an otp to the user's phone number
func (a *API) sendPhoneConfirmation(ctx context.Context, r *http.Request, tx *storage.Connection, user *models.User, phone, otpType string, smsProvider sms_provider.SmsProvider, channel string) (string, error) {
func (a *API) sendPhoneConfirmation(r *http.Request, tx *storage.Connection, user *models.User, phone, otpType string, smsProvider sms_provider.SmsProvider, channel string) (string, error) {
config := a.config

var token *string
Expand Down
4 changes: 1 addition & 3 deletions internal/api/phone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,7 @@ func doTestSendPhoneConfirmation(ts *PhoneTestSuite, useTestOTP bool) {
ts.Run(c.desc, func() {
provider := &TestSmsProvider{}

ctx := req.Context()

_, err = ts.API.sendPhoneConfirmation(ctx, req, ts.API.db, u, "123456789", c.otpType, provider, sms_provider.SMSProvider)
_, err = ts.API.sendPhoneConfirmation(req, ts.API.db, u, "123456789", c.otpType, provider, sms_provider.SMSProvider)
require.Equal(ts.T(), c.expected, err)
u, err = models.FindUserByPhoneAndAudience(ts.API.db, "123456789", ts.Config.JWT.Aud)
require.NoError(ts.T(), err)
Expand Down
2 changes: 1 addition & 1 deletion internal/api/reauthenticate.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (a *API) Reauthenticate(w http.ResponseWriter, r *http.Request) error {
if terr != nil {
return internalServerError("Failed to get SMS provider").WithInternalError(terr)
}
mID, err := a.sendPhoneConfirmation(ctx, r, tx, user, phone, phoneReauthenticationOtp, smsProvider, sms_provider.SMSProvider)
mID, err := a.sendPhoneConfirmation(r, tx, user, phone, phoneReauthenticationOtp, smsProvider, sms_provider.SMSProvider)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions internal/api/resend.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error {
if terr != nil {
return terr
}
mID, terr := a.sendPhoneConfirmation(ctx, r, tx, user, params.Phone, phoneConfirmationOtp, smsProvider, sms_provider.SMSProvider)
mID, terr := a.sendPhoneConfirmation(r, tx, user, params.Phone, phoneConfirmationOtp, smsProvider, sms_provider.SMSProvider)
if terr != nil {
return terr
}
Expand All @@ -143,7 +143,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error {
if terr != nil {
return terr
}
mID, terr := a.sendPhoneConfirmation(ctx, r, tx, user, user.PhoneChange, phoneChangeVerification, smsProvider, sms_provider.SMSProvider)
mID, terr := a.sendPhoneConfirmation(r, tx, user, user.PhoneChange, phoneChangeVerification, smsProvider, sms_provider.SMSProvider)
if terr != nil {
return terr
}
Expand Down
2 changes: 1 addition & 1 deletion internal/api/signup.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error {
if terr != nil {
return internalServerError("Unable to get SMS provider").WithInternalError(terr)
}
if _, terr := a.sendPhoneConfirmation(ctx, r, tx, user, params.Phone, phoneConfirmationOtp, smsProvider, params.Channel); terr != nil {
if _, terr := a.sendPhoneConfirmation(r, tx, user, params.Phone, phoneConfirmationOtp, smsProvider, params.Channel); terr != nil {
return unprocessableEntityError(ErrorCodeSMSSendFailed, "Error sending confirmation sms: %v", terr).WithInternalError(terr)
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/api/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
if terr != nil {
return internalServerError("Error finding SMS provider").WithInternalError(terr)
}
if _, terr := a.sendPhoneConfirmation(ctx, r, tx, user, params.Phone, phoneChangeVerification, smsProvider, params.Channel); terr != nil {
if _, terr := a.sendPhoneConfirmation(r, tx, user, params.Phone, phoneChangeVerification, smsProvider, params.Channel); terr != nil {
return internalServerError("Error sending phone change otp").WithInternalError(terr)
}
}
Expand Down

0 comments on commit 93f52fc

Please sign in to comment.