refac: stale test entries, failing web tests, Postgre backup bug, Hardcoded MySQL table count, Stale Postgre connection in test, own-scope visibility filter, GetAll ignoring its error, JSON tags in use-case types, 31 copies of DESKTOP_QUERY, ProfileView endpoint paths

test/accept_invitation_atomic_test.go

@@ -43,10 +43,10 @@ type atomicInviteRepoDecorator struct {
 	afterGetAll func()
 }
 
-func (d *atomicInviteRepoDecorator) GetAll() []invite.Invite {
-	all := d.Repository.GetAll()
+func (d *atomicInviteRepoDecorator) GetAll() ([]invite.Invite, error) {
+	all, err := d.Repository.GetAll()
 	d.afterGetAll()
-	return all
+	return all, err
 }
 
 type atomicLookupBarrier struct {

test/api_catalogue_drift_test.go

@@ -50,39 +50,15 @@ func routerRoutes(t *testing.T) map[string]bool {
 }
 
 var routesNotYetInTheCatalogue = []string{
-	"DELETE /v1/content-type/{key}",
-	"DELETE /v1/content-type/{key}/entry/{id}",
-	"DELETE /v1/image-variants/{name}",
-	"DELETE /v1/media/{id}/variants/{name}",
 	"DELETE /v1/role/{id}",
-	"GET /v1/builtin-type",
-	"GET /v1/builtin-type/{key}",
-	"GET /v1/content-type",
-	"GET /v1/content-type/{key}",
-	"GET /v1/content-type/{key}/entry",
-	"GET /v1/content-type/{key}/entry/{id}",
-	"GET /v1/image-variants",
-	"GET /v1/image-variants/cache",
-	"GET /v1/media/{id}/variants",
 	"GET /v1/user/lookup",
 	"GET /v1/permission",
 	"GET /v1/role",
 	"GET /v1/role/{id}",
-	"PATCH /v1/image-variants/{name}",
-	"POST /v1/content-type",
-	"POST /v1/content-type/{key}/entry",
-	"POST /v1/image-variants",
-	"POST /v1/image-variants/cache/purge",
-	"POST /v1/image-variants/{name}/purge",
-	"POST /v1/media/{id}/variants",
-	"POST /v1/media/{id}/variants/{name}",
 	"POST /v1/role",
 	"POST /v1/role/{id}/reassign",
 	"POST /v1/role/{id}/reset",
 	"POST /v1/site/transfer-ownership",
-	"PUT /v1/content-type/{key}",
-	"PUT /v1/content-type/{key}/entry/{id}",
-	"PUT /v1/image-variants/{name}",
 	"PUT /v1/role/{id}",
 	"PUT /v1/user/{id}/role",
 }

test/article_test.go

@@ -92,6 +92,9 @@ func (m *mockArticleRepo) FindPage(filters article.Filters) (article.PageResult,
 		if filters.SiteID != nil && a.SiteID != *filters.SiteID {
 			continue
 		}
+		if filters.VisibleTo != nil && !a.IsOnline && a.CreatedBy != *filters.VisibleTo {
+			continue
+		}
 		if filters.Status == article.StatusOnline && !a.IsOnline {
 			continue
 		}
@@ -132,6 +135,22 @@ func (m *mockArticleRepo) CountByVisibility(onlineOnly bool) (article.Counts, er
 	return counts, nil
 }
 
+func (m *mockArticleRepo) CountVisibleTo(userID uint32) (article.Counts, error) {
+	if err := m.fail.trigger("CountVisibleTo"); err != nil {
+		return article.Counts{}, err
+	}
+	counts := article.Counts{}
+	for _, a := range m.articles {
+		if a.IsOnline {
+			counts.Online++
+		} else if a.CreatedBy == userID {
+			counts.Draft++
+		}
+	}
+	counts.All = counts.Online + counts.Draft
+	return counts, nil
+}
+
 func articleSortsBefore(left, right article.Article, order article.Sort) bool {
 	if order.Field == "" {
 		order = article.DefaultSort()
@@ -195,12 +214,12 @@ func (m *mockArticleRepo) GetByName(name string) (article.Article, error) {
 	return article.Article{}, article.ErrArticleNotFound
 }
 
-func (m *mockArticleRepo) GetAll() []article.Article {
+func (m *mockArticleRepo) GetAll() ([]article.Article, error) {
 	result := make([]article.Article, 0)
 	for _, id := range m.sortedIds() {
 		result = append(result, m.articles[id])
 	}
-	return result
+	return result, nil
 }
 
 func (m *mockArticleRepo) slugUsedOn(siteID uint32, slug string, exceptID uint32) bool {

test/custom_content_types_support_test.go

@@ -146,7 +146,7 @@ func (m *mockEntryRepo) ListPage(filters entry.Filters) (entry.PageResult, error
 	}
 	matching := []entry.Entry{}
 	for _, stored := range m.entries {
-		if stored.ContentTypeID == filters.ContentTypeID {
+		if stored.ContentTypeID == filters.ContentTypeID && (filters.CreatedBy == nil || stored.CreatedBy == *filters.CreatedBy) {
 			matching = append(matching, stored)
 		}
 	}

test/get_all_error_propagation_test.go

@@ -0,0 +1,74 @@
+package test
+
+import (
+	"RenewCMS/internal/infrastructure/useCases"
+	"errors"
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestGetAllFailuresReachTheCaller(t *testing.T) {
+	boom := errors.New("storage down")
+
+	t.Run("registration status answers 500 instead of reporting open", func(t *testing.T) {
+		userRepo := newMockUserRepo()
+		userRepo.fail.failNextWith("GetAll", boom)
+		router, _ := newRegistrationStatusRouter(userRepo, newMockMailRepo())
+
+		rec := registrationStatusRequest(router, nil)
+
+		if rec.Code != http.StatusInternalServerError {
+			t.Errorf("status = %d, want 500", rec.Code)
+		}
+	})
+
+	t.Run("IsRegistrationOpen returns the error", func(t *testing.T) {
+		userRepo := newMockUserRepo()
+		userRepo.fail.failNextWith("GetAll", boom)
+		uc := useCases.NewCreateUserUseCase(userRepo, newSeededMockRoleRepo(), &stubHasher{}, newMockMailRepo(), "https://cms.example.com", discardRecorder)
+
+		if _, err := uc.IsRegistrationOpen(); !errors.Is(err, boom) {
+			t.Errorf("err = %v, want %v", err, boom)
+		}
+	})
+
+	t.Run("GetInvite returns the error instead of an invalid invite", func(t *testing.T) {
+		invites := newInviteFakeRepo()
+		invites.fail.failNextWith("GetAll", boom)
+		uc := useCases.NewGetInviteUseCase(invites, &stubHasher{})
+
+		if _, err := uc.GetInvite("code"); !errors.Is(err, boom) {
+			t.Errorf("err = %v, want %v", err, boom)
+		}
+	})
+
+	t.Run("ListUsers returns the error", func(t *testing.T) {
+		userRepo := newMockUserRepo()
+		userRepo.fail.failNextWith("GetAll", boom)
+
+		if _, err := useCases.NewListUsersUseCase(userRepo).ListUsers(); !errors.Is(err, boom) {
+			t.Errorf("err = %v, want %v", err, boom)
+		}
+	})
+
+	t.Run("retention stops on a user listing failure", func(t *testing.T) {
+		userRepo := newMockUserRepo()
+		userRepo.fail.failNextWith("GetAll", boom)
+		uc := useCases.NewEnforceRetentionUseCase(userRepo, newInviteFakeRepo(), &memorySessionRepo{})
+
+		if _, err := uc.Tick(time.Now()); !errors.Is(err, boom) {
+			t.Errorf("err = %v, want %v", err, boom)
+		}
+	})
+
+	t.Run("retention stops on an invite listing failure", func(t *testing.T) {
+		invites := newInviteFakeRepo()
+		invites.fail.failNextWith("GetAll", boom)
+		uc := useCases.NewEnforceRetentionUseCase(newMockUserRepo(), invites, &memorySessionRepo{})
+
+		if _, err := uc.Tick(time.Now()); !errors.Is(err, boom) {
+			t.Errorf("err = %v, want %v", err, boom)
+		}
+	})
+}

test/invitations_test.go

@@ -52,7 +52,10 @@ func (m *inviteFakeRepo) Create(i invite.Invite) (invite.Invite, error) {
 	return m.seed(i), nil
 }
 
-func (m *inviteFakeRepo) GetAll() []invite.Invite {
+func (m *inviteFakeRepo) GetAll() ([]invite.Invite, error) {
+	if err := m.fail.trigger("GetAll"); err != nil {
+		return nil, err
+	}
 	ids := make([]uint32, 0, len(m.invites))
 	for id := range m.invites {
 		ids = append(ids, id)
@@ -62,7 +65,7 @@ func (m *inviteFakeRepo) GetAll() []invite.Invite {
 	for _, id := range ids {
 		result = append(result, m.invites[id])
 	}
-	return result
+	return result, nil
 }
 
 func (m *inviteFakeRepo) Delete(id uint32) error {

test/mysql_server_test.go

@@ -59,8 +59,9 @@ func TestMySQLMigration(t *testing.T) {
 			}
 		}
 		binaryTables := columnCount(t, db, "SELECT COUNT(*) FROM information_schema.tables WHERE table_schema = DATABASE() AND table_collation = 'utf8mb4_bin'")
-		if binaryTables != 6 {
-			t.Errorf("every table must use the utf8mb4_bin collation, got %d of 6", binaryTables)
+		allTables := columnCount(t, db, "SELECT COUNT(*) FROM information_schema.tables WHERE table_schema = DATABASE() AND table_type = 'BASE TABLE'")
+		if binaryTables != allTables {
+			t.Errorf("every table must use the utf8mb4_bin collation, got %d of %d", binaryTables, allTables)
 		}
 	})
 
@@ -247,7 +248,7 @@ func TestMySQLBackup(t *testing.T) {
 		if line := firstDifferingLine(withoutRedundantCharset(dump), withoutRedundantCharset(string(restoredContent))); line != "" {
 			t.Errorf("replaying the dump must restore the same schema and rows, first difference: %s", line)
 		}
-		restoredUsers := newContractRepositories(restored.DB).users.GetAll()
+		restoredUsers := allOf(newContractRepositories(restored.DB).users.GetAll())
 		if len(restoredUsers) != 2 || restoredUsers[0].Username != "o'brien \"quoted\" back\\slash\nline é😀" || restoredUsers[1].PasswordResetCode != "reset-code" {
 			t.Errorf("unexpected restored users %+v", restoredUsers)
 		}

test/own_scope_visibility_persistence_test.go

@@ -0,0 +1,111 @@
+package test
+
+import (
+	"RenewCMS/internal/domain/article"
+	"RenewCMS/internal/domain/contenttype"
+	"RenewCMS/internal/domain/entry"
+	"RenewCMS/internal/infrastructure/persistence/database"
+	persistence "RenewCMS/internal/infrastructure/persistence/repositories"
+	"testing"
+)
+
+func TestArticleRepositoryVisibilityFilterOnSQLite(t *testing.T) {
+	db := openContractSQLite(t)
+	repo := persistence.NewArticleRepository(db)
+	defaultSite, err := persistence.NewSiteRepository(db).GetDefault()
+	if err != nil {
+		t.Fatal(err)
+	}
+	create := func(owner uint32, online bool) {
+		t.Helper()
+		record := articleOn(defaultSite.ID)
+		record.CreatedBy = owner
+		record.IsOnline = online
+		if _, err := repo.Create(record); err != nil {
+			t.Fatal(err)
+		}
+	}
+	create(1, true)
+	create(1, false)
+	create(2, true)
+	create(2, false)
+	create(2, false)
+	one := uint32(1)
+
+	t.Run("FindPage keeps online articles and the viewer's drafts", func(t *testing.T) {
+		found, err := repo.FindPage(article.Filters{VisibleTo: &one, Unpaged: true})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if found.Total != 3 || len(found.Items) != 3 {
+			t.Fatalf("total = %d, items = %d, want 3", found.Total, len(found.Items))
+		}
+		for _, item := range found.Items {
+			if !item.IsOnline && item.CreatedBy != one {
+				t.Errorf("leaked draft %+v", item)
+			}
+		}
+	})
+
+	t.Run("FindPage pages the filtered set", func(t *testing.T) {
+		found, err := repo.FindPage(article.Filters{VisibleTo: &one, Page: 2, Size: 2})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if found.Total != 3 || len(found.Items) != 1 {
+			t.Fatalf("total = %d, items = %d, want 3 and 1", found.Total, len(found.Items))
+		}
+	})
+
+	t.Run("FindPage combines the filter with the draft status", func(t *testing.T) {
+		found, err := repo.FindPage(article.Filters{VisibleTo: &one, Status: article.StatusDraft, Unpaged: true})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if found.Total != 1 || found.Items[0].CreatedBy != one {
+			t.Fatalf("found = %+v", found)
+		}
+	})
+
+	t.Run("CountVisibleTo counts online articles and the viewer's drafts", func(t *testing.T) {
+		counts, err := repo.CountVisibleTo(one)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if counts != (article.Counts{All: 3, Online: 2, Draft: 1}) {
+			t.Fatalf("counts = %+v", counts)
+		}
+	})
+}
+
+func TestEntryRepositoryCreatedByFilterOnSQLite(t *testing.T) {
+	db := openUnmigratedSQLite(t)
+	if err := database.Migrate(db, migrationPublicURL); err != nil {
+		t.Fatal(err)
+	}
+	types := persistence.NewContentTypeRepository(db)
+	entries := persistence.NewEntryRepository(db)
+	created, err := types.Create(contenttype.ContentType{Key: "notes", Name: "Notes", Fields: []contenttype.Field{{Key: "flag", Label: "Flag", Type: contenttype.TypeBoolean}}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, owner := range []uint32{1, 2, 1, 2, 1} {
+		if _, err := entries.Create(entry.Entry{ContentTypeID: created.ID, Data: map[string]any{}, CreatedBy: owner, UpdatedBy: owner}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	one := uint32(1)
+
+	found, err := entries.ListPage(entry.Filters{ContentTypeID: created.ID, CreatedBy: &one, Page: 1, Size: 2, Sort: entry.Sort{Field: entry.SortByID, Direction: entry.SortAsc}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if found.Total != 3 || len(found.Items) != 2 {
+		t.Fatalf("total = %d, items = %d, want 3 and 2", found.Total, len(found.Items))
+	}
+	for _, item := range found.Items {
+		if item.CreatedBy != one {
+			t.Errorf("leaked entry %+v", item)
+		}
+	}
+}

test/personal_data_retention_test.go

@@ -108,7 +108,7 @@ func TestEnforceRetentionUseCase(t *testing.T) {
 		if _, removed := w.users.users[2]; removed {
 			t.Error("the abandoned account must be removed")
 		}
-		remaining := w.invites.GetAll()
+		remaining := allOf(w.invites.GetAll())
 		if len(remaining) != 1 || remaining[0].Email != "pending@example.com" {
 			t.Errorf("expected only the pending invite kept, got %+v", remaining)
 		}

test/postgres_server_test.go

@@ -185,7 +185,7 @@ func TestPostgresMigration(t *testing.T) {
 	})
 
 	t.Run("a legacy users.role column is backfilled into role_id and the invites get Admin", func(t *testing.T) {
-		_, _, db := migratedPostgres(t)
+		server, config, db := migratedPostgres(t)
 		addLegacyRoleColumn(t, db)
 		for _, name := range []string{"first", "second", "third"} {
 			insert := db.Exec("INSERT INTO users (username, password, email, role, is_verified, token_version) VALUES (?, 'x', ?, '', true, 0)", name, name+"@x.example")
@@ -193,6 +193,7 @@ func TestPostgresMigration(t *testing.T) {
 				t.Fatalf("insert %s: %v", name, insert.Error)
 			}
 		}
+		db = server.connect(t, config).DB
 
 		if err := database.Migrate(db, migrationPublicURL); err != nil {
 			t.Fatalf("migrate: %v", err)
@@ -241,7 +242,7 @@ func TestPostgresTimesInLists(t *testing.T) {
 		mustCreateUser(t, repos, user)
 	}
 
-	users := repos.users.GetAll()
+	users := allOf(repos.users.GetAll())
 
 	if len(users) != 2 {
 		t.Fatalf("expected 2 users, got %d", len(users))
@@ -402,7 +403,7 @@ func TestPostgresBackup(t *testing.T) {
 			t.Errorf("replaying the dump must restore the same schema and rows, first difference: %s", line)
 		}
 		restoredRepos := newContractRepositories(restored.DB)
-		restoredUsers := restoredRepos.users.GetAll()
+		restoredUsers := allOf(restoredRepos.users.GetAll())
 		if len(restoredUsers) != 2 || restoredUsers[0].Username != "o'brien \"quoted\" back\\slash\nline é😀" || restoredUsers[1].PasswordResetCode != "reset-code" {
 			t.Errorf("unexpected restored users %+v", restoredUsers)
 		}

test/registration_status_endpoint_http_test.go

@@ -164,7 +164,7 @@ func TestRegistrationStatusEndpointHttp(t *testing.T) {
 		mailRepo := newMockMailRepo()
 		uc := useCases.NewCreateUserUseCase(userRepo, newSeededMockRoleRepo(), &stubHasher{}, mailRepo, "https://cms.example.com", discardRecorder)
 
-		if uc.IsRegistrationOpen() {
+		if open, openErr := uc.IsRegistrationOpen(); openErr != nil || open {
 			t.Fatalf("expected IsRegistrationOpen to return false")
 		}
 

test/repository_contract_test.go

@@ -207,7 +207,7 @@ func runRepositoryContract(t *testing.T, backend contractBackend) {
 			if err == nil {
 				t.Error("expected an error for the duplicate email")
 			}
-			if all := repos.users.GetAll(); len(all) != 1 || all[0].Username != "first" {
+			if all := allOf(repos.users.GetAll()); len(all) != 1 || all[0].Username != "first" {
 				t.Errorf("nothing may be stored for the failed create, got %+v", all)
 			}
 		}},
@@ -231,7 +231,7 @@ func runRepositoryContract(t *testing.T, backend contractBackend) {
 			if !errors.Is(secondErr, domainInvite.ErrInviteNotFound) {
 				t.Errorf("expected ErrInviteNotFound, got %v", secondErr)
 			}
-			if len(repos.users.GetAll()) != 1 {
+			if len(allOf(repos.users.GetAll())) != 1 {
 				t.Errorf("only the first accept may create a user")
 			}
 		}},
@@ -244,7 +244,7 @@ func runRepositoryContract(t *testing.T, backend contractBackend) {
 			if err == nil {
 				t.Fatal("expected the taken username to be refused")
 			}
-			invites := repos.invites.GetAll()
+			invites := allOf(repos.invites.GetAll())
 			if len(invites) != 1 || invites[0].AcceptedAt != nil {
 				t.Errorf("the invite must stay pending, got %+v", invites)
 			}
@@ -366,3 +366,10 @@ func TestSQLiteRepositoryContract(t *testing.T) {
 func TestMySQLRepositoryContract(t *testing.T) {
 	runRepositoryContract(t, contractBackend{name: "mysql", open: openContractMySQL})
 }
+
+func allOf[T any](all []T, err error) []T {
+	if err != nil {
+		panic(err)
+	}
+	return all
+}

test/site_cleanup_test.go

@@ -165,20 +165,20 @@ func TestArticleRepositoryWritesOnSQLite(t *testing.T) {
 			}
 		}
 
-		if all := repo.GetAll(); len(all) != 2 {
+		if all := allOf(repo.GetAll()); len(all) != 2 {
 			t.Errorf("articles = %+v", all)
 		}
 	})
 
-	t.Run("GetAll returns an empty list when the query fails", func(t *testing.T) {
+	t.Run("GetAll returns the error when the query fails", func(t *testing.T) {
 		repo, db, _ := setup(t)
 		fault := injectDatabaseFault(db, "fault:getall", faultOnQuery)
 		fault.armed.Store(true)
 
-		all := repo.GetAll()
+		all, err := repo.GetAll()
 
-		if all == nil || len(all) != 0 {
-			t.Errorf("articles = %#v", all)
+		if err == nil || len(all) != 0 {
+			t.Errorf("articles = %#v, err = %v", all, err)
 		}
 	})
 
@@ -436,7 +436,7 @@ func TestSiteDeletionIsCheckedInsideTheTransaction(t *testing.T) {
 		_, updateErr := articles.Update(created.ID, article.Update{SiteID: &shop.ID})
 
 		unchanged, _ := articles.Get(created.ID)
-		if !errors.Is(createErr, site.ErrSiteNotFound) || !errors.Is(updateErr, site.ErrSiteNotFound) || unchanged.SiteID != blog.ID || len(articles.GetAll()) != 1 {
+		if !errors.Is(createErr, site.ErrSiteNotFound) || !errors.Is(updateErr, site.ErrSiteNotFound) || unchanged.SiteID != blog.ID || len(allOf(articles.GetAll())) != 1 {
 			t.Errorf("create %v, update %v, site %d", createErr, updateErr, unchanged.SiteID)
 		}
 	})

test/unverified_users_and_reregistration_test.go

@@ -113,7 +113,7 @@ func TestUnverifiedUsersAndReregistrationRepository(t *testing.T) {
 			t.Fatal("expected Create to fail because the username is still in use")
 		}
 
-		users := repo.GetAll()
+		users := allOf(repo.GetAll())
 		count := 0
 		for _, u := range users {
 			if u.Username == "alice" {

test/use_case_baseline_test.go

@@ -549,7 +549,7 @@ func TestListUsersReturnsEveryUser(t *testing.T) {
 	repo.users[2] = user.User{ID: 2}
 	uc := useCases.NewListUsersUseCase(repo)
 
-	result := uc.ListUsers()
+	result, _ := uc.ListUsers()
 
 	if len(result) != 2 {
 		t.Fatalf("expected 2 users, got %d", len(result))

test/user_test.go

@@ -115,13 +115,16 @@ func (m *mockUserRepo) GetByEmail(email string) (user.User, error) {
 	return user.User{}, user.ErrUserNotFound
 }
 
-func (m *mockUserRepo) GetAll() []user.User {
+func (m *mockUserRepo) GetAll() ([]user.User, error) {
 	m.getAllCalls++
+	if err := m.fail.trigger("GetAll"); err != nil {
+		return nil, err
+	}
 	result := make([]user.User, 0)
 	for _, id := range m.sortedIds() {
 		result = append(result, m.users[id])
 	}
-	return result
+	return result, nil
 }
 
 func (m *mockUserRepo) FindByIDs(ids []uint32) ([]user.User, error) {

test/users_api_hardening_test.go

@@ -260,7 +260,7 @@ type usersAPIGetAllHookRepo struct {
 	hook func()
 }
 
-func (r *usersAPIGetAllHookRepo) GetAll() []user.User {
+func (r *usersAPIGetAllHookRepo) GetAll() ([]user.User, error) {
 	if r.hook != nil {
 		hook := r.hook
 		r.hook = nil

test/users_api_persistence_test.go

@@ -167,7 +167,7 @@ func TestUsersAPIInviteRepository(t *testing.T) {
 		if err != nil || renewed.RoleID != writer {
 			t.Fatalf("unexpected %+v, %v", renewed, err)
 		}
-		if all := repo.GetAll(); len(all) != 1 || all[0].RoleID != writer {
+		if all := allOf(repo.GetAll()); len(all) != 1 || all[0].RoleID != writer {
 			t.Errorf("unexpected %+v", all)
 		}
 	})