From 3e02e14da41d8bb51e5493d901ed5866a79ba9e8 Mon Sep 17 00:00:00 2001 From: Florian Sylvain Date: Wed, 4 Nov 2026 03:04:35 +0100 Subject: [PATCH] refac: missing mutation tests, spa specs, checks shown to pass, touch targets, catalogue entry, production code --- Taskfile.yml | 7 + api/dependecyInjection.go | 2 +- cmd/server/main.go | 44 +-- .../domain/localization/localizationRules.go | 3 +- .../infrastructure/useCases/UpdateSite.go | 29 +- test/activity_persistence_test.go | 7 +- test/article_body_format_test.go | 4 + test/article_concurrent_save_http_test.go | 60 ++++ test/article_create_site_http_test.go | 32 +++ test/article_domain_rules_test.go | 102 +++++++ test/article_list_localization_http_test.go | 5 + test/article_repository_edges_test.go | 229 +++++++++++++++ test/article_seo_index_persistence_test.go | 49 +++- test/article_seo_index_support_test.go | 14 + test/article_seo_refresh_test.go | 72 +++++ test/article_test.go | 68 ++++- test/author_support_test.go | 28 ++ test/fakes_common.go | 21 +- test/image_test.go | 28 ++ test/invite_management_test.go | 2 +- test/localization_rules_test.go | 125 +++++++++ test/localization_support_test.go | 14 +- test/media_storage_orchestration_test.go | 3 + test/memory_database_support_test.go | 12 + test/not_found_errors_test.go | 7 +- test/role_support_test.go | 22 +- test/roles_content_scope_world_test.go | 4 + test/seo_data_support_test.go | 2 +- test/site_domain_test.go | 2 +- test/site_support_test.go | 54 +++- test/story_delete_http_test.go | 4 + test/story_lock_stability_test.go | 25 ++ test/user_test.go | 56 +++- .../__tests__/EditSlugDialog.spec.ts | 96 +++++++ .../__tests__/IntegrationLanguages.spec.ts | 88 ++++++ .../__tests__/SeoLanguageCards.spec.ts | 137 +++++++++ .../__tests__/useEditorLocalizedSeo.spec.ts | 262 ++++++++++++++++++ web/src/data/__tests__/apiCatalogue.spec.ts | 12 +- web/src/data/apiCatalogue.ts | 17 ++ web/src/i18n/messages.ts | 2 + web/src/lib/__tests__/apiSearch.spec.ts | 8 +- web/src/lib/__tests__/localizedSeo.spec.ts | 197 +++++++++++++ .../__tests__/ArticleLocalization.spec.ts | 15 +- .../__tests__/EditorLanguageMenu.spec.ts | 36 ++- .../views/__tests__/EditorRecovery.spec.ts | 29 ++ .../views/__tests__/IntegrationSearch.spec.ts | 6 +- .../views/__tests__/IntegrationView.spec.ts | 2 +- web/visual/touchTargets.ts | 213 ++++++++++++++ 48 files changed, 2173 insertions(+), 83 deletions(-) create mode 100644 test/article_concurrent_save_http_test.go create mode 100644 test/article_domain_rules_test.go create mode 100644 test/article_repository_edges_test.go create mode 100644 test/article_seo_refresh_test.go create mode 100644 test/localization_rules_test.go create mode 100644 test/memory_database_support_test.go create mode 100644 web/src/components/__tests__/EditSlugDialog.spec.ts create mode 100644 web/src/components/__tests__/IntegrationLanguages.spec.ts create mode 100644 web/src/components/__tests__/SeoLanguageCards.spec.ts create mode 100644 web/src/composables/__tests__/useEditorLocalizedSeo.spec.ts create mode 100644 web/src/lib/__tests__/localizedSeo.spec.ts create mode 100644 web/visual/touchTargets.ts diff --git a/Taskfile.yml b/Taskfile.yml index 33f5956..5369fa5 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -196,6 +196,13 @@ tasks: cmds: - bun visual/check.ts {{.CLI_ARGS}} + visual:touch: + desc: Measure the computed height and tap area of the mobile touch targets at 390 px (Playwright) + dir: web + deps: [web:install] + cmds: + - bun visual/touchTargets.ts + lint: desc: gofmt check, go vet, eslint (--fix) and vue-tsc cmds: diff --git a/api/dependecyInjection.go b/api/dependecyInjection.go index a7c13ed..fb78891 100644 --- a/api/dependecyInjection.go +++ b/api/dependecyInjection.go @@ -420,7 +420,7 @@ func getUseCases(repos *Repositories, imageStorage domainImage.Storage, settings ListPermissionsUseCase: useCases.NewListPermissionsUseCase(), ListSitesUseCase: useCases.NewListSitesUseCase(repos.SiteRepo), CreateSiteUseCase: useCases.NewCreateSiteUseCase(repos.SiteRepo, repos.ImageRepo, recorder), - UpdateSiteUseCase: useCases.NewUpdateSiteUseCase(repos.SiteRepo, repos.ImageRepo, recorder, repos.ArticleRepo), + UpdateSiteUseCase: useCases.NewUpdateSiteUseCase(repos.SiteRepo, repos.ImageRepo, recorder, repos.Transactor), GetSiteLanguagesUseCase: useCases.NewGetSiteLanguagesUseCase(repos.SiteRepo, repos.LocalizationRepo), UpdateSiteLanguagesUseCase: useCases.NewUpdateSiteLanguagesUseCase(repos.Transactor, recorder), GetLanguageSetupUseCase: useCases.NewGetLanguageSetupUseCase(repos.SiteRepo, repos.LocalizationRepo), diff --git a/cmd/server/main.go b/cmd/server/main.go index 9bb636c..76f83fe 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -41,28 +41,28 @@ var requiredEnvVars = []string{ } var envDefaults = map[string]string{ - "ENVIRONMENT": "production", - "PORT": "8080", - "DB_FILE": "./data/renewcms.db", - config.EnvDatabaseDriver: config.DefaultDatabaseDriver, - "DB_TLS": config.DefaultDatabaseTLS, - "DB_SSLMODE": config.DefaultDatabaseSSLMode, - "DB_MAX_OPEN_CONNS": config.DefaultMaxOpenConns, - "DB_MAX_IDLE_CONNS": config.DefaultMaxIdleConns, - "DB_CONN_MAX_LIFETIME_SECONDS": config.DefaultConnMaxLifetimeSeconds, - "UPLOAD_DIR": "./data/uploads", - config.EnvStorageDriver: config.DefaultStorageDriver, - "S3_REGION": config.DefaultS3Region, - "S3_PATH_STYLE": config.DefaultS3PathStyle, - "S3_TLS": config.DefaultS3TLS, - "AUTH_RATE_LIMIT_MAX_ATTEMPTS": "5", - "AUTH_RATE_LIMIT_WINDOW_SECONDS": "60", - "TRUSTED_PROXIES": "", - config.EnvSmtpAllowInsecure: config.DefaultSmtpAllowInsecure, - "UPDATE_FEED_URL": "https://git.floriansylvain.fr", - "RENEWCMS_CONTAINER": "false", - config.EnvImageVariantMaxSourceBytes: config.DefaultImageVariantMaxSourceBytes, - config.EnvImageVariantMaxSourcePixels: config.DefaultImageVariantMaxSourcePixels, + "ENVIRONMENT": "production", + "PORT": "8080", + "DB_FILE": "./data/renewcms.db", + config.EnvDatabaseDriver: config.DefaultDatabaseDriver, + "DB_TLS": config.DefaultDatabaseTLS, + "DB_SSLMODE": config.DefaultDatabaseSSLMode, + "DB_MAX_OPEN_CONNS": config.DefaultMaxOpenConns, + "DB_MAX_IDLE_CONNS": config.DefaultMaxIdleConns, + "DB_CONN_MAX_LIFETIME_SECONDS": config.DefaultConnMaxLifetimeSeconds, + "UPLOAD_DIR": "./data/uploads", + config.EnvStorageDriver: config.DefaultStorageDriver, + "S3_REGION": config.DefaultS3Region, + "S3_PATH_STYLE": config.DefaultS3PathStyle, + "S3_TLS": config.DefaultS3TLS, + "AUTH_RATE_LIMIT_MAX_ATTEMPTS": "5", + "AUTH_RATE_LIMIT_WINDOW_SECONDS": "60", + "TRUSTED_PROXIES": "", + config.EnvSmtpAllowInsecure: config.DefaultSmtpAllowInsecure, + "UPDATE_FEED_URL": "https://git.floriansylvain.fr", + "RENEWCMS_CONTAINER": "false", + config.EnvImageVariantMaxSourceBytes: config.DefaultImageVariantMaxSourceBytes, + config.EnvImageVariantMaxSourcePixels: config.DefaultImageVariantMaxSourcePixels, config.EnvImageVariantMaxConcurrentRenders: config.DefaultImageVariantMaxConcurrentRenders, } diff --git a/internal/domain/localization/localizationRules.go b/internal/domain/localization/localizationRules.go index 014d564..8fd061d 100644 --- a/internal/domain/localization/localizationRules.go +++ b/internal/domain/localization/localizationRules.go @@ -97,7 +97,8 @@ func Plan(current []Row, counts map[string]VersionCount, change Change) ([]Row, confirmed := map[string]bool{} for _, row := range current { confirmed[row.Tag] = row.Confirmed - if !slices.ContainsFunc(entries, func(entry Entry) bool { return entry.Tag == row.Tag }) && counts[row.Tag].Total+counts[row.Tag].Trashed > 0 { + removed := !slices.ContainsFunc(entries, func(entry Entry) bool { return entry.Tag == row.Tag }) + if removed && counts[row.Tag].Total+counts[row.Tag].Trashed > 0 { return nil, ErrLanguageInUse } } diff --git a/internal/infrastructure/useCases/UpdateSite.go b/internal/infrastructure/useCases/UpdateSite.go index 4fe0841..6613b02 100644 --- a/internal/infrastructure/useCases/UpdateSite.go +++ b/internal/infrastructure/useCases/UpdateSite.go @@ -1,10 +1,10 @@ package useCases import ( - "RenewCMS/internal/domain/article" "RenewCMS/internal/domain/audit" "RenewCMS/internal/domain/image" "RenewCMS/internal/domain/site" + "RenewCMS/internal/domain/transaction" "context" ) @@ -12,11 +12,11 @@ type UpdateSiteUseCase struct { siteRepository site.Repository imageRepository image.Repository recorder audit.Recorder - articleSEO article.SEOIndex + transactor transaction.Transactor } -func NewUpdateSiteUseCase(siteRepository site.Repository, imageRepository image.Repository, recorder audit.Recorder, articleSEO article.SEOIndex) *UpdateSiteUseCase { - return &UpdateSiteUseCase{siteRepository, imageRepository, recorder, articleSEO} +func NewUpdateSiteUseCase(siteRepository site.Repository, imageRepository image.Repository, recorder audit.Recorder, transactor transaction.Transactor) *UpdateSiteUseCase { + return &UpdateSiteUseCase{siteRepository, imageRepository, recorder, transactor} } func (u *UpdateSiteUseCase) Update(ctx context.Context, id uint32, patch site.Patch) (site.Site, error) { @@ -37,7 +37,7 @@ func (u *UpdateSiteUseCase) Update(ctx context.Context, id uint32, patch site.Pa if len(changes) == 0 { return before, nil } - saved, err := u.siteRepository.Update(after) + saved, err := u.save(ctx, before, after) if err != nil { recordFailure(ctx, u.recorder, event, err) return site.Site{}, err @@ -45,12 +45,25 @@ func (u *UpdateSiteUseCase) Update(ctx context.Context, id uint32, patch site.Pa event.Target = siteTarget(saved) event.Changes = changes u.recorder.Record(ctx, event) - if before.DefaultDescription != saved.DefaultDescription { - return saved, u.articleSEO.MarkSiteSEOStale(saved.ID) - } return saved, nil } +func (u *UpdateSiteUseCase) save(ctx context.Context, before site.Site, after site.Site) (site.Site, error) { + var saved site.Site + err := u.transactor.Run(ctx, func(tx transaction.Repositories) error { + var err error + saved, err = tx.Sites.Update(after) + if err != nil { + return err + } + if before.DefaultDescription == saved.DefaultDescription { + return nil + } + return tx.ArticleSEO.MarkSiteSEOStale(saved.ID) + }) + return saved, err +} + func (u *UpdateSiteUseCase) apply(before site.Site, patch site.Patch) (site.Site, error) { after := before if patch.IsDefault != nil { diff --git a/test/activity_persistence_test.go b/test/activity_persistence_test.go index b3b3c87..4b21eb6 100644 --- a/test/activity_persistence_test.go +++ b/test/activity_persistence_test.go @@ -5,11 +5,9 @@ import ( "RenewCMS/internal/infrastructure/persistence/database" persistence "RenewCMS/internal/infrastructure/persistence/repositories" "encoding/json" - "fmt" "reflect" "sort" "strings" - "sync/atomic" "testing" "time" @@ -18,12 +16,9 @@ import ( "gorm.io/gorm/logger" ) -var activityDatabaseCounter atomic.Int64 - func openActivityDatabase(t *testing.T) *gorm.DB { t.Helper() - name := fmt.Sprintf("file:activity%d?mode=memory&cache=shared", activityDatabaseCounter.Add(1)) - db, err := gorm.Open(sqlite.Open(name), &gorm.Config{Logger: logger.Discard}) + db, err := gorm.Open(sqlite.Open(uniqueMemoryDSN()), &gorm.Config{Logger: logger.Discard}) if err != nil { t.Fatalf("failed to open the in-memory database: %v", err) } diff --git a/test/article_body_format_test.go b/test/article_body_format_test.go index 57367f2..8c26f57 100644 --- a/test/article_body_format_test.go +++ b/test/article_body_format_test.go @@ -13,6 +13,7 @@ import ( "errors" "net/http" "net/http/httptest" + "sync" "testing" "RenewCMS/api/pkg/authtoken" @@ -20,6 +21,7 @@ import ( ) type fakeBodyRenderer struct { + mu sync.Mutex fail bool calls int } @@ -27,6 +29,8 @@ type fakeBodyRenderer struct { var _ domainArticle.BodyRenderer = &fakeBodyRenderer{} func (f *fakeBodyRenderer) Render(html string, format domainArticle.BodyFormat) (string, error) { + f.mu.Lock() + defer f.mu.Unlock() f.calls++ if f.fail { return "", errors.New("render failed") diff --git a/test/article_concurrent_save_http_test.go b/test/article_concurrent_save_http_test.go new file mode 100644 index 0000000..9b2e096 --- /dev/null +++ b/test/article_concurrent_save_http_test.go @@ -0,0 +1,60 @@ +package test + +import ( + "RenewCMS/api/pkg/apperr" + "net/http" + "strconv" + "sync" + "testing" +) + +const concurrentSaveRounds = 20 + +func TestArticleConcurrentSavesHTTP(t *testing.T) { + t.Run("Two saves of the same article with the same if_revision: one wins, the other conflicts", func(t *testing.T) { + for round := 0; round < concurrentSaveRounds; round++ { + w := newStoryWorld(t) + revision, _ := revisionsOf(w, 10) + bodies := []string{"

Mine

", "

Theirs

"} + responses := make([]storyResponse, len(bodies)) + start := make(chan struct{}) + var saves sync.WaitGroup + for i, body := range bodies { + saves.Add(1) + go func() { + defer saves.Done() + <-start + responses[i] = w.send(storyUpdater, http.MethodPatch, "/v1/article/10", `{"body":"`+body+`","if_revision":`+strconv.FormatUint(uint64(revision), 10)+`}`) + }() + } + close(start) + saves.Wait() + + winners, losers := 0, 0 + var winningBody string + for i, response := range responses { + switch response.Code { + case http.StatusOK: + winners++ + winningBody = bodies[i] + case http.StatusConflict: + losers++ + if response.text() != apperr.RevisionConflictErrorMessage+"\n" { + t.Fatalf("conflict body %q", response.text()) + } + if response.header(apperr.CurrentRevisionHeader) != strconv.FormatUint(uint64(revision+1), 10) { + t.Fatalf("current revision header %q", response.header(apperr.CurrentRevisionHeader)) + } + default: + t.Fatalf("unexpected %d %q", response.Code, response.text()) + } + } + if winners != 1 || losers != 1 { + t.Fatalf("round %d: %d saved, %d conflicted", round, winners, losers) + } + if stored, _ := revisionsOf(w, 10); stored != revision+1 || w.stored(10).Body != winningBody { + t.Fatalf("round %d: stored revision %d body %q, the winner sent %q", round, stored, w.stored(10).Body, winningBody) + } + } + }) +} diff --git a/test/article_create_site_http_test.go b/test/article_create_site_http_test.go index 51bd870..9a615f6 100644 --- a/test/article_create_site_http_test.go +++ b/test/article_create_site_http_test.go @@ -121,6 +121,38 @@ func TestArticleCreateSiteHTTP(t *testing.T) { } }) + t.Run("Without a site the article is created on the default site, which the activity names", func(t *testing.T) { + for _, body := range []string{`{"title":"Plain"}`, `{"title":"Plain","site":null}`} { + t.Run(body, func(t *testing.T) { + w := newCreateSiteWorld(t) + if _, err := w.sites.SetDefault(storyAtelier); err != nil { + t.Fatal(err) + } + + created := createdTranslation(t, w, w.send(storyWriter, http.MethodPost, createSitePath, body)) + + if created.SiteID != storyAtelier || created.Lang != "fr" { + t.Errorf("created on site %d in %q, the default Atelier in fr expected", created.SiteID, created.Lang) + } + events := w.eventsOf(audit.ActionArticleCreate) + if len(events) != 1 || events[0].Outcome != audit.OutcomeSuccess || events[0].Params[audit.ParamSite] != "atelier" { + t.Errorf("one article.create naming the default site expected: %+v", events) + } + }) + } + }) + + t.Run("An unknown site key records the plain reason That site does not exist.", func(t *testing.T) { + w := newCreateSiteWorld(t) + + w.send(storyWriter, http.MethodPost, createSitePath, `{"title":"Plain","site":"missing"}`).expect(t, http.StatusNotFound, apperr.SiteNotFoundErrorMessage) + + events := w.eventsOf(audit.ActionArticleCreate) + if len(events) != 1 || events[0].FailureReason == nil || *events[0].FailureReason != "That site does not exist." { + t.Fatalf("one failed article.create with the site reason expected: %+v", events) + } + }) + t.Run("The language rules are those of the chosen site", func(t *testing.T) { w := newCreateSiteWorld(t) diff --git a/test/article_domain_rules_test.go b/test/article_domain_rules_test.go new file mode 100644 index 0000000..e2c3319 --- /dev/null +++ b/test/article_domain_rules_test.go @@ -0,0 +1,102 @@ +package test + +import ( + "RenewCMS/internal/domain/article" + "RenewCMS/internal/domain/author" + "errors" + "strings" + "testing" +) + +func TestArticleTitleLengthBounds(t *testing.T) { + cases := []struct { + runes int + valid bool + }{ + {article.MinTitleRunes - 1, false}, + {article.MinTitleRunes, true}, + {article.MaxTitleRunes, true}, + {article.MaxTitleRunes + 1, false}, + } + for _, c := range cases { + err := article.ValidateTitle(strings.Repeat("é", c.runes)) + if (err == nil) != c.valid || (err != nil && !errors.Is(err, article.ErrInvalidTitle)) { + t.Errorf("%d runes: err = %v", c.runes, err) + } + } +} + +func TestArticleDetailedErrorsMatchTheirSentinel(t *testing.T) { + cases := []struct { + err error + sentinel error + }{ + {&article.RevisionConflictError{Current: 4}, article.ErrRevisionConflict}, + {&article.LanguageExistsError{ArticleID: 9}, article.ErrLanguageExists}, + {&article.LanguageNotEnabledError{Missing: []string{"de"}}, article.ErrLanguageNotEnabled}, + {&article.SlugConflictError{ArticleIDs: []uint32{3}}, article.ErrSlugConflict}, + } + for _, c := range cases { + if !errors.Is(c.err, c.sentinel) || c.err.Error() != c.sentinel.Error() { + t.Errorf("%T does not match %v", c.err, c.sentinel) + } + if errors.Is(c.err, article.ErrArticleNotFound) { + t.Errorf("%T matches an unrelated sentinel", c.err) + } + } +} + +func TestArticleRevisionsMoveByOne(t *testing.T) { + opened := article.Article{}.Opened() + cases := []struct { + impact article.Impact + edit, content uint32 + }{ + {article.Impact{}, 1, 1}, + {article.Impact{Edited: true}, 2, 1}, + {article.Impact{Edited: true, Content: true}, 2, 2}, + } + for _, c := range cases { + revised := opened.Revised(c.impact) + if revised.EditRevision != c.edit || revised.ContentRevision != c.content { + t.Errorf("%+v: edit %d content %d", c.impact, revised.EditRevision, revised.ContentRevision) + } + } +} + +func TestArticleCopiedIntoKeepsAnAuthorList(t *testing.T) { + source := article.Article{ID: 7, Body: "

Body

", ContentRevision: 3} + copied := source.CopiedInto(article.Article{Lang: "fr"}) + if copied.Authors == nil || len(copied.Authors) != 0 { + t.Fatalf("authors = %#v", copied.Authors) + } + if *copied.SourceArticleID != 7 || copied.SourceRevision != 3 || copied.Body != "

Body

" { + t.Fatalf("copied = %+v", copied) + } + credited := article.Article{Authors: []author.Byline{{}}}.CopiedInto(article.Article{}) + if len(credited.Authors) != 1 { + t.Fatalf("credited authors = %#v", credited.Authors) + } +} + +func TestArticleStoryFiltersPerLanguage(t *testing.T) { + cases := []struct { + filters article.StoryFilters + want bool + }{ + {article.StoryFilters{Lang: "fr"}, true}, + {article.StoryFilters{Lang: "fr", Missing: true}, false}, + {article.StoryFilters{}, false}, + } + for _, c := range cases { + if got := c.filters.PerLanguage(); got != c.want { + t.Errorf("%+v: PerLanguage = %v", c.filters, got) + } + } +} + +func TestArticleSEOSummaryStale(t *testing.T) { + if !(article.SEOSummary{}).Stale() || (article.SEOSummary{Status: article.SEOStatusReady}).Stale() { + t.Fatal("only a summary without status is stale") + } +} diff --git a/test/article_list_localization_http_test.go b/test/article_list_localization_http_test.go index 2d45752..580b47c 100644 --- a/test/article_list_localization_http_test.go +++ b/test/article_list_localization_http_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "slices" "sort" "strconv" "strings" @@ -496,6 +497,10 @@ func runArticleListLocalization(t *testing.T, open func(*testing.T) *gorm.DB) { if len(unknown.Items) != 0 || unknown.Total != 0 { t.Fatal("unknown site must be empty") } + _, everySite := env.get(localizationOwnerID, "?group=story") + if !slices.Contains(everySite.storyIDs(), 70) || everySite.Total <= 1 { + t.Fatalf("without site every site is listed: %v of %d", everySite.storyIDs(), everySite.Total) + } }) t.Run("SEO filter on one version", func(t *testing.T) { diff --git a/test/article_repository_edges_test.go b/test/article_repository_edges_test.go new file mode 100644 index 0000000..b058b32 --- /dev/null +++ b/test/article_repository_edges_test.go @@ -0,0 +1,229 @@ +package test + +import ( + domainArticle "RenewCMS/internal/domain/article" + "RenewCMS/internal/domain/transaction" + entity "RenewCMS/internal/infrastructure/persistence/models" + persistence "RenewCMS/internal/infrastructure/persistence/repositories" + "errors" + "testing" + + "gorm.io/gorm" +) + +const sqliteBusyCode = 5 + +type sqliteBusyError struct{} + +func (sqliteBusyError) Error() string { + return "database is busy" +} + +func (sqliteBusyError) Code() int { + return sqliteBusyCode +} + +func onArticleUpdate(t *testing.T, db *gorm.DB, name string, hook func(*gorm.DB)) { + t.Helper() + if err := db.Callback().Update().Before("gorm:update").Register(name, hook); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Callback().Update().Remove(name) }) +} + +func onArticleQuery(t *testing.T, db *gorm.DB, name string, hook func(*gorm.DB)) { + t.Helper() + if err := db.Callback().Query().After("gorm:query").Register(name, hook); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Callback().Query().Remove(name) }) +} + +type articleEdgesEnv struct { + db *gorm.DB + articles *persistence.ArticleRepository + siteID uint32 +} + +func newArticleEdgesEnv(t *testing.T) *articleEdgesEnv { + t.Helper() + db := openContractSQLite(t) + return &articleEdgesEnv{db: db, articles: persistence.NewArticleRepository(db), siteID: storageDefaultSite(t, db).ID} +} + +func (e *articleEdgesEnv) create(t *testing.T, lang string, slug string, online bool) domainArticle.Article { + t.Helper() + created, err := e.articles.Create(newStorageArticle(e.siteID, lang, slug, 0)) + if err != nil { + t.Fatal(err) + } + if online { + if created, err = e.articles.UpdateIsOnline(created.ID, true); err != nil { + t.Fatal(err) + } + } + return created +} + +func TestArticleRepositoryLanguageScope(t *testing.T) { + e := newArticleEdgesEnv(t) + e.create(t, "fr", "bonjour", true) + e.create(t, "en", "hello", true) + + t.Run("An empty language list matches nothing", func(t *testing.T) { + page, err := e.articles.FindPage(domainArticle.Filters{Langs: &[]string{}, Unpaged: true}) + if err != nil || page.Total != 0 || len(page.Items) != 0 { + t.Fatalf("page = %+v, err = %v", page, err) + } + }) + + t.Run("A language list keeps only its languages", func(t *testing.T) { + page, err := e.articles.FindPage(domainArticle.Filters{Langs: &[]string{"fr"}, Unpaged: true}) + if err != nil || page.Total != 1 || len(page.Items) != 1 || page.Items[0].Lang != "fr" { + t.Fatalf("page = %+v, err = %v", page, err) + } + }) +} + +func TestArticleRepositoryCountVersions(t *testing.T) { + e := newArticleEdgesEnv(t) + e.create(t, "fr", "en-ligne", true) + draft := e.create(t, "fr", "brouillon", false) + e.create(t, "en", "other", false) + online := true + stranger := draft.CreatedBy + 1 + + cases := []struct { + name string + filters domainArticle.Filters + want domainArticle.Counts + }{ + {"every version of the language", domainArticle.Filters{Lang: "fr"}, domainArticle.Counts{All: 2, Online: 1, Draft: 1}}, + {"online only skips the drafts", domainArticle.Filters{Lang: "fr", IsOnline: &online}, domainArticle.Counts{All: 1, Online: 1}}, + {"drafts are counted for their reader only", domainArticle.Filters{Lang: "fr", VisibleTo: &stranger}, domainArticle.Counts{All: 1, Online: 1}}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + counts, err := e.articles.CountVersions(c.filters) + if err != nil || counts != c.want { + t.Fatalf("counts = %+v, err = %v, want %+v", counts, err, c.want) + } + }) + } +} + +func TestArticleRepositorySlugTaken(t *testing.T) { + e := newArticleEdgesEnv(t) + owner := e.create(t, "fr", "bonjour", false) + + cases := []struct { + name string + slug string + exceptID uint32 + want bool + }{ + {"a slug in use is taken", "bonjour", 0, true}, + {"a free slug is not taken", "salut", 0, false}, + {"the slug of the article itself is not taken", "bonjour", owner.ID, false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + taken, err := e.articles.SlugTaken(e.siteID, "fr", c.slug, c.exceptID) + if err != nil || taken != c.want { + t.Fatalf("taken = %v, err = %v", taken, err) + } + }) + } +} + +func TestArticleRepositoryUpdateRetriesLockContention(t *testing.T) { + cases := []struct { + name string + failures int + wantErr bool + attempts int + }{ + {"a write that is busy twice succeeds on the third attempt", 2, false, 3}, + {"a write that stays busy gives up after five attempts", 1000, true, 5}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + e := newArticleEdgesEnv(t) + created := e.create(t, "fr", "bonjour", false) + attempts := 0 + onArticleUpdate(t, e.db, "test:busy_article_writes", func(tx *gorm.DB) { + if tx.Statement.Table != "articles" { + return + } + attempts++ + if attempts <= c.failures { + _ = tx.AddError(sqliteBusyError{}) + } + }) + + updated, err := e.articles.Update(created.ID, domainArticle.Update{Excerpt: localeOf("Renamed")}) + + if c.wantErr != (err != nil) || attempts != c.attempts { + t.Fatalf("err = %v after %d attempts", err, attempts) + } + if !c.wantErr && updated.Excerpt != "Renamed" { + t.Fatalf("updated = %+v", updated) + } + }) + } +} + +func TestArticleRepositoryUpdateGivesUpOnASiteThatKeepsMoving(t *testing.T) { + e := newArticleEdgesEnv(t) + created := e.create(t, "fr", "bonjour", false) + probes := 0 + onArticleQuery(t, e.db, "test:moving_article_site", func(tx *gorm.DB) { + if tx.Statement.Table != "articles" || len(tx.Statement.Selects) != 2 { + return + } + probes++ + _ = tx.Session(&gorm.Session{NewDB: true, SkipHooks: true}).Model(&entity.Article{}).Where("id = ?", created.ID).UpdateColumn("site_id", uint32(probes+1000)).Error + }) + + _, err := e.articles.Update(created.ID, domainArticle.Update{Excerpt: localeOf("Renamed")}) + + if !errors.Is(err, transaction.ErrLockContention) || probes != 5 { + t.Fatalf("err = %v after %d probes", err, probes) + } +} + +func TestArticleRepositoryAuthorOnlyChangeIsAnEdit(t *testing.T) { + e := newArticleEdgesEnv(t) + created := e.create(t, "fr", "bonjour", false) + writer := entity.Author{Name: "Ada", Slug: "ada", SocialLinks: "[]"} + if err := e.db.Create(&writer).Error; err != nil { + t.Fatal(err) + } + credited := []uint32{writer.ID} + + updated, err := e.articles.Update(created.ID, domainArticle.Update{AuthorIDs: &credited}) + if err != nil || updated.EditRevision != created.EditRevision+1 || updated.ContentRevision != created.ContentRevision { + t.Fatalf("after crediting: %+v, err = %v", updated, err) + } + + again, err := e.articles.Update(created.ID, domainArticle.Update{AuthorIDs: &credited}) + if err != nil || again.EditRevision != updated.EditRevision { + t.Fatalf("the same authors bumped the revision: %+v, err = %v", again, err) + } +} + +func TestArticleRepositoryUnrelatedUniqueViolationIsNotALanguageClash(t *testing.T) { + e := newArticleEdgesEnv(t) + created := e.create(t, "fr", "bonjour", false) + writer := entity.Author{Name: "Ada", Slug: "ada", SocialLinks: "[]"} + if err := e.db.Create(&writer).Error; err != nil { + t.Fatal(err) + } + twice := []uint32{writer.ID, writer.ID} + + _, err := e.articles.Update(created.ID, domainArticle.Update{AuthorIDs: &twice}) + + if err == nil || errors.Is(err, domainArticle.ErrLanguageTaken) || errors.Is(err, domainArticle.ErrSlugTaken) { + t.Fatalf("err = %v, the duplicate credit must surface as itself", err) + } +} diff --git a/test/article_seo_index_persistence_test.go b/test/article_seo_index_persistence_test.go index c7cb700..c19f387 100644 --- a/test/article_seo_index_persistence_test.go +++ b/test/article_seo_index_persistence_test.go @@ -5,10 +5,12 @@ import ( "RenewCMS/internal/domain/author" "RenewCMS/internal/domain/localization" "RenewCMS/internal/domain/site" + "RenewCMS/internal/domain/transaction" entity "RenewCMS/internal/infrastructure/persistence/models" persistence "RenewCMS/internal/infrastructure/persistence/repositories" "RenewCMS/internal/infrastructure/useCases" "context" + "errors" "strings" "testing" @@ -25,6 +27,27 @@ type seoIndexEnv struct { siteID uint32 } +var errSEOClearFailed = errors.New("the SEO clear failed") + +type failingSEOClear struct { + domainArticle.SEOIndex +} + +func (failingSEOClear) MarkSiteSEOStale(uint32) error { + return errSEOClearFailed +} + +type failingSEOClearTransactor struct { + inner transaction.Transactor +} + +func (f failingSEOClearTransactor) Run(ctx context.Context, work func(transaction.Repositories) error) error { + return f.inner.Run(ctx, func(repositories transaction.Repositories) error { + repositories.ArticleSEO = failingSEOClear{repositories.ArticleSEO} + return work(repositories) + }) +} + func newSEOIndexEnv(t *testing.T, open func(*testing.T) *gorm.DB) *seoIndexEnv { t.Helper() db := open(t) @@ -123,6 +146,30 @@ func runArticleSEOIndex(t *testing.T, open func(*testing.T) *gorm.DB) { } }) + t.Run("A site description change whose SEO clear fails saves nothing", func(t *testing.T) { + e := newSEOIndexEnv(t, open) + created := e.create("en", 0) + e.must(e.refresh.RefreshStale()) + before, err := e.sites.Get(e.siteID) + e.must(err) + description := "A description that is never saved." + update := useCases.NewUpdateSiteUseCase(e.sites, persistence.NewImageRepository(e.db), discardRecorder, failingSEOClearTransactor{persistence.NewTransactor(e.db)}) + + _, err = update.Update(context.Background(), e.siteID, site.Patch{DefaultDescription: &description}) + + if !errors.Is(err, errSEOClearFailed) { + t.Fatalf("err = %v", err) + } + after, err := e.sites.Get(e.siteID) + e.must(err) + if after.DefaultDescription != before.DefaultDescription { + t.Fatalf("the description %q was saved", after.DefaultDescription) + } + if e.status(created.ID) == "" { + t.Fatal("the stored status was cleared") + } + }) + t.Run("A status computed before a concurrent write is not stored", func(t *testing.T) { e := newSEOIndexEnv(t, open) created := e.create("en", 0) @@ -143,7 +190,7 @@ func runArticleSEOIndex(t *testing.T, open func(*testing.T) *gorm.DB) { english := e.create("en", 0) french := e.create("fr", english.StoryID) description := strings.Repeat("Renovation notes. ", 6) - update := useCases.NewUpdateSiteUseCase(e.sites, persistence.NewImageRepository(e.db), discardRecorder, e.articles) + update := useCases.NewUpdateSiteUseCase(e.sites, persistence.NewImageRepository(e.db), discardRecorder, persistence.NewTransactor(e.db)) _, err := update.Update(context.Background(), e.siteID, site.Patch{DefaultDescription: &description}) e.must(err) diff --git a/test/article_seo_index_support_test.go b/test/article_seo_index_support_test.go index 61c0ee9..2b9d608 100644 --- a/test/article_seo_index_support_test.go +++ b/test/article_seo_index_support_test.go @@ -26,6 +26,8 @@ func (m *mockArticleRepo) seoFresh(version article.Article) bool { } func (m *mockArticleRepo) StaleSEO(afterID uint32, limit int) ([]article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("StaleSEO"); err != nil { return nil, err } @@ -45,6 +47,8 @@ func (m *mockArticleRepo) StaleSEO(afterID uint32, limit int) ([]article.Article } func (m *mockArticleRepo) StoreSEO(records []article.SEORecord) error { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("StoreSEO"); err != nil { return err } @@ -64,6 +68,8 @@ func (m *mockArticleRepo) StoreSEO(records []article.SEORecord) error { } func (m *mockArticleRepo) MarkSiteSEOStale(siteID uint32) error { + m.mu.Lock() + defer m.mu.Unlock() for id, version := range m.articles { if version.SiteID == siteID { delete(m.assessed, id) @@ -73,6 +79,8 @@ func (m *mockArticleRepo) MarkSiteSEOStale(siteID uint32) error { } func (m *mockArticleRepo) CountSuggestions(filters article.Filters) (int, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("CountSuggestions"); err != nil { return 0, err } @@ -86,6 +94,8 @@ func (m *mockArticleRepo) CountSuggestions(filters article.Filters) (int, error) } func (m *mockArticleRepo) CountSuggestedStories(readable article.StoryFilters) (int, error) { + m.mu.Lock() + defer m.mu.Unlock() m.listCalls++ if err := m.fail.trigger("CountSuggestedStories"); err != nil { return 0, err @@ -116,6 +126,8 @@ func mockVisible(version article.Article, onlineOnly bool, visibleTo *uint32) bo } func (m *mockArticleRepo) ListVersions(ids []uint32) ([]article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() m.listCalls++ if err := m.fail.trigger("ListVersions"); err != nil { return nil, err @@ -130,6 +142,8 @@ func (m *mockArticleRepo) ListVersions(ids []uint32) ([]article.Article, error) } func (m *mockArticleRepo) DeleteStory(storyID uint32) error { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("DeleteStory"); err != nil { return err } diff --git a/test/article_seo_refresh_test.go b/test/article_seo_refresh_test.go new file mode 100644 index 0000000..faf4702 --- /dev/null +++ b/test/article_seo_refresh_test.go @@ -0,0 +1,72 @@ +package test + +import ( + "RenewCMS/internal/domain/article" + "RenewCMS/internal/infrastructure/useCases" + "errors" + "testing" +) + +type batchedSEOIndex struct { + batches [][]article.Article + stored [][]article.SEORecord + storeErr error + afterIDs []uint32 + staleRead int +} + +var _ article.SEOIndex = &batchedSEOIndex{} + +func (b *batchedSEOIndex) StaleSEO(afterID uint32, limit int) ([]article.Article, error) { + b.afterIDs = append(b.afterIDs, afterID) + if b.staleRead >= len(b.batches) { + return nil, nil + } + batch := b.batches[b.staleRead] + b.staleRead++ + return batch, nil +} + +func (b *batchedSEOIndex) StoreSEO(records []article.SEORecord) error { + b.stored = append(b.stored, records) + return b.storeErr +} + +func (b *batchedSEOIndex) MarkSiteSEOStale(uint32) error { + return nil +} + +func staleVersion(id uint32) article.Article { + return article.Article{ID: id, SiteID: 1, Lang: "en", Title: "Renovation costs", SEOVersion: 3} +} + +func TestRefreshArticleSEOBatches(t *testing.T) { + t.Run("Every stale batch is stored, each read after the last stored id", func(t *testing.T) { + index := &batchedSEOIndex{batches: [][]article.Article{{staleVersion(4), staleVersion(7)}, {staleVersion(9)}}} + + err := useCases.NewRefreshArticleSEOUseCase(index, plainSites(), plainLocalization()).RefreshStale() + + if err != nil { + t.Fatal(err) + } + if len(index.stored) != 2 || len(index.stored[0]) != 2 || index.stored[1][0].ArticleID != 9 || index.stored[1][0].Version != 3 { + t.Fatalf("stored %+v", index.stored) + } + if len(index.afterIDs) != 3 || index.afterIDs[0] != 0 || index.afterIDs[1] != 7 || index.afterIDs[2] != 9 { + t.Fatalf("read after %v", index.afterIDs) + } + }) + + t.Run("A failed store stops the refresh with its error", func(t *testing.T) { + index := &batchedSEOIndex{batches: [][]article.Article{{staleVersion(4)}, {staleVersion(9)}}, storeErr: errRepositoryFailure} + + err := useCases.NewRefreshArticleSEOUseCase(index, plainSites(), plainLocalization()).RefreshStale() + + if !errors.Is(err, errRepositoryFailure) { + t.Fatalf("err = %v", err) + } + if len(index.stored) != 1 || index.staleRead != 1 { + t.Fatalf("the refresh went on after the failure: %d stores, %d reads", len(index.stored), index.staleRead) + } + }) +} diff --git a/test/article_test.go b/test/article_test.go index 71031e3..726171b 100644 --- a/test/article_test.go +++ b/test/article_test.go @@ -10,6 +10,7 @@ import ( "slices" "sort" "strings" + "sync" "testing" "unicode" @@ -19,6 +20,7 @@ import ( ) type mockArticleRepo struct { + mu sync.Mutex articles map[uint32]article.Article failUpdateBodyFor map[uint32]error nextID uint32 @@ -64,6 +66,8 @@ func (m *mockArticleRepo) withAuthors(a article.Article) article.Article { } func (m *mockArticleRepo) ListForAccount(creatorID uint32, authorID *uint32) ([]article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("ListForAccount"); err != nil { return nil, err } @@ -91,6 +95,8 @@ func (m *mockArticleRepo) sortedIds() []uint32 { } func (m *mockArticleRepo) FindByFilters(filters article.Filters) ([]article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("FindByFilters"); err != nil { return nil, err } @@ -106,6 +112,8 @@ func (m *mockArticleRepo) FindByFilters(filters article.Filters) ([]article.Arti } func (m *mockArticleRepo) FindPage(filters article.Filters) (article.PageResult, error) { + m.mu.Lock() + defer m.mu.Unlock() m.listCalls++ if err := m.fail.trigger("FindPage"); err != nil { return article.PageResult{}, err @@ -159,6 +167,8 @@ func (m *mockArticleRepo) FindPage(filters article.Filters) (article.PageResult, } func (m *mockArticleRepo) CountByVisibility(onlineOnly bool) (article.Counts, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("CountByVisibility"); err != nil { return article.Counts{}, err } @@ -175,6 +185,8 @@ func (m *mockArticleRepo) CountByVisibility(onlineOnly bool) (article.Counts, er } func (m *mockArticleRepo) CountVersions(filters article.Filters) (article.Counts, error) { + m.mu.Lock() + defer m.mu.Unlock() m.listCalls++ if err := m.fail.trigger("CountVersions"); err != nil { return article.Counts{}, err @@ -195,26 +207,36 @@ func (m *mockArticleRepo) CountVersions(filters article.Filters) (article.Counts } func (m *mockArticleRepo) FindStoryPage(filters article.StoryFilters) (article.StoryPage, error) { + m.mu.Lock() + defer m.mu.Unlock() m.listCalls++ return article.StoryPage{Refs: []article.StoryRef{}}, m.fail.trigger("FindStoryPage") } func (m *mockArticleRepo) CountStories(filters article.StoryFilters) (article.StoryCounts, error) { + m.mu.Lock() + defer m.mu.Unlock() m.listCalls++ return article.StoryCounts{}, m.fail.trigger("CountStories") } func (m *mockArticleRepo) ListByStories(storyIDs []uint32, readable article.StoryFilters) ([]article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() m.listCalls++ return []article.Article{}, m.fail.trigger("ListByStories") } func (m *mockArticleRepo) FindTitleMatches(q string, readable article.StoryFilters) ([]uint32, error) { + m.mu.Lock() + defer m.mu.Unlock() m.listCalls++ return []uint32{}, m.fail.trigger("FindTitleMatches") } func (m *mockArticleRepo) CountVisibleTo(userID uint32) (article.Counts, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("CountVisibleTo"); err != nil { return article.Counts{}, err } @@ -270,11 +292,13 @@ func foldArticleSearch(value string) string { } func (m *mockArticleRepo) Get(id uint32) (article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Get"); err != nil { return article.Article{}, err } if m.onGet != nil { - if err := m.onGet(id); err != nil { + if err := outsideLockErr(&m.mu, func() error { return m.onGet(id) }); err != nil { return article.Article{}, err } } @@ -286,6 +310,8 @@ func (m *mockArticleRepo) Get(id uint32) (article.Article, error) { } func (m *mockArticleRepo) GetByName(name string) (article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("GetByName"); err != nil { return article.Article{}, err } @@ -299,6 +325,8 @@ func (m *mockArticleRepo) GetByName(name string) (article.Article, error) { } func (m *mockArticleRepo) GetAll() ([]article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() result := make([]article.Article, 0) for _, id := range m.sortedIds() { result = append(result, m.articles[id]) @@ -319,6 +347,8 @@ func (m *mockArticleRepo) slugUsedOn(siteID uint32, lang string, slug string, ex } func (m *mockArticleRepo) SlugTaken(siteID uint32, lang string, slug string, exceptID uint32) (bool, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("SlugTaken"); err != nil { return false, err } @@ -326,6 +356,8 @@ func (m *mockArticleRepo) SlugTaken(siteID uint32, lang string, slug string, exc } func (m *mockArticleRepo) FindByImage(imageID uint32) ([]article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("FindByImage"); err != nil { return nil, err } @@ -339,6 +371,8 @@ func (m *mockArticleRepo) FindByImage(imageID uint32) ([]article.Article, error) } func (m *mockArticleRepo) ClearImage(imageID uint32) (article.ImageReferences, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("ClearImage"); err != nil { return article.ImageReferences{}, err } @@ -359,6 +393,8 @@ func (m *mockArticleRepo) ClearImage(imageID uint32) (article.ImageReferences, e } func (m *mockArticleRepo) RestoreImage(references article.ImageReferences, imageID uint32) error { + m.mu.Lock() + defer m.mu.Unlock() for _, id := range references.Cover { a := m.articles[id] a.CoverImageID = idPointer(imageID) @@ -373,6 +409,8 @@ func (m *mockArticleRepo) RestoreImage(references article.ImageReferences, image } func (m *mockArticleRepo) RepointImage(fromImageIDs []uint32, toImageID uint32) ([]uint32, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("RepointImage"); err != nil { return nil, err } @@ -397,10 +435,12 @@ func (m *mockArticleRepo) RepointImage(fromImageIDs []uint32, toImageID uint32) } func (m *mockArticleRepo) Create(a article.Article) (article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() m.createCalled = true if hook := m.beforeCreate; hook != nil { m.beforeCreate = nil - hook() + outsideLock(&m.mu, hook) } if err := m.fail.trigger("Create"); err != nil { return article.Article{}, err @@ -439,6 +479,8 @@ func (m *mockArticleRepo) Create(a article.Article) (article.Article, error) { } func (m *mockArticleRepo) UpdateBody(id uint32, body string) (article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() m.updateBodyIDs = append(m.updateBodyIDs, id) if err := m.fail.trigger("UpdateBody"); err != nil { return article.Article{}, err @@ -474,6 +516,8 @@ func (m *mockArticleRepo) storyHasLanguage(storyID uint32, lang string) bool { } func (m *mockArticleRepo) GetByLangSlug(siteID uint32, lang string, slug string) (article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("GetByLangSlug"); err != nil { return article.Article{}, err } @@ -486,6 +530,8 @@ func (m *mockArticleRepo) GetByLangSlug(siteID uint32, lang string, slug string) } func (m *mockArticleRepo) ListByStory(storyID uint32) ([]article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("ListByStory"); err != nil { return nil, err } @@ -502,12 +548,14 @@ func (m *mockArticleRepo) ListByStory(storyID uint32) ([]article.Article, error) } func (m *mockArticleRepo) MoveToSite(ids []uint32, siteID uint32) error { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("MoveToSite"); err != nil { return err } if hook := m.beforeMove; hook != nil { m.beforeMove = nil - hook() + outsideLock(&m.mu, hook) } snapshot := maps.Clone(m.articles) for position, id := range ids { @@ -532,6 +580,8 @@ func (m *mockArticleRepo) MoveToSite(ids []uint32, siteID uint32) error { } func (m *mockArticleRepo) Update(id uint32, update article.Update) (article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Update"); err != nil { return article.Article{}, err } @@ -541,7 +591,7 @@ func (m *mockArticleRepo) Update(id uint32, update article.Update) (article.Arti } if hook := m.beforeUpdate; hook != nil { m.beforeUpdate = nil - hook() + outsideLock(&m.mu, hook) a = m.articles[id] } if update.IfRevision != nil && a.EditRevision != *update.IfRevision { @@ -609,6 +659,8 @@ func applySeoUpdate(a *article.Article, update article.Update) { } func (m *mockArticleRepo) UpdateIsOnline(id uint32, isOnline bool) (article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("UpdateIsOnline"); err != nil { return article.Article{}, err } @@ -622,6 +674,8 @@ func (m *mockArticleRepo) UpdateIsOnline(id uint32, isOnline bool) (article.Arti } func (m *mockArticleRepo) Delete(id uint32) error { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Delete"); err != nil { return err } @@ -634,6 +688,8 @@ func (m *mockArticleRepo) Delete(id uint32) error { } func (m *mockArticleRepo) FindByBodyContaining(filename string) ([]article.Article, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("FindByBodyContaining"); err != nil { return nil, err } @@ -645,12 +701,14 @@ func (m *mockArticleRepo) FindByBodyContaining(filename string) ([]article.Artic } } if m.afterFindByBody != nil { - defer m.afterFindByBody() + defer outsideLock(&m.mu, m.afterFindByBody) } return result, nil } func (m *mockArticleRepo) AssignUnassignedLanguage(siteID uint32, lang string) (int, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("AssignUnassignedLanguage"); err != nil { return 0, err } diff --git a/test/author_support_test.go b/test/author_support_test.go index 2559396..3fa4070 100644 --- a/test/author_support_test.go +++ b/test/author_support_test.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "sort" "strings" + "sync" "testing" "time" @@ -24,6 +25,7 @@ type authorDeleteCall struct { } type mockAuthorRepo struct { + mu sync.Mutex authors map[uint32]author.Author articles *mockArticleRepo nextID uint32 @@ -99,6 +101,8 @@ func (m *mockAuthorRepo) sortedIDsDescending() []uint32 { } func (m *mockAuthorRepo) List(query author.ListQuery) (author.Page, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("List"); err != nil { return author.Page{}, err } @@ -120,6 +124,8 @@ func (m *mockAuthorRepo) List(query author.ListQuery) (author.Page, error) { } func (m *mockAuthorRepo) Get(id uint32) (author.Author, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Get"); err != nil { return author.Author{}, err } @@ -131,6 +137,8 @@ func (m *mockAuthorRepo) Get(id uint32) (author.Author, error) { } func (m *mockAuthorRepo) GetBySlug(slug string) (author.Author, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("GetBySlug"); err != nil { return author.Author{}, err } @@ -143,6 +151,8 @@ func (m *mockAuthorRepo) GetBySlug(slug string) (author.Author, error) { } func (m *mockAuthorRepo) GetByUserID(userID uint32) (author.Author, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("GetByUserID"); err != nil { return author.Author{}, err } @@ -170,6 +180,8 @@ func (m *mockAuthorRepo) conflictFor(candidate author.Author) error { } func (m *mockAuthorRepo) Create(a author.Author) (author.Author, error) { + m.mu.Lock() + defer m.mu.Unlock() m.createCalls++ if err := m.fail.trigger("Create"); err != nil { return author.Author{}, err @@ -186,6 +198,8 @@ func (m *mockAuthorRepo) Create(a author.Author) (author.Author, error) { } func (m *mockAuthorRepo) Update(a author.Author) (author.Author, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Update"); err != nil { return author.Author{}, err } @@ -201,6 +215,8 @@ func (m *mockAuthorRepo) Update(a author.Author) (author.Author, error) { } func (m *mockAuthorRepo) DeleteMovingCredits(id uint32, reassignTo *uint32) error { + m.mu.Lock() + defer m.mu.Unlock() m.deleteCalls = append(m.deleteCalls, authorDeleteCall{id: id, reassignTo: reassignTo}) if err := m.fail.trigger("DeleteMovingCredits"); err != nil { return err @@ -221,6 +237,8 @@ func (m *mockAuthorRepo) DeleteMovingCredits(id uint32, reassignTo *uint32) erro } func (m *mockAuthorRepo) DeleteUncrediting(id uint32) error { + m.mu.Lock() + defer m.mu.Unlock() m.deleteCalls = append(m.deleteCalls, authorDeleteCall{id: id}) if err := m.fail.trigger("DeleteMovingCredits"); err != nil { return err @@ -273,6 +291,8 @@ func containsID(ids []uint32, wanted uint32) bool { } func (m *mockAuthorRepo) AvatarUsage() (map[uint32]int, error) { + m.mu.Lock() + defer m.mu.Unlock() usage := map[uint32]int{} for _, a := range m.authors { if a.AvatarID != nil { @@ -283,6 +303,8 @@ func (m *mockAuthorRepo) AvatarUsage() (map[uint32]int, error) { } func (m *mockAuthorRepo) FindByAvatar(imageID uint32) ([]author.Author, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("FindByAvatar"); err != nil { return nil, err } @@ -302,6 +324,8 @@ func (m *mockAuthorRepo) sortedIDsAscending() []uint32 { } func (m *mockAuthorRepo) ClearAvatar(imageID uint32) ([]uint32, error) { + m.mu.Lock() + defer m.mu.Unlock() cleared := []uint32{} for _, id := range m.sortedIDsDescending() { a := m.authors[id] @@ -315,6 +339,8 @@ func (m *mockAuthorRepo) ClearAvatar(imageID uint32) ([]uint32, error) { } func (m *mockAuthorRepo) AssignAvatar(authorIDs []uint32, imageID uint32) error { + m.mu.Lock() + defer m.mu.Unlock() for _, id := range authorIDs { a := m.authors[id] a.AvatarID = idPointer(imageID) @@ -324,6 +350,8 @@ func (m *mockAuthorRepo) AssignAvatar(authorIDs []uint32, imageID uint32) error } func (m *mockAuthorRepo) RepointAvatar(fromImageIDs []uint32, toImageID uint32) (int, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("RepointAvatar"); err != nil { return 0, err } diff --git a/test/fakes_common.go b/test/fakes_common.go index 5866e96..cf5d6ab 100644 --- a/test/fakes_common.go +++ b/test/fakes_common.go @@ -1,9 +1,16 @@ package test -import "errors" +import ( + "errors" + "sync" +) var errRepositoryFailure = errors.New("repository failure") +var errRunawayLocking = errors.New("the use case kept locking sites without settling") + +const runawayLockCalls = 1000 + type failSwitch struct { method string err error @@ -33,3 +40,15 @@ func (f *failSwitch) trigger(method string) error { } const migrationPublicURL = "https://www.example.com" + +func outsideLock(mu *sync.Mutex, hook func()) { + mu.Unlock() + defer mu.Lock() + hook() +} + +func outsideLockErr(mu *sync.Mutex, hook func() error) error { + mu.Unlock() + defer mu.Lock() + return hook() +} diff --git a/test/image_test.go b/test/image_test.go index 8fbc473..6e2fe03 100644 --- a/test/image_test.go +++ b/test/image_test.go @@ -10,6 +10,7 @@ import ( "sort" "strconv" "strings" + "sync" "testing" "time" ) @@ -73,6 +74,7 @@ func (m *mockImageStorage) Delete(name string) error { var _ imageDomain.Storage = &mockImageStorage{} type mockImageRepo struct { + mu sync.Mutex images map[uint32]imageDomain.Image nextID uint32 fail failSwitch @@ -101,6 +103,8 @@ func (m *mockImageRepo) sortedImages() []imageDomain.Image { } func (m *mockImageRepo) Create(input imageDomain.Image) (imageDomain.Image, error) { + m.mu.Lock() + defer m.mu.Unlock() m.createCalls++ if err := m.fail.trigger("Create"); err != nil { return imageDomain.Image{}, err @@ -117,6 +121,8 @@ func (m *mockImageRepo) Create(input imageDomain.Image) (imageDomain.Image, erro } func (m *mockImageRepo) ListByUploader(userID uint32) ([]imageDomain.Image, error) { + m.mu.Lock() + defer m.mu.Unlock() m.uploaderLists++ if m.onListByUploader != nil { m.onListByUploader() @@ -134,6 +140,8 @@ func (m *mockImageRepo) ListByUploader(userID uint32) ([]imageDomain.Image, erro } func (m *mockImageRepo) ListMissingMetadata() ([]imageDomain.Image, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("ListMissingMetadata"); err != nil { return nil, err } @@ -147,6 +155,8 @@ func (m *mockImageRepo) ListMissingMetadata() ([]imageDomain.Image, error) { } func (m *mockImageRepo) UpdateMetadata(id uint32, metadata imageDomain.Metadata) error { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("UpdateMetadata"); err != nil { return err } @@ -161,6 +171,8 @@ func (m *mockImageRepo) UpdateMetadata(id uint32, metadata imageDomain.Metadata) } func (m *mockImageRepo) Get(id uint32) (imageDomain.Image, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Get"); err != nil { return imageDomain.Image{}, err } @@ -172,6 +184,8 @@ func (m *mockImageRepo) Get(id uint32) (imageDomain.Image, error) { } func (m *mockImageRepo) Delete(id uint32) error { + m.mu.Lock() + defer m.mu.Unlock() m.deleteCalls = append(m.deleteCalls, id) if m.onDelete != nil { m.onDelete(id) @@ -190,6 +204,8 @@ func (m *mockImageRepo) Delete(id uint32) error { } func (m *mockImageRepo) List(query imageDomain.SearchQuery) ([]imageDomain.Image, error) { + m.mu.Lock() + defer m.mu.Unlock() m.listCalls++ if err := m.fail.trigger("List"); err != nil { return nil, err @@ -208,6 +224,8 @@ func (m *mockImageRepo) List(query imageDomain.SearchQuery) ([]imageDomain.Image } func (m *mockImageRepo) FindByHash(hash string) ([]imageDomain.Image, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("FindByHash"); err != nil { return nil, err } @@ -221,6 +239,8 @@ func (m *mockImageRepo) FindByHash(hash string) ([]imageDomain.Image, error) { } func (m *mockImageRepo) Rename(id uint32, filename string) (imageDomain.Image, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Rename"); err != nil { return imageDomain.Image{}, err } @@ -244,6 +264,8 @@ func (m *mockImageRepo) hashCounts() map[string]int { } func (m *mockImageRepo) ListDuplicates() ([]imageDomain.Image, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("ListDuplicates"); err != nil { return nil, err } @@ -258,6 +280,8 @@ func (m *mockImageRepo) ListDuplicates() ([]imageDomain.Image, error) { } func (m *mockImageRepo) Counts() (imageDomain.Counts, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Counts"); err != nil { return imageDomain.Counts{}, err } @@ -271,6 +295,8 @@ func (m *mockImageRepo) Counts() (imageDomain.Counts, error) { } func (m *mockImageRepo) ExistsByStoredName(storedName string) (bool, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("ExistsByStoredName"); err != nil { return false, err } @@ -283,6 +309,8 @@ func (m *mockImageRepo) ExistsByStoredName(storedName string) (bool, error) { } func (m *mockImageRepo) ListByStoredNames(storedNames []string) ([]imageDomain.Image, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("ListByStoredNames"); err != nil { return nil, err } diff --git a/test/invite_management_test.go b/test/invite_management_test.go index 62adeeb..f05c85f 100644 --- a/test/invite_management_test.go +++ b/test/invite_management_test.go @@ -556,7 +556,7 @@ func TestManageInviteUseCases(t *testing.T) { } func TestInviteRepositoryRenew(t *testing.T) { - db, err := gorm.Open(sqlite.Open("file:inviterenew?mode=memory&cache=private"), &gorm.Config{Logger: logger.Discard}) + db, err := gorm.Open(sqlite.Open(uniqueMemoryDSN()), &gorm.Config{Logger: logger.Discard}) if err != nil { t.Fatalf("failed to open db: %v", err) } diff --git a/test/localization_rules_test.go b/test/localization_rules_test.go new file mode 100644 index 0000000..1deecb4 --- /dev/null +++ b/test/localization_rules_test.go @@ -0,0 +1,125 @@ +package test + +import ( + "RenewCMS/internal/domain/localization" + "errors" + "testing" +) + +func socialLocale(value string) *string { + return &value +} + +func TestLocalizationRowEquality(t *testing.T) { + base := localization.Row{Tag: "fr", IsDefault: true, Enabled: true, Confirmed: true, SocialLocale: socialLocale("fr_FR")} + differing := map[string]localization.Row{ + "tag": {Tag: "de", IsDefault: true, Enabled: true, Confirmed: true, SocialLocale: socialLocale("fr_FR")}, + "default": {Tag: "fr", IsDefault: false, Enabled: true, Confirmed: true, SocialLocale: socialLocale("fr_FR")}, + "enabled": {Tag: "fr", IsDefault: true, Enabled: false, Confirmed: true, SocialLocale: socialLocale("fr_FR")}, + "confirmed": {Tag: "fr", IsDefault: true, Enabled: true, Confirmed: false, SocialLocale: socialLocale("fr_FR")}, + "social locale": {Tag: "fr", IsDefault: true, Enabled: true, Confirmed: true, SocialLocale: socialLocale("fr_CA")}, + "missing social": {Tag: "fr", IsDefault: true, Enabled: true, Confirmed: true}, + "same row": base, + "same social copied": {Tag: "fr", IsDefault: true, Enabled: true, Confirmed: true, SocialLocale: socialLocale("fr_FR")}, + } + for name, other := range differing { + t.Run(name, func(t *testing.T) { + want := name == "same row" || name == "same social copied" + if base.Equal(other) != want || other.Equal(base) != want { + t.Fatalf("Equal = %v, want %v", base.Equal(other), want) + } + }) + } + t.Run("two rows without a social locale are equal", func(t *testing.T) { + plain := localization.Row{Tag: "en", Enabled: true} + if !plain.Equal(localization.Row{Tag: "en", Enabled: true}) { + t.Fatal("rows without social locale differ") + } + }) +} + +func TestLocalizationRevisionTracksEveryField(t *testing.T) { + rows := []localization.Row{{Tag: "en", IsDefault: true, Enabled: true}, {Tag: "fr", Enabled: true, SocialLocale: socialLocale("fr_FR")}} + changed := [][]localization.Row{ + {{Tag: "en", IsDefault: false, Enabled: true}, {Tag: "fr", IsDefault: true, Enabled: true, SocialLocale: socialLocale("fr_FR")}}, + {{Tag: "en", IsDefault: true, Enabled: true}, {Tag: "fr", Enabled: false, SocialLocale: socialLocale("fr_FR")}}, + {{Tag: "en", IsDefault: true, Enabled: true}, {Tag: "fr", Enabled: true}}, + } + for _, other := range changed { + if localization.SameRows(rows, other) { + t.Errorf("%+v has the revision of %+v", other, rows) + } + } + reordered := []localization.Row{rows[1], rows[0]} + if !localization.SameRows(rows, reordered) { + t.Error("the order of the rows changed the revision") + } +} + +func TestLocalizationSocialLocaleState(t *testing.T) { + cases := []struct { + name string + row localization.Row + siteLocale *string + want string + }{ + {"an explicit social locale is set", localization.Row{Tag: "fr", SocialLocale: socialLocale("fr_FR")}, nil, localization.SocialLocaleSet}, + {"a regional tag derives one", localization.Row{Tag: "fr-CA"}, nil, localization.SocialLocaleDerived}, + {"the site locale lends one", localization.Row{Tag: "fr"}, socialLocale("fr-BE"), localization.SocialLocaleDerived}, + {"nothing to derive from", localization.Row{Tag: "fr"}, socialLocale("de-DE"), localization.SocialLocaleNotSet}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := localization.SocialLocaleState(c.row, c.siteLocale); got != c.want { + t.Fatalf("state = %q, want %q", got, c.want) + } + }) + } +} + +func TestLocalizationPlanKeepsLanguagesInUse(t *testing.T) { + current := []localization.Row{{Tag: "en", IsDefault: true, Enabled: true, Confirmed: true}, {Tag: "fr", Enabled: true}} + keepEnglishOnly := localization.Change{Default: "en", Languages: []localization.Entry{{Tag: "en", Enabled: true}}} + cases := []struct { + name string + counts map[string]localization.VersionCount + want error + }{ + {"a removed language with live versions is refused", map[string]localization.VersionCount{"fr": {Total: 1}}, localization.ErrLanguageInUse}, + {"a removed language with only trashed versions is refused", map[string]localization.VersionCount{"fr": {Trashed: 1}}, localization.ErrLanguageInUse}, + {"a removed language without versions is dropped", map[string]localization.VersionCount{"en": {Total: 4}}, nil}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + rows, err := localization.Plan(current, c.counts, keepEnglishOnly) + if !errors.Is(err, c.want) { + t.Fatalf("err = %v, want %v", err, c.want) + } + if c.want == nil && (len(rows) != 1 || rows[0].Tag != "en" || !rows[0].IsDefault) { + t.Fatalf("rows = %+v", rows) + } + }) + } + t.Run("a kept language with versions stays", func(t *testing.T) { + keepBoth := localization.Change{Default: "en", Languages: []localization.Entry{{Tag: "en", Enabled: true}, {Tag: "fr", Enabled: false}}} + rows, err := localization.Plan(current, map[string]localization.VersionCount{"fr": {Total: 2}}, keepBoth) + if err != nil || len(rows) != 2 { + t.Fatalf("rows = %+v, err = %v", rows, err) + } + }) +} + +func TestLocalizationRowLookups(t *testing.T) { + rows := []localization.Row{{Tag: "en", IsDefault: true, Enabled: true}, {Tag: "fr", Enabled: false}} + t.Run("HasRow finds a disabled row and nothing else", func(t *testing.T) { + if !localization.HasRow(rows, "fr") || !localization.HasRow(rows, "en") || localization.HasRow(rows, "de") { + t.Fatal("HasRow answered wrongly") + } + }) + t.Run("MissingTags lists each tag that is not enabled once, sorted, never the unassigned tag", func(t *testing.T) { + missing := localization.MissingTags(rows, []string{"fr", "", "de", "en", "fr"}) + if len(missing) != 2 || missing[0] != "de" || missing[1] != "fr" { + t.Fatalf("missing = %v", missing) + } + }) +} diff --git a/test/localization_support_test.go b/test/localization_support_test.go index 57ecf86..df965ed 100644 --- a/test/localization_support_test.go +++ b/test/localization_support_test.go @@ -16,6 +16,7 @@ import ( "net/http/httptest" "slices" "strings" + "sync" "testing" "github.com/go-chi/chi/v5" @@ -32,6 +33,7 @@ const ( var errInjectedConfirm = errors.New("injected confirm failure") type fakeLocalizationStore struct { + mu sync.Mutex articles *mockArticleRepo rows map[uint32][]localization.Row trashed map[uint32]map[string]int @@ -49,10 +51,14 @@ func newFakeLocalizationStore(articles *mockArticleRepo) *fakeLocalizationStore } func (f *fakeLocalizationStore) Rows(siteID uint32) ([]localization.Row, error) { + f.mu.Lock() + defer f.mu.Unlock() return slices.Clone(f.rows[siteID]), nil } func (f *fakeLocalizationStore) ConfirmedSites() (map[uint32]bool, error) { + f.mu.Lock() + defer f.mu.Unlock() confirmed := map[uint32]bool{} for siteID, rows := range f.rows { if !localization.NeedsSetup(rows) { @@ -63,6 +69,8 @@ func (f *fakeLocalizationStore) ConfirmedSites() (map[uint32]bool, error) { } func (f *fakeLocalizationStore) VersionCounts(siteID uint32) (map[string]localization.VersionCount, error) { + f.mu.Lock() + defer f.mu.Unlock() counts := map[string]localization.VersionCount{} for _, stored := range f.articles.articles { if stored.SiteID != siteID { @@ -84,12 +92,16 @@ func (f *fakeLocalizationStore) VersionCounts(siteID uint32) (map[string]localiz } func (f *fakeLocalizationStore) Replace(siteID uint32, rows []localization.Row) error { + f.mu.Lock() + defer f.mu.Unlock() f.replaceCalls++ f.rows[siteID] = slices.Clone(rows) return nil } func (f *fakeLocalizationStore) ConfirmSetup(siteID uint32, tag string) error { + f.mu.Lock() + defer f.mu.Unlock() f.confirmCalls++ if f.failConfirmAt == f.confirmCalls { return errInjectedConfirm @@ -156,7 +168,7 @@ func newLocalizationEnv(t *testing.T) *localizationEnv { transactor := &rollbackLocalizationTransactor{store: store, sites: sites} c := e.container c.ListSitesUseCase = useCases.NewListSitesUseCase(sites) - c.UpdateSiteUseCase = useCases.NewUpdateSiteUseCase(sites, e.images, e.audit.recorder, e.articles) + c.UpdateSiteUseCase = useCases.NewUpdateSiteUseCase(sites, e.images, e.audit.recorder, siteWriteTransactor(sites, e.articles)) c.GetSiteLanguagesUseCase = useCases.NewGetSiteLanguagesUseCase(sites, store) c.UpdateSiteLanguagesUseCase = useCases.NewUpdateSiteLanguagesUseCase(transactor, e.audit.recorder) c.GetLanguageSetupUseCase = useCases.NewGetLanguageSetupUseCase(sites, store) diff --git a/test/media_storage_orchestration_test.go b/test/media_storage_orchestration_test.go index 56a3137..56ee6d3 100644 --- a/test/media_storage_orchestration_test.go +++ b/test/media_storage_orchestration_test.go @@ -197,6 +197,9 @@ func TestDriverRegistries(t *testing.T) { if err := connection.Close(); err != nil { t.Errorf("Close failed: %v", err) } + if sqlDB, err := connection.DB.DB(); err != nil || sqlDB.Ping() == nil { + t.Error("the database is still open after Close") + } }) t.Run("The storage registry returns a working local storage", func(t *testing.T) { diff --git a/test/memory_database_support_test.go b/test/memory_database_support_test.go new file mode 100644 index 0000000..e129e18 --- /dev/null +++ b/test/memory_database_support_test.go @@ -0,0 +1,12 @@ +package test + +import ( + "fmt" + "sync/atomic" +) + +var memoryDatabaseCounter atomic.Int64 + +func uniqueMemoryDSN() string { + return fmt.Sprintf("file:renewcms-test-%d?mode=memory&cache=shared", memoryDatabaseCounter.Add(1)) +} diff --git a/test/not_found_errors_test.go b/test/not_found_errors_test.go index 41300ee..b5b4a6d 100644 --- a/test/not_found_errors_test.go +++ b/test/not_found_errors_test.go @@ -26,10 +26,15 @@ import ( func openInMemoryDb(t *testing.T) *gorm.DB { t.Helper() - db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{}) + db, err := gorm.Open(sqlite.Open(uniqueMemoryDSN()), &gorm.Config{}) if err != nil { t.Fatalf("failed to open in-memory database: %v", err) } + t.Cleanup(func() { + if sqlDB, err := db.DB(); err == nil { + _ = sqlDB.Close() + } + }) return db } diff --git a/test/role_support_test.go b/test/role_support_test.go index 1006755..5faf618 100644 --- a/test/role_support_test.go +++ b/test/role_support_test.go @@ -3,6 +3,7 @@ package test import ( "RenewCMS/internal/domain/role" "slices" + "sync" ) const ( @@ -12,6 +13,7 @@ const ( ) type mockRoleRepo struct { + mu sync.Mutex roles map[uint32]role.Role nextID uint32 fail failSwitch @@ -48,6 +50,8 @@ func (m *mockRoleRepo) sortedIDs() []uint32 { } func (m *mockRoleRepo) List() ([]role.Role, error) { + m.mu.Lock() + defer m.mu.Unlock() m.calls["List"]++ if err := m.fail.trigger("List"); err != nil { return nil, err @@ -60,6 +64,8 @@ func (m *mockRoleRepo) List() ([]role.Role, error) { } func (m *mockRoleRepo) Get(id uint32) (role.Role, error) { + m.mu.Lock() + defer m.mu.Unlock() m.calls["Get"]++ if err := m.fail.trigger("Get"); err != nil { return role.Role{}, err @@ -72,6 +78,8 @@ func (m *mockRoleRepo) Get(id uint32) (role.Role, error) { } func (m *mockRoleRepo) GetByName(name string) (role.Role, error) { + m.mu.Lock() + defer m.mu.Unlock() m.calls["GetByName"]++ if err := m.fail.trigger("GetByName"); err != nil { return role.Role{}, err @@ -90,6 +98,8 @@ func (m *mockRoleRepo) GetByName(name string) (role.Role, error) { } func (m *mockRoleRepo) Create(created role.Role) (role.Role, error) { + m.mu.Lock() + defer m.mu.Unlock() m.calls["Create"]++ if err := m.fail.trigger("Create"); err != nil { return role.Role{}, err @@ -101,6 +111,8 @@ func (m *mockRoleRepo) Create(created role.Role) (role.Role, error) { } func (m *mockRoleRepo) Save(saved role.Role) (role.Role, error) { + m.mu.Lock() + defer m.mu.Unlock() m.calls["Save"]++ if err := m.fail.trigger("Save"); err != nil { return role.Role{}, err @@ -108,7 +120,7 @@ func (m *mockRoleRepo) Save(saved role.Role) (role.Role, error) { if m.beforeSave != nil { hook := m.beforeSave m.beforeSave = nil - hook() + outsideLock(&m.mu, hook) } stored, ok := m.roles[saved.ID] if !ok { @@ -147,6 +159,8 @@ func (m *mockRoleRepo) memberCount(roleID uint32) int { } func (m *mockRoleRepo) MemberCounts() (map[uint32]int, error) { + m.mu.Lock() + defer m.mu.Unlock() m.calls["MemberCounts"]++ if err := m.fail.trigger("MemberCounts"); err != nil { return nil, err @@ -170,6 +184,8 @@ func (m *mockRoleRepo) MemberCounts() (map[uint32]int, error) { } func (m *mockRoleRepo) Delete(id uint32) error { + m.mu.Lock() + defer m.mu.Unlock() m.calls["Delete"]++ if err := m.fail.trigger("Delete"); err != nil { return err @@ -185,6 +201,8 @@ func (m *mockRoleRepo) Delete(id uint32) error { } func (m *mockRoleRepo) ReassignMembers(from uint32, to uint32) (int, error) { + m.mu.Lock() + defer m.mu.Unlock() m.calls["ReassignMembers"]++ if err := m.fail.trigger("ReassignMembers"); err != nil { return 0, err @@ -212,6 +230,8 @@ func (m *mockRoleRepo) ReassignMembers(from uint32, to uint32) (int, error) { } func (m *mockRoleRepo) RemoveTypedGrants(typeKey string) (int, error) { + m.mu.Lock() + defer m.mu.Unlock() m.calls["RemoveTypedGrants"]++ if err := m.fail.trigger("RemoveTypedGrants"); err != nil { return 0, err diff --git a/test/roles_content_scope_world_test.go b/test/roles_content_scope_world_test.go index ddea891..f386a1d 100644 --- a/test/roles_content_scope_world_test.go +++ b/test/roles_content_scope_world_test.go @@ -16,6 +16,7 @@ import ( "sort" "strconv" "strings" + "sync" "testing" "time" ) @@ -42,12 +43,15 @@ const ( ) type contentScopeRecorder struct { + mu sync.Mutex events []audit.Event } var _ audit.Recorder = &contentScopeRecorder{} func (r *contentScopeRecorder) Record(_ context.Context, event audit.Event) { + r.mu.Lock() + defer r.mu.Unlock() r.events = append(r.events, event) } diff --git a/test/seo_data_support_test.go b/test/seo_data_support_test.go index ef8011e..54eebc0 100644 --- a/test/seo_data_support_test.go +++ b/test/seo_data_support_test.go @@ -68,7 +68,7 @@ func newSeoEnv(t *testing.T) *seoEnv { c.PreviewPurgeUseCase = useCases.NewPreviewPurgeUseCase(p.images, p.articles, sites, authors, newMockContentTypeRepo(), newMockEntryRepo()) c.ListSitesUseCase = useCases.NewListSitesUseCase(sites) c.CreateSiteUseCase = useCases.NewCreateSiteUseCase(sites, p.images, recorder) - c.UpdateSiteUseCase = useCases.NewUpdateSiteUseCase(sites, p.images, recorder, p.articles) + c.UpdateSiteUseCase = useCases.NewUpdateSiteUseCase(sites, p.images, recorder, siteWriteTransactor(sites, p.articles)) c.SetDefaultSiteUseCase = useCases.NewSetDefaultSiteUseCase(sites, recorder) c.DeleteSiteUseCase = useCases.NewDeleteSiteUseCase(sites, recorder) c.ListActivityUseCase = useCases.NewListActivityUseCase(p.audit.store, p.users, p.articles, p.images, p.keys, p.audit.clock, sites, authors) diff --git a/test/site_domain_test.go b/test/site_domain_test.go index 66af8ff..5a8a215 100644 --- a/test/site_domain_test.go +++ b/test/site_domain_test.go @@ -153,7 +153,7 @@ func TestSiteUseCaseRules(t *testing.T) { repo := newMockArticleRepo() sites := newMockSiteRepo(repo) sites.seed(2, "Shop", "shop", false) - uc := useCases.NewUpdateSiteUseCase(sites, newMockImageRepo(), discardRecorder, repo) + uc := useCases.NewUpdateSiteUseCase(sites, newMockImageRepo(), discardRecorder, siteWriteTransactor(sites, repo)) flag := true _, err := uc.Update(testCtx, 2, site.Patch{IsDefault: &flag}) diff --git a/test/site_support_test.go b/test/site_support_test.go index 1c6fed1..1842ccd 100644 --- a/test/site_support_test.go +++ b/test/site_support_test.go @@ -14,6 +14,7 @@ import ( "sort" "strconv" "strings" + "sync" "testing" "time" @@ -21,6 +22,7 @@ import ( ) type mockSiteRepo struct { + mu sync.Mutex sites map[uint32]site.Site articles *mockArticleRepo nextID uint32 @@ -36,7 +38,12 @@ type mockSiteRepo struct { } func (m *mockSiteRepo) LockSite(id uint32) error { + m.mu.Lock() + defer m.mu.Unlock() m.lockedSites = append(m.lockedSites, id) + if len(m.lockedSites) > runawayLockCalls { + return errRunawayLocking + } if m.busy { return transaction.ErrLockContention } @@ -44,13 +51,18 @@ func (m *mockSiteRepo) LockSite(id uint32) error { } func (m *mockSiteRepo) TryLockSites(ids []uint32) error { + m.mu.Lock() + defer m.mu.Unlock() m.tryLocked = append(m.tryLocked, slices.Clone(ids)) + if len(m.tryLocked) > runawayLockCalls { + return errRunawayLocking + } if m.busy { return transaction.ErrLockContention } if hook := m.afterTryLock; hook != nil { m.afterTryLock = nil - hook() + outsideLock(&m.mu, hook) } return nil } @@ -98,9 +110,15 @@ func (m *mockSiteRepo) defaults() []uint32 { } func (m *mockSiteRepo) List() ([]site.Site, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("List"); err != nil { return nil, err } + return m.sortedSites(), nil +} + +func (m *mockSiteRepo) sortedSites() []site.Site { result := make([]site.Site, 0, len(m.sites)) for _, s := range m.sites { result = append(result, m.withCount(s)) @@ -115,17 +133,19 @@ func (m *mockSiteRepo) List() ([]site.Site, error) { } return result[i].ID < result[j].ID }) - return result, nil + return result } func (m *mockSiteRepo) Find(query site.ListQuery) (site.Page, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Find"); err != nil { return site.Page{}, err } - all, err := m.List() - if err != nil { + if err := m.fail.trigger("List"); err != nil { return site.Page{}, err } + all := m.sortedSites() needle := foldArticleSearch(strings.TrimSpace(query.Q)) matching := []site.Site{} for _, s := range all { @@ -142,6 +162,8 @@ func (m *mockSiteRepo) Find(query site.ListQuery) (site.Page, error) { } func (m *mockSiteRepo) Get(id uint32) (site.Site, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Get"); err != nil { return site.Site{}, err } @@ -153,6 +175,8 @@ func (m *mockSiteRepo) Get(id uint32) (site.Site, error) { } func (m *mockSiteRepo) GetByKey(key string) (site.Site, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("GetByKey"); err != nil { return site.Site{}, err } @@ -165,6 +189,8 @@ func (m *mockSiteRepo) GetByKey(key string) (site.Site, error) { } func (m *mockSiteRepo) GetDefault() (site.Site, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("GetDefault"); err != nil { return site.Site{}, err } @@ -175,6 +201,8 @@ func (m *mockSiteRepo) GetDefault() (site.Site, error) { } func (m *mockSiteRepo) Create(s site.Site) (site.Site, error) { + m.mu.Lock() + defer m.mu.Unlock() m.createCalls++ if err := m.fail.trigger("Create"); err != nil { return site.Site{}, err @@ -192,6 +220,8 @@ func (m *mockSiteRepo) Create(s site.Site) (site.Site, error) { } func (m *mockSiteRepo) Update(s site.Site) (site.Site, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Update"); err != nil { return site.Site{}, err } @@ -207,6 +237,8 @@ func (m *mockSiteRepo) Update(s site.Site) (site.Site, error) { } func (m *mockSiteRepo) SetDefault(id uint32) (site.Site, error) { + m.mu.Lock() + defer m.mu.Unlock() m.setDefaultCalls++ if _, ok := m.sites[id]; !ok { return site.Site{}, site.ErrSiteNotFound @@ -227,6 +259,8 @@ func (m *mockSiteRepo) SetDefault(id uint32) (site.Site, error) { } func (m *mockSiteRepo) DeleteMovingArticles(id uint32, reassignTo *uint32) error { + m.mu.Lock() + defer m.mu.Unlock() m.deleteCalls++ m.deleteReassignments = append(m.deleteReassignments, reassignTo) if err := m.fail.trigger("DeleteMovingArticles"); err != nil { @@ -258,6 +292,8 @@ func (m *mockSiteRepo) DeleteMovingArticles(id uint32, reassignTo *uint32) error } func (m *mockSiteRepo) ClearImage(imageID uint32) ([]uint32, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("ClearImage"); err != nil { return nil, err } @@ -274,6 +310,8 @@ func (m *mockSiteRepo) ClearImage(imageID uint32) ([]uint32, error) { } func (m *mockSiteRepo) AssignImage(siteIDs []uint32, imageID uint32) error { + m.mu.Lock() + defer m.mu.Unlock() for _, id := range siteIDs { s := m.sites[id] assigned := imageID @@ -284,6 +322,8 @@ func (m *mockSiteRepo) AssignImage(siteIDs []uint32, imageID uint32) error { } func (m *mockSiteRepo) RepointImage(fromImageIDs []uint32, toImageID uint32) (int, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("RepointImage"); err != nil { return 0, err } @@ -338,7 +378,7 @@ func newSiteEnv(t *testing.T) *siteEnv { c.RunPurgeUseCase = useCases.NewRunPurgeUseCase(p.images, p.articles, mergeMedia, newMockPurgeSettingsRepo(), func() time.Time { return p.now }, log.New(io.Discard, "", 0), recorder, sites, plainAuthors()) c.ListSitesUseCase = useCases.NewListSitesUseCase(sites) c.CreateSiteUseCase = useCases.NewCreateSiteUseCase(sites, p.images, recorder) - c.UpdateSiteUseCase = useCases.NewUpdateSiteUseCase(sites, p.images, recorder, p.articles) + c.UpdateSiteUseCase = useCases.NewUpdateSiteUseCase(sites, p.images, recorder, siteWriteTransactor(sites, p.articles)) c.SetDefaultSiteUseCase = useCases.NewSetDefaultSiteUseCase(sites, recorder) c.DeleteSiteUseCase = useCases.NewDeleteSiteUseCase(sites, recorder) router := chi.NewRouter() @@ -427,3 +467,7 @@ func plainSites() *mockSiteRepo { sites.seed(1, "Default", "default", true) return sites } + +func siteWriteTransactor(sites site.Repository, seo article.SEOIndex) transaction.Transactor { + return repositoriesTransactor{transaction.Repositories{Sites: sites, ArticleSEO: seo}} +} diff --git a/test/story_delete_http_test.go b/test/story_delete_http_test.go index 4bad0c6..8fba7ba 100644 --- a/test/story_delete_http_test.go +++ b/test/story_delete_http_test.go @@ -74,6 +74,10 @@ func TestStoryDeleteHTTP(t *testing.T) { if !slices.Equal(w.storyVersionIDs(), before) { t.Fatalf("versions changed: %v", w.storyVersionIDs()) } + failed := w.eventsOf(audit.ActionArticleDelete) + if len(failed) != 1 || failed[0].Outcome != audit.OutcomeFailed || failed[0].Target.ID != "10" { + t.Fatalf("one failed article.delete on the oldest readable version expected: %+v", failed) + } }) t.Run("An unknown or unreadable article answers the same not found", func(t *testing.T) { diff --git a/test/story_lock_stability_test.go b/test/story_lock_stability_test.go index dbaa681..bc371fb 100644 --- a/test/story_lock_stability_test.go +++ b/test/story_lock_stability_test.go @@ -74,3 +74,28 @@ func TestMoveStoryRelocksUntilTheSiteSetIsStable(t *testing.T) { } }) } + +func TestDeleteStoryRelocksUntilTheSiteSetIsStable(t *testing.T) { + t.Run("A set that never settles is refused as lock contention after the last round", func(t *testing.T) { + w := newStoryWorld(t) + listing := &shiftingStoryRepo{mockArticleRepo: w.articles, shiftFrom: 1} + listing.shift = func() { + shifted := w.articles.articles[13] + shifted.SiteID = 100 + uint32(listing.listings) + w.articles.articles[13] = shifted + } + deleter := useCases.NewDeleteStoryUseCase(repositoriesTransactor{transaction.Repositories{Articles: listing, Sites: w.sites}}, listing, discardRecorder) + + err := deleter.DeleteStory(context.Background(), useCases.DeleteStoryCommand{StoryID: storyStory, Caller: ownerCaller(storyOwner)}) + + if !errors.Is(err, transaction.ErrLockContention) { + t.Errorf("expected lock contention, got %v", err) + } + if listing.listings != 6 { + t.Errorf("expected five lock rounds plus the refusal lookup, got %d listings", listing.listings) + } + if _, kept := w.articles.articles[13]; !kept { + t.Error("nothing may be deleted") + } + }) +} diff --git a/test/user_test.go b/test/user_test.go index ad1245e..0cf2f7e 100644 --- a/test/user_test.go +++ b/test/user_test.go @@ -35,6 +35,7 @@ func (s *stubHasher) Compare(hashed string, value string) error { } type mockUserRepo struct { + mu sync.Mutex users map[uint32]user.User nextID uint32 findByIDsCalls int @@ -60,7 +61,6 @@ type mockUserRepo struct { beforeTransfer func() beforeClearVerification func() knownRoles *mockRoleRepo - confirmLock sync.Mutex } func newMockUserRepo() *mockUserRepo { @@ -83,8 +83,8 @@ func (m *mockUserRepo) sortedIds() []uint32 { } func (m *mockUserRepo) Get(id uint32) (user.User, error) { - m.confirmLock.Lock() - defer m.confirmLock.Unlock() + m.mu.Lock() + defer m.mu.Unlock() m.getCalls++ if err := m.fail.trigger("Get"); err != nil { return user.User{}, err @@ -97,6 +97,8 @@ func (m *mockUserRepo) Get(id uint32) (user.User, error) { } func (m *mockUserRepo) GetByUsername(username string) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("GetByUsername"); err != nil { return user.User{}, err } @@ -110,6 +112,8 @@ func (m *mockUserRepo) GetByUsername(username string) (user.User, error) { } func (m *mockUserRepo) GetByEmail(email string) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("GetByEmail"); err != nil { return user.User{}, err } @@ -123,8 +127,8 @@ func (m *mockUserRepo) GetByEmail(email string) (user.User, error) { } func (m *mockUserRepo) IsEmailInUse(email string, exceptUserId uint32) (bool, error) { - m.confirmLock.Lock() - defer m.confirmLock.Unlock() + m.mu.Lock() + defer m.mu.Unlock() return m.emailInUse(email, exceptUserId) } @@ -141,6 +145,8 @@ func (m *mockUserRepo) emailInUse(email string, exceptUserId uint32) (bool, erro } func (m *mockUserRepo) SetPendingEmailChange(userId uint32, pendingEmail string, code string, expiration time.Time) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("SetPendingEmailChange"); err != nil { return user.User{}, err } @@ -156,6 +162,8 @@ func (m *mockUserRepo) SetPendingEmailChange(userId uint32, pendingEmail string, } func (m *mockUserRepo) ClearPendingEmailChange(userId uint32) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("ClearPendingEmailChange"); err != nil { return user.User{}, err } @@ -171,8 +179,8 @@ func (m *mockUserRepo) ClearPendingEmailChange(userId uint32) (user.User, error) } func (m *mockUserRepo) ConfirmEmailChange(userId uint32, pendingEmail string, code string, now time.Time) (user.User, error) { - m.confirmLock.Lock() - defer m.confirmLock.Unlock() + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("ConfirmEmailChange"); err != nil { return user.User{}, err } @@ -198,6 +206,8 @@ func (m *mockUserRepo) ConfirmEmailChange(userId uint32, pendingEmail string, co } func (m *mockUserRepo) GetAll() ([]user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() m.getAllCalls++ if err := m.fail.trigger("GetAll"); err != nil { return nil, err @@ -210,6 +220,8 @@ func (m *mockUserRepo) GetAll() ([]user.User, error) { } func (m *mockUserRepo) FindByIDs(ids []uint32) ([]user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("FindByIDs"); err != nil { return nil, err } @@ -224,6 +236,8 @@ func (m *mockUserRepo) FindByIDs(ids []uint32) ([]user.User, error) { } func (m *mockUserRepo) Create(u user.User) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Create"); err != nil { return user.User{}, err } @@ -234,6 +248,8 @@ func (m *mockUserRepo) Create(u user.User) (user.User, error) { } func (m *mockUserRepo) Delete(id uint32) error { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("Delete"); err != nil { return err } @@ -245,6 +261,8 @@ func (m *mockUserRepo) Delete(id uint32) error { } func (m *mockUserRepo) DeleteIfRole(id uint32, roleID uint32) error { + m.mu.Lock() + defer m.mu.Unlock() m.deleteIfRoleCalls++ if err := m.fail.trigger("DeleteIfRole"); err != nil { return err @@ -252,7 +270,7 @@ func (m *mockUserRepo) DeleteIfRole(id uint32, roleID uint32) error { if m.beforeDeleteIfRole != nil { hook := m.beforeDeleteIfRole m.beforeDeleteIfRole = nil - hook() + outsideLock(&m.mu, hook) } stored, ok := m.users[id] if !ok { @@ -266,6 +284,8 @@ func (m *mockUserRepo) DeleteIfRole(id uint32, roleID uint32) error { } func (m *mockUserRepo) UpdateVerificationStatus(userId uint32, isVerified bool) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("UpdateVerificationStatus"); err != nil { return user.User{}, err } @@ -279,6 +299,8 @@ func (m *mockUserRepo) UpdateVerificationStatus(userId uint32, isVerified bool) } func (m *mockUserRepo) UpdatePassword(userId uint32, password string) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() if err := m.fail.trigger("UpdatePassword"); err != nil { return user.User{}, err } @@ -292,6 +314,8 @@ func (m *mockUserRepo) UpdatePassword(userId uint32, password string) (user.User } func (m *mockUserRepo) UpdatePasswordResetCode(userId uint32, code string, expiration time.Time) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() m.updatePasswordResetCodeCalls++ m.lastUpdatePasswordResetCodeUserId = userId m.lastUpdatePasswordResetCodeCode = code @@ -310,6 +334,8 @@ func (m *mockUserRepo) UpdatePasswordResetCode(userId uint32, code string, expir } func (m *mockUserRepo) ClearVerificationCode(userId uint32) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() m.clearVerificationCodeCalls++ m.clearVerificationCodeUserIds = append(m.clearVerificationCodeUserIds, userId) if err := m.fail.trigger("ClearVerificationCode"); err != nil { @@ -318,7 +344,7 @@ func (m *mockUserRepo) ClearVerificationCode(userId uint32) (user.User, error) { if m.beforeClearVerification != nil { hook := m.beforeClearVerification m.beforeClearVerification = nil - hook() + outsideLock(&m.mu, hook) } u, ok := m.users[userId] if !ok { @@ -338,6 +364,8 @@ func (m *mockUserRepo) ClearVerificationCode(userId uint32) (user.User, error) { } func (m *mockUserRepo) ResetPassword(userId uint32, hashedPassword string) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() m.resetPasswordCalls++ m.lastResetPasswordUserId = userId m.lastResetPasswordHash = hashedPassword @@ -357,6 +385,8 @@ func (m *mockUserRepo) ResetPassword(userId uint32, hashedPassword string) (user } func (m *mockUserRepo) UpdateDisplayName(userId uint32, displayName string) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() m.updateDisplayNameCalls++ m.lastUpdateDisplayName = displayName if err := m.fail.trigger("UpdateDisplayName"); err != nil { @@ -380,6 +410,8 @@ func (m *mockUserRepo) roleExists(id uint32) bool { } func (m *mockUserRepo) UpdateRole(id uint32, from uint32, to uint32, assignedBy uint32) (user.User, error) { + m.mu.Lock() + defer m.mu.Unlock() m.updateRoleCalls++ if err := m.fail.trigger("UpdateRole"); err != nil { return user.User{}, err @@ -387,7 +419,7 @@ func (m *mockUserRepo) UpdateRole(id uint32, from uint32, to uint32, assignedBy if m.beforeUpdateRole != nil { hook := m.beforeUpdateRole m.beforeUpdateRole = nil - hook() + outsideLock(&m.mu, hook) } stored, ok := m.users[id] if !ok { @@ -406,6 +438,8 @@ func (m *mockUserRepo) UpdateRole(id uint32, from uint32, to uint32, assignedBy } func (m *mockUserRepo) TransferOwnership(from uint32, to uint32, ownerRoleID uint32, formerRoleID uint32) error { + m.mu.Lock() + defer m.mu.Unlock() m.transferCalls++ if err := m.fail.trigger("TransferOwnership"); err != nil { return err @@ -413,7 +447,7 @@ func (m *mockUserRepo) TransferOwnership(from uint32, to uint32, ownerRoleID uin if m.beforeTransfer != nil { hook := m.beforeTransfer m.beforeTransfer = nil - hook() + outsideLock(&m.mu, hook) } former, formerFound := m.users[from] if !formerFound || former.RoleID != ownerRoleID { diff --git a/web/src/components/__tests__/EditSlugDialog.spec.ts b/web/src/components/__tests__/EditSlugDialog.spec.ts new file mode 100644 index 0000000..7a80acf --- /dev/null +++ b/web/src/components/__tests__/EditSlugDialog.spec.ts @@ -0,0 +1,96 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { flushPromises } from '@vue/test-utils' +import EditSlugDialog from '../EditSlugDialog.vue' +import { byText, mountInApp, text, unmountApp } from './directoryKit' + +const SLUG = 'comment-nous-chiffrons-une-renovation' + +function props(extra: Record = {}) { + return { language: 'fr', slug: SLUG, published: false, pending: false, error: '', ...extra } +} + +function input() { + return document.body.querySelector('#edit-slug-input')! +} + +function confirmButton() { + return byText('button', 'Change address') as HTMLButtonElement +} + +async function typeSlug(value: string) { + input().value = value + input().dispatchEvent(new Event('input', { bubbles: true })) + await flushPromises() +} + +afterEach(unmountApp) + +describe('Edit slug (loc.slug)', () => { + it('shows the title, the scope, the current slug and the title note', async () => { + await mountInApp(EditSlugDialog, props()) + + expect(text(document.body)).toContain('Edit slug for the fr version') + expect(text(document.body)).toContain('The address changes for fr only.') + expect(text(document.body)).toContain('Editing the title never changes the slug.') + expect(input().value).toBe(SLUG) + }) + + it.todo( + 'focuses the slug field on open: the modal auto-focus moves focus to the close button after the dialog focused the field', + ) + + it('keeps Change address disabled while the slug is unchanged or empty', async () => { + await mountInApp(EditSlugDialog, props()) + + expect(confirmButton().disabled).toBe(true) + await typeSlug(' ') + expect(confirmButton().disabled).toBe(true) + await typeSlug('couts-de-renovation') + expect(confirmButton().disabled).toBe(false) + }) + + it('emits the normalized slug on Change address', async () => { + const wrapper = await mountInApp(EditSlugDialog, props()) + + await typeSlug('Couts de Renovation') + confirmButton().click() + await flushPromises() + + expect(wrapper.findComponent(EditSlugDialog).emitted('confirm')).toEqual([ + ['couts-de-renovation'], + ]) + }) + + it('warns about old links on a published version only', async () => { + await mountInApp(EditSlugDialog, props({ published: true })) + + expect(text(document.body)).toContain('This version is published') + expect(text(document.body)).toContain( + 'Links to the old address will stop working. RenewCMS does not create redirects.', + ) + unmountApp() + + await mountInApp(EditSlugDialog, props()) + expect(text(document.body)).not.toContain('This version is published') + }) + + it('shows the error as an alert and marks the field invalid', async () => { + await mountInApp( + EditSlugDialog, + props({ error: `The slug "${SLUG}" is already used on Journal.` }), + ) + + const alert = document.body.querySelector('[role="alert"]') + expect(text(alert)).toBe(`The slug "${SLUG}" is already used on Journal.`) + expect(input().getAttribute('aria-invalid')).toBe('true') + }) + + it('emits close from Cancel', async () => { + const wrapper = await mountInApp(EditSlugDialog, props()) + + byText('button', 'Cancel')!.click() + await flushPromises() + + expect(wrapper.findComponent(EditSlugDialog).emitted('close')).toHaveLength(1) + }) +}) diff --git a/web/src/components/__tests__/IntegrationLanguages.spec.ts b/web/src/components/__tests__/IntegrationLanguages.spec.ts new file mode 100644 index 0000000..bf7c647 --- /dev/null +++ b/web/src/components/__tests__/IntegrationLanguages.spec.ts @@ -0,0 +1,88 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { initializeLocale, setLocale } from '@/i18n' +import { PUBLIC_LOOKUP_ROUTE, PUBLIC_STORY_ROUTE } from '@/lib/publicArticleRoutes' +import IntegrationLanguages from '../IntegrationLanguages.vue' +import { mountInApp, text, unmountApp } from './directoryKit' + +function card(id: string) { + return document.body.querySelector(`[data-testid="${id}"]`) +} + +beforeEach(() => { + localStorage.clear() + initializeLocale() +}) + +afterEach(() => { + unmountApp() + localStorage.clear() + initializeLocale() +}) + +describe('Integration: Languages section', () => { + it('anchors the section under the Languages heading with its intro', async () => { + await mountInApp(IntegrationLanguages) + + const section = card('integration-languages')! + expect(section.getAttribute('data-anchor')).toBe('languages') + expect(section.getAttribute('aria-labelledby')).toBe('h-languages') + expect(text(section.querySelector('#h-languages'))).toBe('Languages') + expect(text(section)).toContain( + 'Read one published language of an article, or resolve an article in a language. Both requests are public.', + ) + }) + + it('shows the exact lookup route, its no-fallback rule and a sample with alternates', async () => { + await mountInApp(IntegrationLanguages) + + const lookup = text(card('integration-lookup')) + expect(lookup).toContain('1. Exact lookup: site + language + slug') + expect(lookup).toContain(PUBLIC_LOOKUP_ROUTE) + expect(lookup).toContain('It never falls back, because a slug belongs to one language.') + expect(lookup).toContain('"alternates": [{ "id": 10, "lang": "en"') + }) + + it('shows the story route with the default fallback rules', async () => { + await mountInApp(IntegrationLanguages) + + const story = text(card('integration-story')) + expect(story).toContain('2. Resolve by article group, with fallback') + expect(story).toContain(PUBLIC_STORY_ROUTE) + expect(story).toContain('"requested_lang": "fr", "fallback": true') + expect(story).toContain('drafts are never returned.') + }) + + it('lists the four error codes and what each means', async () => { + await mountInApp(IntegrationLanguages) + + const errors = text(card('integration-errors')) + expect(errors).toContain('3. Errors') + expect(errors).toContain( + 'article_not_found · language_not_available · site_not_found · invalid_language', + ) + expect(errors).toContain('site_not_found: no site has that key.') + }) + + it('keeps the existing article-ID routes listed as unchanged', async () => { + await mountInApp(IntegrationLanguages) + + const unchanged = text(card('integration-unchanged')) + expect(unchanged).toContain('Existing, unchanged') + expect(unchanged).toContain('GET /v1/article/{id}') + expect(unchanged).toContain('PATCH /v1/article/{id}/publish') + expect(unchanged).toContain('Editor: /article/{id}/edit') + expect(unchanged).toContain( + 'Each article ID is one language version. Article-ID links keep working.', + ) + }) + + it('translates the copy and keeps the routes and codes literal in French', async () => { + setLocale('fr') + await mountInApp(IntegrationLanguages) + + const section = text(card('integration-languages')) + expect(text(card('integration-languages')!.querySelector('#h-languages'))).toBe('Langues') + expect(section).toContain(PUBLIC_LOOKUP_ROUTE) + expect(section).toContain('article_not_found · language_not_available') + }) +}) diff --git a/web/src/components/__tests__/SeoLanguageCards.spec.ts b/web/src/components/__tests__/SeoLanguageCards.spec.ts new file mode 100644 index 0000000..bbf219c --- /dev/null +++ b/web/src/components/__tests__/SeoLanguageCards.spec.ts @@ -0,0 +1,137 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { flushPromises } from '@vue/test-utils' +import { initializeLocale } from '@/i18n' +import type { LocalizedSeoContext } from '@/lib/localizedSeo' +import type { ResolvedSeo } from '@/lib/seoTypes' +import router from '@/router' +import SeoLanguageCards from '../SeoLanguageCards.vue' +import { byText, mountInApp, text, unmountApp } from './directoryKit' + +const PATTERN = 'https://journal.example.com/{lang}/{slug}' +const CANONICAL = 'https://journal.example.com/fr/comment-nous-chiffrons-une-renovation' + +const ALTERNATES = [ + { + id: 10, + lang: 'en', + story_id: 5, + slug: 'how-we-price-a-bathroom-renovation', + url: 'https://journal.example.com/en/how-we-price-a-bathroom-renovation', + }, + { id: 11, lang: 'de', story_id: 5, slug: 'so-kalkulieren-wir-eine-badsanierung', url: null }, +] + +function resolved(locale = 'fr_FR'): ResolvedSeo { + return { + title: 'Couts de renovation | Journal', + description: '', + canonical_url: CANONICAL, + robots: 'index, follow', + og: { + type: 'article', + title: '', + description: '', + image: '', + image_alt: '', + url: '', + site_name: 'Journal', + locale, + }, + twitter: { card: 'summary', title: '', description: '', image: '', image_alt: '' }, + image: { url: '', alt: '', source: 'none' }, + authors: [], + published_at: null, + modified_at: '2026-09-26T11:02:00Z', + fallbacks: { title: 'title', description: 'none', image: 'none', canonical: 'pattern' }, + json_ld: {}, + alternates: ALTERNATES, + } as ResolvedSeo +} + +function context(overrides: Partial = {}): LocalizedSeoContext { + return { + lang: 'fr', + siteName: 'Journal', + siteKey: 'journal', + pattern: PATTERN, + defaultLang: 'en', + disabledLangs: ['es'], + published: true, + socialState: 'derived', + resolved: resolved(), + ...overrides, + } +} + +function card(id: string) { + return document.body.querySelector(`[data-testid="${id}"]`) +} + +beforeEach(() => { + localStorage.clear() + initializeLocale() +}) + +afterEach(() => { + unmountApp() + vi.restoreAllMocks() +}) + +describe('Search and sharing cards for one language', () => { + it('shows the canonical with its source note', async () => { + await mountInApp(SeoLanguageCards, { context: context() }) + + const canonical = text(card('seo-canonical-card')) + expect(canonical).toContain('Canonical') + expect(canonical).toContain(CANONICAL) + expect(canonical).toContain(`Built from the Public URL pattern ${PATTERN}.`) + }) + + it('lists the published alternates with their URL, or their slug without one, and the disabled note', async () => { + await mountInApp(SeoLanguageCards, { context: context() }) + + const rows = [...document.body.querySelectorAll('[data-testid="seo-alternate-row"]')] + expect(rows.map((row) => text(row))).toEqual([ + 'en https://journal.example.com/en/how-we-price-a-bathroom-renovation Published', + 'de so-kalkulieren-wir-eine-badsanierung Published', + ]) + expect(rows[0]!.querySelector('[aria-label]')!.getAttribute('aria-label')).toBe('English') + const alternates = text(card('seo-alternates-card')) + expect(alternates).toContain('Alternate languages') + expect(alternates).toContain('Published versions in enabled languages only.') + expect(alternates).toContain('Drafts and the disabled Spanish language are left out.') + }) + + it('shows the derived social locale', async () => { + await mountInApp(SeoLanguageCards, { context: context() }) + + const social = text(card('seo-social-card')) + expect(social).toContain('Social locale') + expect(social).toContain('og:locale') + expect(social).toContain('Derived: fr_FR') + }) + + it('shows Social locale not set when nothing resolves', async () => { + await mountInApp(SeoLanguageCards, { context: context({ resolved: resolved('') }) }) + + expect(text(card('seo-social-card'))).toContain('Social locale not set') + }) + + it('replaces the canonical and alternates with Public URL not configured and opens its settings', async () => { + const push = vi.spyOn(router, 'push').mockResolvedValue(undefined) + await mountInApp(SeoLanguageCards, { context: context({ pattern: null }) }) + + expect(card('seo-canonical-card')).toBeNull() + expect(card('seo-alternates-card')).toBeNull() + expect(text(document.body)).toContain('Public URL not configured') + expect(text(document.body)).toContain( + 'Add a public URL pattern for Journal to show page and canonical addresses. API links by article ID still work.', + ) + expect(card('seo-social-card')).not.toBeNull() + + byText('button', 'Open Public URL settings')!.click() + await flushPromises() + + expect(push).toHaveBeenCalledWith('/settings/sites/journal/public-url') + }) +}) diff --git a/web/src/composables/__tests__/useEditorLocalizedSeo.spec.ts b/web/src/composables/__tests__/useEditorLocalizedSeo.spec.ts new file mode 100644 index 0000000..fe1a58b --- /dev/null +++ b/web/src/composables/__tests__/useEditorLocalizedSeo.spec.ts @@ -0,0 +1,262 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { computed, nextTick, ref } from 'vue' +import { flushPromises } from '@vue/test-utils' +import { initializeLocale } from '@/i18n' +import type { Article } from '@/composables/useArticles' +import type { SaveOutcome, useArticleDraft } from '@/composables/useArticleDraft' +import type { useEditorVersions } from '@/composables/useEditorVersions' +import type { Site } from '@/composables/useSites' +import { useEditorLocalizedSeo } from '@/composables/useEditorLocalizedSeo' +import type { ResolvedSeo } from '@/lib/seoTypes' +import type { SiteLanguage } from '@/lib/siteLanguages' + +const fixtures = vi.hoisted(() => ({ + resolved: { value: null as unknown }, + load: vi.fn(), + toasts: [] as (() => { title: string; description: string; color: string })[], +})) + +vi.mock('@/composables/useResolvedSeo', async () => { + const { ref: vueRef } = await import('vue') + const resolved = vueRef(null) + fixtures.resolved = resolved + return { useResolvedSeo: () => ({ resolved, load: fixtures.load }) } +}) + +vi.mock('@/i18n/presentation', () => ({ + useLocalizedToast: () => ({ + add: (content: () => { title: string; description: string; color: string }) => { + fixtures.toasts.push(content) + }, + }), +})) + +const SITE = { + id: 1, + key: 'journal', + name: 'Journal (internal)', + site_name: 'Journal', + public_url_pattern: 'https://journal.example.com/{lang}/{slug}', +} as Site + +function language(lang: string, extra: Partial = {}): SiteLanguage { + return { + lang, + enabled: true, + is_default: false, + version_count: 1, + published_count: 1, + social_locale: null, + social_locale_state: 'derived', + ...extra, + } +} + +const LANGUAGES = [ + language('en', { is_default: true }), + language('fr', { social_locale_state: 'set', social_locale: 'fr_CA' }), + language('es', { enabled: false, published_count: 2 }), + language('it', { enabled: false, published_count: 0 }), +] + +function setup( + options: { + multilingual?: boolean + site?: Site | undefined + article?: Partial
| null + outcome?: SaveOutcome + panelOpen?: boolean + } = {}, +) { + const article = ref
( + options.article === null + ? null + : ({ id: 12, lang: 'fr', is_online: true, ...options.article } as Article), + ) + const site = ref('site' in options ? options.site : SITE) + const panelOpen = ref(options.panelOpen ?? false) + const seo = ref({ slug: 'couts', seo_title: '' }) + const draft = { + seo, + editSeo: vi.fn((next: Record) => { + seo.value = { ...seo.value, ...next } + }), + } as unknown as ReturnType + const versions = { + multilingual: computed(() => options.multilingual ?? true), + siteLanguages: ref(LANGUAGES), + } as unknown as ReturnType + const save = vi.fn(async () => options.outcome ?? 'saved') + const seoState = useEditorLocalizedSeo({ + articleId: 12, + article, + site: computed(() => site.value), + versions, + draft, + panelOpen, + save, + }) + return { seoState, article, panelOpen, draft, save, seo } +} + +beforeEach(() => { + localStorage.clear() + initializeLocale() + fixtures.load.mockReset() + fixtures.toasts.length = 0 + ;(fixtures.resolved as { value: unknown }).value = null +}) + +describe('Localized SEO context', () => { + it('builds the context from the site, the version and the site languages', () => { + const { seoState } = setup() + + expect(seoState.context.value).toEqual({ + lang: 'fr', + siteName: 'Journal', + siteKey: 'journal', + pattern: 'https://journal.example.com/{lang}/{slug}', + defaultLang: 'en', + disabledLangs: ['es'], + published: true, + socialState: 'set', + resolved: null, + }) + }) + + it('falls back to the site name, a null pattern and no default', () => { + const { seoState } = setup({ + site: { id: 2, key: 'atelier', name: 'Atelier', site_name: '' } as Site, + article: { lang: 'de', is_online: false }, + }) + + expect(seoState.context.value).toMatchObject({ + siteName: 'Atelier', + pattern: null, + published: false, + socialState: '', + }) + }) + + it('carries the resolved SEO once it is loaded', async () => { + const { seoState } = setup() + const answer = { canonical_url: 'https://journal.example.com/fr/couts' } as ResolvedSeo + + ;(fixtures.resolved as { value: unknown }).value = answer + await nextTick() + + expect(seoState.context.value?.resolved).toStrictEqual(answer) + }) + + it('has no context on a single-language site, without a site or without a language', () => { + expect(setup({ multilingual: false }).seoState.context.value).toBeNull() + expect(setup({ site: undefined }).seoState.context.value).toBeNull() + expect(setup({ article: { lang: '' } }).seoState.context.value).toBeNull() + expect(setup({ article: null }).seoState.context.value).toBeNull() + }) +}) + +describe('Resolved SEO loading', () => { + it('loads when the panel opens with a context, and not before', async () => { + const { panelOpen } = setup() + expect(fixtures.load).not.toHaveBeenCalled() + + panelOpen.value = true + await nextTick() + + expect(fixtures.load).toHaveBeenCalledTimes(1) + }) + + it('loads at once when the panel is already open', () => { + setup({ panelOpen: true }) + + expect(fixtures.load).toHaveBeenCalledTimes(1) + }) + + it('never loads without a context', async () => { + const { panelOpen } = setup({ multilingual: false }) + + panelOpen.value = true + await nextTick() + + expect(fixtures.load).not.toHaveBeenCalled() + }) +}) + +describe('Edit slug', () => { + it('opens with no error and closes on request', () => { + const { seoState } = setup() + + seoState.openSlugDialog() + expect(seoState.slugDialogOpen.value).toBe(true) + expect(seoState.slugError.value).toBe('') + seoState.closeSlugDialog() + expect(seoState.slugDialogOpen.value).toBe(false) + }) + + it('saves the slug, closes, reloads the resolved SEO and confirms with a toast', async () => { + const { seoState, save, seo } = setup() + seoState.openSlugDialog() + + const pending = seoState.confirmSlug('couts-de-renovation') + expect(seoState.slugPending.value).toBe(true) + await pending + await flushPromises() + + expect(seo.value.slug).toBe('couts-de-renovation') + expect(save).toHaveBeenCalledTimes(1) + expect(seoState.slugPending.value).toBe(false) + expect(seoState.slugDialogOpen.value).toBe(false) + expect(fixtures.load).toHaveBeenCalledTimes(1) + const toast = fixtures.toasts[0]!() + expect(toast.color).toBe('success') + expect(toast.title).toBe('Address changed') + expect(toast.description).toBe('The address of the fr version is now couts-de-renovation.') + }) + + it('stays open with the taken slug named on the site', async () => { + const { seoState } = setup({ outcome: 'slug-taken' }) + seoState.openSlugDialog() + + await seoState.confirmSlug('kosten') + + expect(seoState.slugDialogOpen.value).toBe(true) + expect(seoState.slugError.value).toBe('The slug "kosten" is already used on Journal.') + expect(fixtures.toasts).toHaveLength(0) + }) + + it('stays open with the slug rules on an invalid slug', async () => { + const { seoState } = setup({ outcome: 'invalid-slug' }) + seoState.openSlugDialog() + + await seoState.confirmSlug('--') + + expect(seoState.slugDialogOpen.value).toBe(true) + expect(seoState.slugError.value).toBe( + 'Lowercase letters, numbers and hyphens. Unique on Journal.', + ) + }) + + it('closes on a revision conflict so the conflict dialog can take over', async () => { + const { seoState } = setup({ outcome: 'conflict' }) + seoState.openSlugDialog() + + await seoState.confirmSlug('couts-2') + + expect(seoState.slugDialogOpen.value).toBe(false) + expect(seoState.slugError.value).toBe('') + expect(fixtures.toasts).toHaveLength(0) + }) + + it('stays open without a message on another failure, and reopening clears the last error', async () => { + const { seoState } = setup({ outcome: 'error' }) + seoState.openSlugDialog() + + await seoState.confirmSlug('couts-3') + + expect(seoState.slugDialogOpen.value).toBe(true) + expect(seoState.slugError.value).toBe('') + seoState.openSlugDialog() + expect(seoState.slugError.value).toBe('') + }) +}) diff --git a/web/src/data/__tests__/apiCatalogue.spec.ts b/web/src/data/__tests__/apiCatalogue.spec.ts index 327cb4b..cfd09e5 100644 --- a/web/src/data/__tests__/apiCatalogue.spec.ts +++ b/web/src/data/__tests__/apiCatalogue.spec.ts @@ -28,6 +28,7 @@ const EXPECTED_GROUPS: Record = { 'PATCH /v1/article/{id}/publish', 'PATCH /v1/article/{id}/unpublish', 'DELETE /v1/article/{id}', + 'DELETE /v1/story/{story_id}', 'POST /v1/article/{id}/seo/check', ], 'Search and sharing': ['GET /v1/article/{id}/seo', 'GET /v1/site/{key}/sitemap'], @@ -245,7 +246,7 @@ function expectOnlyKeys(value: unknown, allowed: string[]) { } describe('Catalogue routes', () => { - it('holds exactly the 45 key-callable routes in 9 groups, in order', () => { + it('holds exactly the 46 key-callable routes in 9 groups, in order', () => { expect(API_GROUPS.map((group) => group.title)).toEqual([ 'Articles', 'Search and sharing', @@ -262,12 +263,12 @@ describe('Catalogue routes', () => { API_GROUPS.map((group) => [group.title, group.endpoints.map(label)]), ), ).toEqual(EXPECTED_GROUPS) - expect(allEndpoints()).toHaveLength(45) + expect(allEndpoints()).toHaveLength(46) }) it('counts the routes of each group', () => { expect(API_GROUPS.map((group) => group.endpoints.length)).toEqual([ - 10, 2, 5, 5, 2, 11, 3, 6, 1, + 11, 2, 5, 5, 2, 11, 3, 6, 1, ]) }) @@ -297,7 +298,7 @@ describe('Catalogue routes', () => { ), ...PUBLIC_AUTH_ROWS.map((item) => [label(item), item.description] as const), ] - expect(described).toHaveLength(9 + 45 + 40 + 6) + expect(described).toHaveLength(9 + 46 + 40 + 6) const blank = described.filter(([, description]) => description.trim() === '') expect(blank.map(([name]) => name)).toEqual([]) }) @@ -632,9 +633,10 @@ describe('Response samples use the real domain JSON keys', () => { ]) }) - it('answers 204 for the three deletes without a JSON body', () => { + it('answers 204 for the four deletes without a JSON body', () => { for (const route of [ 'DELETE /v1/article/{id}', + 'DELETE /v1/story/{story_id}', 'DELETE /v1/media/{id}', 'DELETE /v1/user/invite/{id}', ]) { diff --git a/web/src/data/apiCatalogue.ts b/web/src/data/apiCatalogue.ts index 5405c1c..52dd914 100644 --- a/web/src/data/apiCatalogue.ts +++ b/web/src/data/apiCatalogue.ts @@ -1076,6 +1076,23 @@ export const API_GROUPS: ApiGroup[] = [ NO_CONTENT_RESPONSE, { query: [FORMAT_QUERY] }, ), + endpoint( + 'story-del', + 'DELETE', + '/v1/story/{story_id}', + 'Delete a whole article: every version of the story, readable or not, in one step. Needs the permission to delete content at scope all.', + 'Verified user', + verified( + articleErrors( + e(400, ID_FORMAT), + eMissingPermission, + e(404, 'Article not found'), + e(500, 'Failed to delete article'), + ), + ), + NO_CONTENT_RESPONSE, + { query: [FORMAT_QUERY] }, + ), endpoint( 'art-check', 'POST', diff --git a/web/src/i18n/messages.ts b/web/src/i18n/messages.ts index eed6c83..2ab6a02 100644 --- a/web/src/i18n/messages.ts +++ b/web/src/i18n/messages.ts @@ -1666,6 +1666,8 @@ export const messages = { 'Publish an article.': 'Publie un article.', 'Unpublish an article.': 'Dépublie un article.', 'Delete an article.': 'Supprime un article.', + 'Delete a whole article: every version of the story, readable or not, in one step. Needs the permission to delete content at scope all.': + 'Supprime un article entier : toutes ses versions, lisibles ou non, en une seule fois. Demande la permission de supprimer du contenu sur la portée « tout ».', 'List images.': 'Liste les images.', 'Upload an image (multipart).': 'Téléverse une image (multipart).', 'Rename an image. The URL stays the same.': 'Renomme une image. L’URL reste la même.', diff --git a/web/src/lib/__tests__/apiSearch.spec.ts b/web/src/lib/__tests__/apiSearch.spec.ts index cf73b42..702114e 100644 --- a/web/src/lib/__tests__/apiSearch.spec.ts +++ b/web/src/lib/__tests__/apiSearch.spec.ts @@ -65,9 +65,9 @@ describe('index', () => { ]) }) - it('holds 91 endpoints: 45 documented, 40 session only and 6 public auth', () => { - expect(index.endpoints).toHaveLength(91) - expect(index.endpoints.filter((entry) => entry.cardId !== null)).toHaveLength(45) + it('holds 92 endpoints: 46 documented, 40 session only and 6 public auth', () => { + expect(index.endpoints).toHaveLength(92) + expect(index.endpoints.filter((entry) => entry.cardId !== null)).toHaveLength(46) expect(index.endpoints.filter((entry) => entry.session)).toHaveLength(40) expect(index.endpoints.filter((entry) => entry.key.startsWith('public:'))).toHaveLength(6) }) @@ -202,7 +202,7 @@ describe('matching', () => { describe('custom content types', () => { it.each([ ['media', 16, 1], - ['404', 26, 1], + ['404', 27, 1], ['login', 1, 1], ['entry', 7, 1], ['capacity', 2, 0], diff --git a/web/src/lib/__tests__/localizedSeo.spec.ts b/web/src/lib/__tests__/localizedSeo.spec.ts new file mode 100644 index 0000000..46911c9 --- /dev/null +++ b/web/src/lib/__tests__/localizedSeo.spec.ts @@ -0,0 +1,197 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { initializeLocale, setLocale } from '@/i18n' +import type { ResolvedSeo } from '@/lib/seoTypes' +import { + altHelp, + breadcrumbOf, + canonicalNote, + descriptionBadge, + descriptionHelp, + disabledNote, + pageLanguage, + socialLocaleLine, + socialLocaleNote, + titleHelp, + type LocalizedSeoContext, +} from '../localizedSeo' + +const PATTERN = 'https://journal.example.com/{lang}/{slug}' + +function resolved( + fallbacks: Partial = {}, + locale = 'fr_FR', +): ResolvedSeo { + return { + title: 'Couts de renovation | Journal', + description: '', + canonical_url: 'https://journal.example.com/fr/couts', + robots: 'index, follow', + og: { + type: 'article', + title: '', + description: '', + image: '', + image_alt: '', + url: '', + site_name: 'Journal', + locale, + }, + twitter: { card: 'summary', title: '', description: '', image: '', image_alt: '' }, + image: { url: '', alt: '', source: 'none' }, + authors: [], + published_at: null, + modified_at: '2026-09-26T11:02:00Z', + fallbacks: { + title: 'title', + description: 'none', + image: 'none', + canonical: 'pattern', + ...fallbacks, + }, + json_ld: {}, + } +} + +function context(overrides: Partial = {}): LocalizedSeoContext { + return { + lang: 'fr', + siteName: 'Journal', + siteKey: 'journal', + pattern: PATTERN, + defaultLang: 'en', + disabledLangs: [], + published: true, + socialState: 'derived', + resolved: resolved(), + ...overrides, + } +} + +beforeEach(() => { + localStorage.clear() + initializeLocale() +}) + +afterEach(() => { + localStorage.clear() + initializeLocale() +}) + +describe('Page language and breadcrumb', () => { + it('names the version language in the interface language', () => { + expect(pageLanguage(context())).toBe('French') + setLocale('fr') + expect(pageLanguage(context())).toBe('Français') + }) + + it('turns a URL into host › segments and keeps anything else as is', () => { + expect(breadcrumbOf('https://journal.example.com/fr/couts')).toBe( + 'journal.example.com › fr › couts', + ) + expect(breadcrumbOf('not a url')).toBe('not a url') + }) +}) + +describe('Canonical note', () => { + it('says the canonical is built from the Public URL pattern', () => { + expect(canonicalNote(context())).toBe( + `Built from the Public URL pattern ${PATTERN}. Each language points to itself unless you set a canonical override.`, + ) + }) + + it('says a canonical override was set by hand', () => { + expect(canonicalNote(context({ resolved: resolved({ canonical: 'canonical_url' }) }))).toBe( + 'Set by hand as a canonical override on this version.', + ) + }) +}) + +describe('Disabled languages note', () => { + it('leaves out drafts only when no language is disabled', () => { + expect(disabledNote(context())).toBe('Drafts are left out.') + }) + + it('names one disabled language', () => { + expect(disabledNote(context({ disabledLangs: ['es'] }))).toBe( + 'Drafts and the disabled Spanish language are left out.', + ) + }) + + it('lists several disabled languages', () => { + expect(disabledNote(context({ disabledLangs: ['es', 'de'] }))).toBe( + 'Drafts and the disabled languages Spanish, German are left out.', + ) + }) +}) + +describe('Social locale', () => { + it('reads Derived for a locale taken from the language code', () => { + const derived = context() + expect(socialLocaleLine(derived)).toBe('Derived: fr_FR') + expect(socialLocaleNote(derived)).toBe( + 'Taken from the region of the language code or of the site locale. An owner can set another in Journal languages.', + ) + }) + + it('reads Set for a locale an owner chose', () => { + const set = context({ socialState: 'set' }) + expect(socialLocaleLine(set)).toBe('Set: fr_FR') + expect(socialLocaleNote(set)).toBe( + 'Set by an owner for French in Journal languages. A region is never guessed from the language.', + ) + }) + + it('reads Social locale not set when nothing resolves', () => { + const unset = context({ resolved: resolved({}, '') }) + expect(socialLocaleLine(unset)).toBe('Social locale not set') + expect(socialLocaleNote(unset)).toBe( + 'French has no region in its code and no social locale is set for it, so og:locale and og:locale:alternate are left out. An owner can set one in Journal languages.', + ) + expect(socialLocaleLine(context({ resolved: null }))).toBe('Social locale not set') + }) +}) + +describe('Field help follows the fallbacks of the API', () => { + it('names the title fallback only when the title is used', () => { + expect(titleHelp(context())).toBe('Fallback: French title') + expect(titleHelp(context({ resolved: resolved({ title: 'seo_title' }) }))).toBe('') + expect(titleHelp(context({ resolved: null }))).toBe('') + }) + + it('names each description fallback', () => { + const help = (description: string, siteTextSkipped = false) => + descriptionHelp( + context({ + resolved: resolved({ description, site_text_skipped: siteTextSkipped }), + }), + ) + expect(help('excerpt')).toBe('Fallback: French excerpt') + expect(help('body')).toBe('Fallback: French body') + expect(help('site')).toBe('Fallback: French site description') + expect(help('none')).toBe('No fallback') + expect(help('none', true)).toBe( + 'No fallback. Empty. English site default not used. The tag is left out.', + ) + expect(help('seo_description')).toBe('') + expect(descriptionHelp(context({ resolved: null }))).toBe('') + }) + + it('badges the site text and the withheld tag', () => { + expect(descriptionBadge(context({ resolved: resolved({ description: 'site' }) }))).toBe( + 'Uses site text', + ) + expect( + descriptionBadge( + context({ resolved: resolved({ description: 'none', site_text_skipped: true }) }), + ), + ).toBe('Tag left out') + expect(descriptionBadge(context({ resolved: resolved({ description: 'excerpt' }) }))).toBe( + '', + ) + expect(descriptionBadge(context({ resolved: null }))).toBe('') + }) + + it('says the image alt is written per language', () => { + expect(altHelp()).toBe('Written per language.') + }) +}) diff --git a/web/src/views/__tests__/ArticleLocalization.spec.ts b/web/src/views/__tests__/ArticleLocalization.spec.ts index 9101d3a..fa4bcf7 100644 --- a/web/src/views/__tests__/ArticleLocalization.spec.ts +++ b/web/src/views/__tests__/ArticleLocalization.spec.ts @@ -40,14 +40,14 @@ afterEach(async () => { await unmountMeta() }) -function backendFor(viewer: StoryViewer = {}, reader = false): MetaBackend { +function backendFor(viewer: StoryViewer = {}, reader = false, defaultKey = 'journal'): MetaBackend { const [first] = renewSites() const sites = ['journal', 'atelier', 'press'].map((key, index) => ({ ...first!, id: index + 1, key, name: key.charAt(0).toUpperCase() + key.slice(1), - is_default: index === 0, + is_default: key === defaultKey, })) const backend = createMetaBackend({ sites }) if (reader) { @@ -584,6 +584,17 @@ describe('URL state', () => { expect(first.get('tab')).toBe(expected.tab ?? null) }) + it('lists every site and takes the filter options from the default site when the URL names no site', async () => { + const backend = backendFor({}, false, 'atelier') + await mountMeta('articles', '/article', { backend }) + await flushPromises() + + expect(currentQuery()).toEqual({}) + expect(listCalls(backend).every((call) => !call.query.has('site'))).toBe(true) + expect(callsTo(backend, 'GET', '/v1/site/atelier/languages')).not.toHaveLength(0) + expect(callsTo(backend, 'GET', '/v1/site/journal/languages')).toHaveLength(0) + }) + it('drops a language the site does not have once the languages are known', async () => { const backend = backendFor() await mountMeta('articles', '/article?site=journal&lang=pt', { backend }) diff --git a/web/src/views/__tests__/EditorLanguageMenu.spec.ts b/web/src/views/__tests__/EditorLanguageMenu.spec.ts index 4030ef2..2642062 100644 --- a/web/src/views/__tests__/EditorLanguageMenu.spec.ts +++ b/web/src/views/__tests__/EditorLanguageMenu.spec.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import router from '@/router' import { initializeLocale } from '@/i18n' +import { announce } from '@/composables/useLiveAnnouncement' import { MOBILE_WIDTH } from '@/components/__tests__/viewport' import { settle, text, unmountRoute } from './localizationKit' import { @@ -22,7 +23,11 @@ vi.mock('@/composables/useApi', async (importOriginal) => { return { ...actual, useApi: vi.fn() } }) -beforeEach(() => initializeLocale()) +beforeEach(() => { + initializeLocale() + sessionStorage.clear() + announce('') +}) afterEach(unmountRoute) const LANGUAGE_NAMES = ['English', 'Arabic', 'French', 'German'] @@ -189,6 +194,35 @@ describe('The menu is keyboard operable', () => { expect(document.activeElement?.textContent).toContain('French') }) + it('opens on Enter with focus on the current row', async () => { + await mountEditor() + languageTrigger()!.focus() + languageTrigger()!.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }), + ) + await settle() + + expect(document.body.querySelector('[role="menu"]')).not.toBeNull() + expect(document.activeElement?.textContent).toContain('French') + }) + + it('switches to the version chosen with Enter', async () => { + await mountEditor() + languageTrigger()!.focus() + languageTrigger()!.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }), + ) + await settle() + const english = rowFor('English')! + english.focus() + english.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })) + await settle() + + expect(router.currentRoute.value.path).toBe('/article/10/edit') + expect(document.body.querySelector('[role="menu"]')).toBeNull() + expect(titleInput().value).toBe('Renovation costs') + }) + it('focuses the current row when the sheet opens on a phone', async () => { await mountEditor({ width: MOBILE_WIDTH }) await openLanguageMenu() diff --git a/web/src/views/__tests__/EditorRecovery.spec.ts b/web/src/views/__tests__/EditorRecovery.spec.ts index 6bcc7fb..8cd9143 100644 --- a/web/src/views/__tests__/EditorRecovery.spec.ts +++ b/web/src/views/__tests__/EditorRecovery.spec.ts @@ -12,6 +12,8 @@ import { } from './localizationKit' import { EDIT_ONLY, READ, articleOf, editBody, mountEditor, titleInput } from './editorKit' import { removeStoredDraftsOf } from '@/lib/storedDraft' +import { ApiError, redirectToLogin } from '@/composables/useApi' +import router from '@/router' vi.mock('@/composables/useApi', async (importOriginal) => { const actual = await importOriginal() @@ -53,6 +55,33 @@ describe('A 401 on save mirrors the draft for this user and article', () => { }) }) +describe('A 401 on save sends the writer to log in and keeps the draft for after', () => { + it('redirects to /login with the editor as redirect and reason expired, then restores the draft', async () => { + const expireSession = () => { + redirectToLogin() + throw new ApiError(401) + } + await mountEditor({ extra: { 'PATCH /v1/article/12': expireSession } }) + await typeInto(titleInput(), 'Couts reels') + await editBody('

Edited body

') + await press('Save') + await settle() + + expect(router.currentRoute.value.path).toBe('/login') + expect(router.currentRoute.value.query).toEqual({ + redirect: '/article/12/edit', + reason: 'expired', + }) + + await unmountRoute() + await mountEditor() + + expect(titleInput().value).toBe('Couts reels') + expect(editorBody()).toBe('

Edited body

') + expect(text()).toContain('Unsaved changes') + }) +}) + describe('loc.l11.restored: Restoring after login', () => { it('shows the stored title and body, not the loaded ones, with the info alert and unsaved state', async () => { store() diff --git a/web/src/views/__tests__/IntegrationSearch.spec.ts b/web/src/views/__tests__/IntegrationSearch.spec.ts index d8aff7c..46c0ad4 100644 --- a/web/src/views/__tests__/IntegrationSearch.spec.ts +++ b/web/src/views/__tests__/IntegrationSearch.spec.ts @@ -382,11 +382,11 @@ describe('8d More endpoints than the cap', () => { expect(options().at(-1)!.getAttribute('aria-label')).toBe( 'Section Errors. Status codes and messages', ) - expect(document.body.textContent).toContain('27 results') + expect(document.body.textContent).toContain('28 results') expect(document.body.textContent).toContain( - '20 more endpoints. Keep typing to narrow the list.', + '21 more endpoints. Keep typing to narrow the list.', ) - expect(liveRegion()).toBe('27 results, showing 7') + expect(liveRegion()).toBe('28 results, showing 7') }) }) diff --git a/web/src/views/__tests__/IntegrationView.spec.ts b/web/src/views/__tests__/IntegrationView.spec.ts index aeb896e..1bbfe9d 100644 --- a/web/src/views/__tests__/IntegrationView.spec.ts +++ b/web/src/views/__tests__/IntegrationView.spec.ts @@ -176,7 +176,7 @@ describe('Default, every card collapsed', () => { }) it.each([ - ['articles', 'Articles', '10 routes', 10], + ['articles', 'Articles', '11 routes', 11], ['seo', 'Search and sharing', '2 routes', 2], ['ctypes', 'Content types', '5 routes', 5], ['entries', 'Entries', '5 routes', 5], diff --git a/web/visual/touchTargets.ts b/web/visual/touchTargets.ts new file mode 100644 index 0000000..f734aab --- /dev/null +++ b/web/visual/touchTargets.ts @@ -0,0 +1,213 @@ +import { join, resolve } from 'node:path' +import { build, preview } from 'vite' +import { chromium, type Browser, type Locator, type Page } from 'playwright' +import { listApi } from './articleListStates' +import { visualStates, type ApiMock, type VisualState } from './states' + +interface TouchTarget { + name: string + find: (page: Page) => Locator + todo?: string +} + +interface TouchCheck { + label: string + setup: VisualState + targets: TouchTarget[] +} + +const MOBILE = { width: 390, height: 844 } +const MIN_TARGET_PX = 44 +const SETTLE_MS = 400 + +const webRoot = resolve(import.meta.dirname, '..') +const distRoot = join(webRoot, '..', 'tmp', 'visual', 'touch-dist') +const configFile = join(webRoot, 'vite.config.ts') + +const shellApi: Record = { + 'GET /v1/content-type': { status: 200, body: [] }, +} + +const editorStates = visualStates['localization-editor'] ?? {} + +function editorState(name: string) { + const setup = editorStates[name] + if (!setup) { + throw new Error(`No localization-editor setup named ${name}`) + } + return setup +} + +const byRole = (role: 'button' | 'menuitem', name: string | RegExp) => (page: Page) => + page.getByRole(role, { name, exact: typeof name === 'string' }) + +const checks: TouchCheck[] = [ + { + label: 'Editor header (loc.l5.default)', + setup: editorState('loc.l5.default'), + targets: [ + { name: 'Language switcher', find: byRole('button', /Switch version$/) }, + { name: 'Save', find: byRole('button', 'Save') }, + { name: 'Publish French', find: byRole('button', 'Publish French') }, + { name: 'More actions', find: byRole('button', 'More actions') }, + ], + }, + { + label: 'Language sheet rows (loc.l5.menu)', + setup: editorState('loc.l5.menu'), + targets: ['English', 'French', 'German', 'Arabic'].map((language) => ({ + name: `${language} row`, + find: byRole('menuitem', new RegExp(`^${language}`)), + })), + }, + { + label: 'Article list coverage overflow chip (loc.l7.long)', + setup: { + path: '/article?site=atelier&status=draft&seo=suggestions&size=5', + api: listApi(), + }, + targets: [ + { + name: '+N chip', + find: byRole('button', /more languages?:/), + todo: 'components.md wants a 44 px tap area at 390 px; StoryCoverage draws the 22 px chip with no larger hit area (Phase 4)', + }, + ], + }, +] + +async function serveApp() { + await build({ + configFile, + root: webRoot, + logLevel: 'warn', + build: { outDir: distRoot, emptyOutDir: true }, + }) + const server = await preview({ + configFile, + root: webRoot, + logLevel: 'warn', + build: { outDir: distRoot }, + preview: { port: 4190, strictPort: false, open: false }, + }) + const origin = server.resolvedUrls?.local[0]?.replace(/\/$/, '') + if (!origin) { + throw new Error('Vite preview did not report a local URL') + } + return { origin, close: () => server.close() } +} + +function fulfilment(mock: ApiMock, url: URL, call: number) { + const body = mock.bodyFor ? mock.bodyFor(url, call) : mock.body + const text = typeof body === 'string' + return { + status: mock.status, + headers: { ...mock.headers, ...mock.headersFor?.(url, call) }, + contentType: text ? 'text/plain; charset=utf-8' : 'application/json', + body: body === undefined ? '' : text ? (body as string) : JSON.stringify(body), + } +} + +async function openState(browser: Browser, origin: string, setup: VisualState) { + const context = await browser.newContext({ viewport: MOBILE, reducedMotion: 'reduce' }) + await context.route( + (url) => url.hostname.startsWith('api.') && url.hostname !== new URL(origin).hostname, + (route) => route.fulfill({ status: 404, body: '' }), + ) + const calls = new Map() + await context.route('**/v1/**', async (route) => { + const request = route.request() + const url = new URL(request.url()) + const key = `${request.method()} ${url.pathname}` + const call = (calls.get(key) ?? 0) + 1 + calls.set(key, call) + const mock = setup.api?.[key] ?? shellApi[key] + if (!mock) { + await route.fulfill({ status: 404, body: '' }) + return + } + if (mock.hang) { + return + } + await route.fulfill(fulfilment(mock, url, call)) + }) + const page = await context.newPage() + await page.goto(`${origin}${setup.path}`) + await Promise.race([page.waitForLoadState('networkidle'), page.waitForTimeout(2000)]) + await page.waitForFunction( + () => (document.querySelector('#app')?.textContent ?? '').trim().length > 0, + ) + await setup.steps?.(page) + await page.waitForTimeout(SETTLE_MS) + return page +} + +async function hitHeight(target: Locator) { + return target.evaluate((element) => { + const box = element.getBoundingClientRect() + const x = box.left + box.width / 2 + const middle = box.top + box.height / 2 + const hits = (y: number) => { + const found = document.elementFromPoint(x, y) + return found !== null && (found === element || element.contains(found)) + } + let top = middle + while (top > middle - 40 && hits(top - 1)) { + top -= 1 + } + let bottom = middle + while (bottom < middle + 40 && hits(bottom + 1)) { + bottom += 1 + } + return { box: box.height, hit: bottom - top + 1 } + }) +} + +async function main() { + const app = await serveApp() + const browser = await chromium.launch() + const lines: string[] = [] + let failures = 0 + let todos = 0 + try { + for (const check of checks) { + const page = await openState(browser, app.origin, check.setup) + for (const target of check.targets) { + const located = target.find(page).first() + if ((await located.count()) === 0) { + failures++ + lines.push(`FAIL ${check.label} · ${target.name}: not found`) + continue + } + const { box, hit } = await hitHeight(located) + const height = Math.max(Math.round(box), hit) + const pass = height >= MIN_TARGET_PX + const measured = `box ${box.toFixed(1)} px, tap area ${hit} px` + if (target.todo && !pass) { + todos++ + lines.push(`TODO ${check.label} · ${target.name}: ${measured} (${target.todo})`) + continue + } + if (!pass) { + failures++ + } + lines.push(`${pass ? 'PASS' : 'FAIL'} ${check.label} · ${target.name}: ${measured}`) + } + await page.context().close() + } + } finally { + await browser.close() + await app.close() + } + console.log(lines.join('\n')) + console.log( + failures + ? `FAIL: ${failures} touch targets under ${MIN_TARGET_PX} px at 390 px` + : `PASS: every checked touch target is at least ${MIN_TARGET_PX} px high at 390 px${todos ? `, ${todos} known gap${todos > 1 ? 's' : ''} listed as TODO` : ''}`, + ) + if (failures) { + process.exitCode = 1 + } +} + +await main()