From c705cb7e49a1b45f3bc1afd3874072da8ce202b6 Mon Sep 17 00:00:00 2001 From: Dionysos <75300347+ice-dionysos@users.noreply.github.com> Date: Mon, 15 Jan 2024 09:59:09 +0100 Subject: [PATCH 1/3] kyc/quiz: return Failure for continue on expired session --- kyc/quiz/quiz.go | 14 +++++++++++--- kyc/quiz/quiz_test.go | 9 +++++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/kyc/quiz/quiz.go b/kyc/quiz/quiz.go index d2264f60..e8c35a52 100644 --- a/kyc/quiz/quiz.go +++ b/kyc/quiz/quiz.go @@ -99,9 +99,11 @@ func (r *repositoryImpl) validateKycStep(user *users.User) error { func (r *repositoryImpl) SkipQuizSession(ctx context.Context, userID UserID) error { //nolint:funlen //. // $1: user_id. + // $2: max session duration (seconds). const stmt = ` select started_at, + started_at + make_interval(secs => $2) as deadline, ended_at is not null as finished, ended_successfully from @@ -120,9 +122,10 @@ func (r *repositoryImpl) SkipQuizSession(ctx context.Context, userID UserID) err data, err := storage.ExecOne[struct { StartedAt *time.Time `db:"started_at"` + Deadline *time.Time `db:"deadline"` Finished bool `db:"finished"` Success bool `db:"ended_successfully"` - }](ctx, tx, stmt, userID) + }](ctx, tx, stmt, userID, r.config.MaxSessionDurationSeconds) if err != nil { if errors.Is(err, storage.ErrNotFound) { return wrapErrorInTx(ErrUnknownSession) @@ -135,7 +138,7 @@ func (r *repositoryImpl) SkipQuizSession(ctx context.Context, userID UserID) err case data.StartedAt == nil: return wrapErrorInTx(ErrUnknownSession) - case data.StartedAt.Add(stdlibtime.Duration(r.config.MaxSessionDurationSeconds) * stdlibtime.Second).Before(*now.Time): + case data.Deadline.Before(*now.Time): return wrapErrorInTx(ErrSessionExpired) case data.Finished: @@ -555,7 +558,7 @@ func (r *repositoryImpl) modifyUser(ctx context.Context, success bool, now *time return errors.Wrapf(r.Users.ModifyUser(ctx, usr, nil), "failed to modify user %#v", usr) } -func (r *repositoryImpl) ContinueQuizSession( //nolint:funlen,revive //. +func (r *repositoryImpl) ContinueQuizSession( //nolint:funlen,revive,gocognit //. ctx context.Context, userID UserID, question, answer uint8, @@ -564,6 +567,11 @@ func (r *repositoryImpl) ContinueQuizSession( //nolint:funlen,revive //. now := stdlibtime.Now().Truncate(stdlibtime.Second).UTC() progress, pErr := r.CheckUserRunningSession(ctx, userID, now, tx) if pErr != nil { + if errors.Is(pErr, ErrSessionExpired) { + quiz = &Quiz{Result: FailureResult} + pErr = r.UserMarkSessionAsFinished(ctx, userID, now, tx, false, false) + } + return wrapErrorInTx(pErr) } _, err = r.CheckQuestionNumber(ctx, progress.Lang, progress.Questions, question, tx) diff --git a/kyc/quiz/quiz_test.go b/kyc/quiz/quiz_test.go index d28f2315..f6ff25da 100644 --- a/kyc/quiz/quiz_test.go +++ b/kyc/quiz/quiz_test.go @@ -280,8 +280,13 @@ func testManagerSessionContinueErrors(ctx context.Context, t *testing.T, r *repo data, err := r.StartQuizSession(ctx, "bogus", "en") require.NoError(t, err) helperForceResetSessionStartedAt(t, r, "bogus") - _, err = r.ContinueQuizSession(ctx, "bogus", data.Progress.NextQuestion.Number, 1) - require.ErrorIs(t, err, ErrSessionExpired) + data, err = r.ContinueQuizSession(ctx, "bogus", data.Progress.NextQuestion.Number, 1) + require.NoError(t, err) + require.Nil(t, data.Progress) + require.Equal(t, FailureResult, data.Result) + + _, err = r.StartQuizSession(ctx, "bogus", "en") + require.ErrorIs(t, err, ErrSessionFinishedWithError) }) t.Run("UnknownQuestionNumber", func(t *testing.T) { From 7a30ad3b0ce934344dfd3cc91cb9ed1f2e4436be Mon Sep 17 00:00:00 2001 From: Dionysos <75300347+ice-dionysos@users.noreply.github.com> Date: Mon, 15 Jan 2024 11:21:32 +0100 Subject: [PATCH 2/3] kyc/quiz: finish expired session before starting a new one --- kyc/quiz/quiz.go | 93 ++++++++++++++++++++++++++++++++++++++----- kyc/quiz/quiz_test.go | 9 +---- 2 files changed, 85 insertions(+), 17 deletions(-) diff --git a/kyc/quiz/quiz.go b/kyc/quiz/quiz.go index e8c35a52..5b429411 100644 --- a/kyc/quiz/quiz.go +++ b/kyc/quiz/quiz.go @@ -202,17 +202,63 @@ func wrapErrorInTx(err error) error { return err } -func (r *repositoryImpl) StartQuizSession(ctx context.Context, userID UserID, lang string) (*Quiz, error) { //nolint:funlen //. - err := r.CheckUserKYC(ctx, userID) +func (r *repositoryImpl) finishExpiredSession( //nolint:funlen //. + ctx context.Context, + userID UserID, + now *time.Time, + tx storage.QueryExecer, +) (*time.Time, error) { + // $1: user_id. + // $2: max session duration (seconds). + // $3: session cool down (seconds). + const stmt = ` + with result as ( + update quiz_sessions + set + ended_at = now(), + ended_successfully = false + where + user_id = $1 and + ended_at is null and + started_at + make_interval(secs => $2) < now() + returning * + ) + insert into failed_quiz_sessions (started_at, ended_at, questions, answers, language, user_id, skipped) + select + result.started_at, + result.ended_at, + result.questions, + result.answers, + result.language, + result.user_id, + false + from + result + returning + ended_at + make_interval(secs => $3) as cooldown_at + ` + data, err := storage.ExecOne[struct { + CooldownAt *time.Time `db:"cooldown_at"` + }](ctx, tx, stmt, userID, r.config.MaxSessionDurationSeconds, r.config.SessionCoolDownSeconds) if err != nil { - return nil, err - } + if errors.Is(err, storage.ErrNotFound) { + err = nil + } - questions, err := r.SelectQuestions(ctx, r.DB, lang) - if err != nil { return nil, err } + return data.CooldownAt, errors.Wrapf(r.modifyUser(ctx, false, now, userID), "failed to modifyUser") +} + +func (r *repositoryImpl) startNewSession( //nolint:funlen,revive //. + ctx context.Context, + userID UserID, + now *time.Time, + tx storage.QueryExecer, + lang string, + questions []*Question, +) (*Quiz, error) { // $1: user_id. // $2: language. // $3: questions. @@ -297,7 +343,7 @@ func (r *repositoryImpl) StartQuizSession(ctx context.Context, userID UserID, la ActiveEndedAt *time.Time `db:"active_ended_at"` UpsertStartedAt *time.Time `db:"upsert_started_at"` UpsertDeadline *time.Time `db:"upsert_deadline"` - }](ctx, r.DB, stmt, userID, lang, questionsToSlice(questions), r.config.SessionCoolDownSeconds, r.config.MaxSessionDurationSeconds) + }](ctx, tx, stmt, userID, lang, questionsToSlice(questions), r.config.SessionCoolDownSeconds, r.config.MaxSessionDurationSeconds) if err != nil { if errors.Is(err, storage.ErrRelationNotFound) { err = ErrUnknownUser @@ -306,7 +352,6 @@ func (r *repositoryImpl) StartQuizSession(ctx context.Context, userID UserID, la return nil, errors.Wrap(err, "failed to start session") } - now := stdlibtime.Now().Truncate(stdlibtime.Second).UTC() switch { case data.FailedAt != nil: // Failed session is still in cool down. return nil, errors.Wrapf(ErrSessionFinishedWithError, "wait until %v", @@ -321,8 +366,8 @@ func (r *repositoryImpl) StartQuizSession(ctx context.Context, userID UserID, la return nil, ErrSessionFinishedWithError } - if data.ActiveDeadline.After(now) { - return nil, errors.Wrapf(ErrSessionIsAlreadyRunning, "wait %s before next session", data.ActiveDeadline.Sub(now)) + if data.ActiveDeadline.After(*now.Time) { + return nil, errors.Wrapf(ErrSessionIsAlreadyRunning, "wait %s before next session", data.ActiveDeadline.Sub(*now.Time)) } case data.UpsertStartedAt != nil: // New session is started. @@ -338,6 +383,34 @@ func (r *repositoryImpl) StartQuizSession(ctx context.Context, userID UserID, la panic("unreachable: " + userID) } +func (r *repositoryImpl) StartQuizSession(ctx context.Context, userID UserID, lang string) (quiz *Quiz, err error) { + err = r.CheckUserKYC(ctx, userID) + if err != nil { + return nil, err + } + + questions, err := r.SelectQuestions(ctx, r.DB, lang) + if err != nil { + return nil, err + } + + err = storage.DoInTransaction(ctx, r.DB, func(tx storage.QueryExecer) error { + now := time.Now() + cooldown, fErr := r.finishExpiredSession(ctx, userID, now, tx) + if fErr != nil { + return wrapErrorInTx(fErr) + } else if cooldown != nil { + return wrapErrorInTx(errors.Wrapf(ErrSessionFinishedWithError, "wait until %v", cooldown)) + } + + quiz, err = r.startNewSession(ctx, userID, now, tx, lang, questions) + + return wrapErrorInTx(err) + }) + + return quiz, err +} + func calculateProgress(correctAnswers, currentAnswers []uint8) (correctNum, incorrectNum uint8) { correct := correctAnswers if len(currentAnswers) < len(correctAnswers) { diff --git a/kyc/quiz/quiz_test.go b/kyc/quiz/quiz_test.go index f6ff25da..39b49e8f 100644 --- a/kyc/quiz/quiz_test.go +++ b/kyc/quiz/quiz_test.go @@ -164,13 +164,8 @@ func testManagerSessionStart(ctx context.Context, t *testing.T, r *repositoryImp t.Run("Expired", func(t *testing.T) { helperForceResetSessionStartedAt(t, r, "bogus") session, err := r.StartQuizSession(ctx, "bogus", "en") - require.NoError(t, err) - require.NotNil(t, session) - require.NotNil(t, session.Progress) - require.NotNil(t, session.Progress.ExpiresAt) - require.NotEmpty(t, session.Progress.NextQuestion) - require.Equal(t, uint8(3), session.Progress.MaxQuestions) - require.Equal(t, uint8(1), session.Progress.NextQuestion.Number) + require.ErrorIs(t, err, ErrSessionFinishedWithError) + require.Nil(t, session) }) t.Run("CoolDown", func(t *testing.T) { helperSessionReset(t, r, "bogus", true) From 25386db9992d2aa9afa307f1a8e2410dad968506 Mon Sep 17 00:00:00 2001 From: Dionysos <75300347+ice-dionysos@users.noreply.github.com> Date: Mon, 15 Jan 2024 12:42:51 +0100 Subject: [PATCH 3/3] review comments --- cmd/eskimo-hut/kyc.go | 4 ++-- kyc/quiz/contract.go | 5 +++-- kyc/quiz/quiz.go | 34 +++++++++++----------------------- kyc/quiz/quiz_test.go | 2 +- 4 files changed, 17 insertions(+), 28 deletions(-) diff --git a/cmd/eskimo-hut/kyc.go b/cmd/eskimo-hut/kyc.go index f1419fe8..85e5cdae 100644 --- a/cmd/eskimo-hut/kyc.go +++ b/cmd/eskimo-hut/kyc.go @@ -108,7 +108,7 @@ func (s *service) StartOrContinueKYCStep4Session( //nolint:gocritic,funlen // . case errors.Is(err, kycquiz.ErrUnknownUser) || errors.Is(err, kycquiz.ErrUnknownSession): return nil, server.NotFound(err, userNotFoundErrorCode) - case errors.Is(err, kycquiz.ErrSessionExpired), errors.Is(err, kycquiz.ErrSessionFinished), errors.Is(err, kycquiz.ErrSessionFinishedWithError), errors.Is(err, kycquiz.ErrInvalidKYCState): //nolint:lll // . + case errors.Is(err, kycquiz.ErrSessionFinished), errors.Is(err, kycquiz.ErrSessionFinishedWithError), errors.Is(err, kycquiz.ErrInvalidKYCState): //nolint:lll // . return nil, server.BadRequest(err, raceConditionErrorCode) case errors.Is(err, kycquiz.ErrUnknownQuestionNumber): @@ -244,7 +244,7 @@ func (s *service) TryResetKYCSteps( //nolint:gocritic,funlen,gocognit,revive,cyc } case users.QuizKYCStep: if err := s.quizRepository.SkipQuizSession(ctx, req.Data.UserID); err != nil { - if errors.Is(err, kycquiz.ErrInvalidKYCState) || errors.Is(err, kycquiz.ErrSessionFinished) || errors.Is(err, kycquiz.ErrSessionFinishedWithError) || errors.Is(err, kycquiz.ErrSessionExpired) { //nolint:lll // . + if errors.Is(err, kycquiz.ErrInvalidKYCState) || errors.Is(err, kycquiz.ErrSessionFinished) || errors.Is(err, kycquiz.ErrSessionFinishedWithError) { //nolint:lll // . log.Error(errors.Wrapf(err, "skipQuizSession failed unexpectedly during tryResetKYCSteps for userID:%v", req.Data.UserID)) err = nil } diff --git a/kyc/quiz/contract.go b/kyc/quiz/contract.go index 7e9611c8..f72207cf 100644 --- a/kyc/quiz/contract.go +++ b/kyc/quiz/contract.go @@ -69,7 +69,6 @@ var ( ErrSessionIsAlreadyRunning = newError("another session is already running") ErrSessionFinished = newError("session closed") ErrSessionFinishedWithError = newError("session closed with error") - ErrSessionExpired = newError("session expired") ErrUnknownQuestionNumber = newError("unknown question number") ErrUnknownSession = newError("unknown session and/or user") ) @@ -78,9 +77,11 @@ const ( applicationYamlKey = "kyc/quiz" ) -var ( //nolint:gofumpt //. +var ( //go:embed DDL.sql ddl string + + errSessionExpired = newError("session expired") ) type ( diff --git a/kyc/quiz/quiz.go b/kyc/quiz/quiz.go index 5b429411..35710802 100644 --- a/kyc/quiz/quiz.go +++ b/kyc/quiz/quiz.go @@ -99,11 +99,8 @@ func (r *repositoryImpl) validateKycStep(user *users.User) error { func (r *repositoryImpl) SkipQuizSession(ctx context.Context, userID UserID) error { //nolint:funlen //. // $1: user_id. - // $2: max session duration (seconds). const stmt = ` select - started_at, - started_at + make_interval(secs => $2) as deadline, ended_at is not null as finished, ended_successfully from @@ -121,11 +118,9 @@ func (r *repositoryImpl) SkipQuizSession(ctx context.Context, userID UserID) err now := time.Now() data, err := storage.ExecOne[struct { - StartedAt *time.Time `db:"started_at"` - Deadline *time.Time `db:"deadline"` - Finished bool `db:"finished"` - Success bool `db:"ended_successfully"` - }](ctx, tx, stmt, userID, r.config.MaxSessionDurationSeconds) + Finished bool `db:"finished"` + Success bool `db:"ended_successfully"` + }](ctx, tx, stmt, userID) if err != nil { if errors.Is(err, storage.ErrNotFound) { return wrapErrorInTx(ErrUnknownSession) @@ -134,14 +129,7 @@ func (r *repositoryImpl) SkipQuizSession(ctx context.Context, userID UserID) err return errors.Wrap(wrapErrorInTx(err), "failed to get session data") } - switch { - case data.StartedAt == nil: - return wrapErrorInTx(ErrUnknownSession) - - case data.Deadline.Before(*now.Time): - return wrapErrorInTx(ErrSessionExpired) - - case data.Finished: + if data.Finished { if data.Success { return wrapErrorInTx(ErrSessionFinished) } @@ -384,11 +372,6 @@ func (r *repositoryImpl) startNewSession( //nolint:funlen,revive //. } func (r *repositoryImpl) StartQuizSession(ctx context.Context, userID UserID, lang string) (quiz *Quiz, err error) { - err = r.CheckUserKYC(ctx, userID) - if err != nil { - return nil, err - } - questions, err := r.SelectQuestions(ctx, r.DB, lang) if err != nil { return nil, err @@ -403,6 +386,11 @@ func (r *repositoryImpl) StartQuizSession(ctx context.Context, userID UserID, la return wrapErrorInTx(errors.Wrapf(ErrSessionFinishedWithError, "wait until %v", cooldown)) } + err = r.CheckUserKYC(ctx, userID) + if err != nil { + return wrapErrorInTx(err) + } + quiz, err = r.startNewSession(ctx, userID, now, tx, lang, questions) return wrapErrorInTx(err) @@ -488,7 +476,7 @@ group by deadline := data.StartedAt.Add(stdlibtime.Duration(r.config.MaxSessionDurationSeconds) * stdlibtime.Second) if deadline.Before(now) { - return userProgress{}, ErrSessionExpired + return userProgress{}, errSessionExpired } return data.userProgress, nil @@ -640,7 +628,7 @@ func (r *repositoryImpl) ContinueQuizSession( //nolint:funlen,revive,gocognit // now := stdlibtime.Now().Truncate(stdlibtime.Second).UTC() progress, pErr := r.CheckUserRunningSession(ctx, userID, now, tx) if pErr != nil { - if errors.Is(pErr, ErrSessionExpired) { + if errors.Is(pErr, errSessionExpired) { quiz = &Quiz{Result: FailureResult} pErr = r.UserMarkSessionAsFinished(ctx, userID, now, tx, false, false) } diff --git a/kyc/quiz/quiz_test.go b/kyc/quiz/quiz_test.go index 39b49e8f..9d4ba96e 100644 --- a/kyc/quiz/quiz_test.go +++ b/kyc/quiz/quiz_test.go @@ -213,7 +213,7 @@ func testManagerSessionSkip(ctx context.Context, t *testing.T, r *repositoryImpl helperForceResetSessionStartedAt(t, r, "bogus") err = r.SkipQuizSession(ctx, "bogus") - require.ErrorIs(t, err, ErrSessionExpired) + require.NoError(t, err) }) t.Run("Finished", func(t *testing.T) { t.Run("Success", func(t *testing.T) {