From 7bf95f136f4c815fcccf3d2c3961d5b20cabae2e Mon Sep 17 00:00:00 2001 From: frx Date: Thu, 9 Jul 2026 13:55:55 +0000 Subject: [PATCH] fix(issue-16): fix critical transaction bug and high-severity repository issues --- .../adapters/sqlite_course_repository.go | 26 ++++++++++-- .../adapters/sqlite_course_repository_test.go | 32 ++++++++++++++ .../sqlite_organization_repository.go | 2 +- .../sqlite_organization_repository_test.go | 42 +++++++++++++++++++ 4 files changed, 97 insertions(+), 5 deletions(-) diff --git a/internal/kurious/adapters/sqlite_course_repository.go b/internal/kurious/adapters/sqlite_course_repository.go index 830f5c8..703d352 100644 --- a/internal/kurious/adapters/sqlite_course_repository.go +++ b/internal/kurious/adapters/sqlite_course_repository.go @@ -9,6 +9,7 @@ import ( "strings" "time" + cerrors "git.loyso.art/frx/kurious/internal/common/errors" "git.loyso.art/frx/kurious/internal/common/nullable" "git.loyso.art/frx/kurious/internal/common/xslices" "git.loyso.art/frx/kurious/internal/kurious/domain" @@ -76,6 +77,20 @@ func (r *sqliteCourseRepository) List( if !params.Ascending { direction = "DESC" } + + allowedOrderBy := map[string]bool{ + "id": true, + "name": true, + "created_at": true, + "updated_at": true, + "full_price": true, + "discount": true, + "duration": true, + } + if !allowedOrderBy[params.OrderBy] { + return result, fmt.Errorf("invalid order by field: %s", params.OrderBy) + } + query += " ORDER BY " + params.OrderBy + " " + direction if params.Limit > 0 { @@ -277,6 +292,9 @@ func (r *sqliteCourseRepository) Get( var courseDB sqliteCourseDB err = r.db.GetContext(ctx, &courseDB, query, id) if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return course, cerrors.ErrNotFound + } return course, fmt.Errorf("executing query: %w", err) } @@ -289,7 +307,7 @@ func (r *sqliteCourseRepository) GetByExternalID( return course, errors.New("not implemented") } -func (r *sqliteCourseRepository) CreateBatch(ctx context.Context, params ...domain.CreateCourseParams) error { +func (r *sqliteCourseRepository) CreateBatch(ctx context.Context, params ...domain.CreateCourseParams) (err error) { tx, err := r.db.BeginTxx(ctx, &sql.TxOptions{Isolation: sql.LevelDefault}) if err != nil { return fmt.Errorf("beginning tx: %w", err) @@ -317,9 +335,9 @@ func (r *sqliteCourseRepository) CreateBatch(ctx context.Context, params ...doma } for _, param := range params { - _, err := stmt.ExecContext(ctx, createCourseParamsAsValues(param)...) - if err != nil { - return fmt.Errorf("executing statement query: %w", err) + _, execErr := stmt.ExecContext(ctx, createCourseParamsAsValues(param)...) + if execErr != nil { + return fmt.Errorf("executing statement query: %w", execErr) } } diff --git a/internal/kurious/adapters/sqlite_course_repository_test.go b/internal/kurious/adapters/sqlite_course_repository_test.go index 8691c60..cc7586b 100644 --- a/internal/kurious/adapters/sqlite_course_repository_test.go +++ b/internal/kurious/adapters/sqlite_course_repository_test.go @@ -1,10 +1,12 @@ package adapters import ( + "errors" "strconv" "testing" "time" + cerrors "git.loyso.art/frx/kurious/internal/common/errors" "git.loyso.art/frx/kurious/internal/common/nullable" "git.loyso.art/frx/kurious/internal/kurious/domain" @@ -105,3 +107,33 @@ func (s *sqliteCourseRepositorySuite) TestListLimitOffset() { s.NoError(err) s.Empty(result.Courses) } + +func (s *sqliteCourseRepositorySuite) TestCreateBatchPartialFailure() { + cr := s.connection.CourseRepository() + + baseParams := domain.CreateCourseParams{ + SourceType: domain.SourceTypeManual, + } + + first := baseParams + first.ID = "dup-id" + + duplicate := baseParams + duplicate.ID = "dup-id" + + err := cr.CreateBatch(s.ctx, first, duplicate) + s.Require().Error(err) + + result, listErr := cr.List(s.ctx, domain.ListCoursesParams{}) + s.Require().NoError(listErr) + s.Empty(result.Courses, "partial batch should be rolled back, not committed") + s.Zero(result.Count, "partial batch should be rolled back, not committed") +} + +func (s *sqliteCourseRepositorySuite) TestGetNotFound() { + cr := s.connection.CourseRepository() + + _, err := cr.Get(s.ctx, "does-not-exist") + s.Require().Error(err) + s.True(errors.Is(err, cerrors.ErrNotFound), "expected ErrNotFound, got %v", err) +} diff --git a/internal/kurious/adapters/sqlite_organization_repository.go b/internal/kurious/adapters/sqlite_organization_repository.go index 68132e4..6e2f718 100644 --- a/internal/kurious/adapters/sqlite_organization_repository.go +++ b/internal/kurious/adapters/sqlite_organization_repository.go @@ -187,7 +187,7 @@ func (r *sqliteOrganizationRepository) List(ctx context.Context, params domain.L }() organizations := make([]organizationDB, 0, 1<<8) - err = r.db.SelectContext(ctx, &organizations, query) + err = r.db.SelectContext(ctx, &organizations, query, args...) if err != nil { return nil, fmt.Errorf("executing query: %w", err) } diff --git a/internal/kurious/adapters/sqlite_organization_repository_test.go b/internal/kurious/adapters/sqlite_organization_repository_test.go index 4914576..11beb8c 100644 --- a/internal/kurious/adapters/sqlite_organization_repository_test.go +++ b/internal/kurious/adapters/sqlite_organization_repository_test.go @@ -126,3 +126,45 @@ func (s *sqliteOrganzationRepositorySuite) TestCreate() { expectedOrganization.UpdatedAt = gotOrganization.UpdatedAt s.Equal(expectedOrganization, gotOrganization) } + +func (s *sqliteOrganzationRepositorySuite) TestListByIDs() { + const itemscount = 3 + baseOrg := domain.Organization{ + Alias: "test-alias", + Name: "test-name", + Site: "test-site", + LogoLink: "test-logo", + } + + ids := make([]string, 0, itemscount) + for i := 0; i < itemscount; i++ { + iStr := strconv.Itoa(i) + id := "test-id-" + iStr + ids = append(ids, id) + + _, err := s.connection.Organization().Create(s.ctx, domain.CreateOrganizationParams{ + ID: id, + ExternalID: nullable.NewValue("test-ext-id-" + iStr), + Alias: baseOrg.Alias, + Name: baseOrg.Name, + Site: baseOrg.Site, + LogoLink: baseOrg.LogoLink, + }) + s.Require().NoError(err) + } + + wantedIDs := ids[:2] + + gotOrgs, err := s.connection.Organization().List(s.ctx, domain.ListOrganizationsParams{ + IDs: wantedIDs, + }) + s.Require().NoError(err) + + gotIDs := make([]string, 0, len(gotOrgs)) + for _, o := range gotOrgs { + gotIDs = append(gotIDs, o.ID) + } + + s.Len(gotOrgs, len(wantedIDs)) + s.ElementsMatch(wantedIDs, gotIDs) +}