From 351bf95530ec81351055b131f18db031dd6923c6 Mon Sep 17 00:00:00 2001 From: Shaobiao Lin Date: Sun, 17 May 2026 23:09:23 +0800 Subject: [PATCH 1/2] feat(cli): add URL prefix fix command closes #861 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cmd/command.go | 58 +++++- internal/cli/fix.go | 327 +++++++++++++++++++++++++++++++++ internal/cli/fix_avatar.go | 92 ++++++++++ internal/cli/fix_branding.go | 105 +++++++++++ internal/cli/fix_cache.go | 63 +++++++ internal/cli/fix_filerecord.go | 57 ++++++ internal/cli/fix_post.go | 188 +++++++++++++++++++ 7 files changed, 889 insertions(+), 1 deletion(-) create mode 100644 internal/cli/fix.go create mode 100644 internal/cli/fix_avatar.go create mode 100644 internal/cli/fix_branding.go create mode 100644 internal/cli/fix_cache.go create mode 100644 internal/cli/fix_filerecord.go create mode 100644 internal/cli/fix_post.go diff --git a/cmd/command.go b/cmd/command.go index ca01aa80b..00470efea 100644 --- a/cmd/command.go +++ b/cmd/command.go @@ -59,6 +59,10 @@ var ( resetPasswordEmail string // resetPasswordPassword new password for password reset resetPasswordPassword string + // fixDryRun decides whether to run fix command in dry-run mode, it prints the affected rows but does not update the database + fixDryRun bool + // fixLimit limits the total number of rows fixed across all selected types. 0 means unlimited. + fixLimit int64 ) func init() { @@ -85,7 +89,10 @@ func init() { resetPasswordCmd.Flags().StringVarP(&resetPasswordEmail, "email", "e", "", "user email address") resetPasswordCmd.Flags().StringVarP(&resetPasswordPassword, "password", "p", "", "new password (not recommended, will be recorded in shell history)") - for _, cmd := range []*cobra.Command{initCmd, checkCmd, runCmd, dumpCmd, upgradeCmd, buildCmd, pluginCmd, configCmd, i18nCmd, resetPasswordCmd} { + fixCmd.Flags().BoolVar(&fixDryRun, "dry-run", false, "show what would be changed without modifying the database") + fixCmd.Flags().Int64Var(&fixLimit, "limit", 0, "stop after fixing NUM rows total across all selected fix types (0 = unlimited)") + + for _, cmd := range []*cobra.Command{initCmd, checkCmd, runCmd, dumpCmd, upgradeCmd, buildCmd, pluginCmd, configCmd, i18nCmd, resetPasswordCmd, fixCmd} { rootCmd.AddCommand(cmd) } } @@ -332,6 +339,55 @@ To run answer, use: } }, } + + fixCmd = &cobra.Command{ + Use: "fix [all|branding|avatar|post] SRC_PREFIX DST_PREFIX", + Short: "Fix stored URL prefixes in the database (stop Answer and back up data first)", + Long: `Fix stored URL prefixes for branding, avatars, and post content. + +WARNING: Stop Answer and back up your database before running this command. + +Rows already using DST_PREFIX are skipped so the command can be rerun safely. +If a text field contains both prefixes, it will be skipped and reported. + +After fixing branding data, the related site info cache will be invalidated.`, + Example: ` # Preview what would change + answer fix all /uploads/ /uploads/new/ --dry-run + + # Fix up to 50 rows + answer fix all /uploads/ /cdn/ --limit 50 + + # Fix only avatars + answer fix avatar /uploads/ /cdn/`, + Args: cobra.ExactArgs(3), + Run: func(_ *cobra.Command, args []string) { + path.FormatAllPath(dataDirPath) + c, err := conf.ReadConfig(path.GetConfigFilePath()) + if err != nil { + fmt.Fprintf(os.Stderr, "read config failed: %v\n", err) + os.Exit(1) + } + var cacheFilePath string + if c.Data.Cache != nil { + cacheFilePath = c.Data.Cache.FilePath + } + opts := &cli.FixURLPrefixOptions{ + FixType: args[0], + SrcPrefix: args[1], + DstPrefix: args[2], + DryRun: fixDryRun, + Limit: fixLimit, + CacheFilePath: cacheFilePath, + } + if fixDryRun { + fmt.Println("[dry-run] no changes will be written to the database") + } + if err := cli.FixURLPrefix(c.Data.Database, opts); err != nil { + fmt.Fprintf(os.Stderr, "fix failed: %v\n", err) + os.Exit(1) + } + }, + } ) // Execute adds all child commands to the root command and sets flags appropriately. diff --git a/internal/cli/fix.go b/internal/cli/fix.go new file mode 100644 index 000000000..bc2a5e0c9 --- /dev/null +++ b/internal/cli/fix.go @@ -0,0 +1,327 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cli + +import ( + "fmt" + "strings" + + "github.com/apache/answer/internal/base/data" +) + +const fixBatchSize = 100 + +const ( + fixTypeAll = "all" + fixTypeBranding = "branding" + fixTypeAvatar = "avatar" + fixTypePost = "post" +) + +// FixURLPrefixOptions holds options for the fix command. +type FixURLPrefixOptions struct { + // FixType is one of: all, branding, avatar, post + FixType string + SrcPrefix string + DstPrefix string + DryRun bool + // Limit is the max total number of rows fixed across all selected types. + // 0 means unlimited. + Limit int64 + // CacheFilePath is the path to the file-backed cache. + // If non-empty, affected cache entries are cleared after DB writes. + CacheFilePath string +} + +type fixResult struct { + Affected int64 + Fixed int64 + Skipped int64 +} + +func (r *fixResult) add(other fixResult) { + r.Affected += other.Affected + r.Fixed += other.Fixed + r.Skipped += other.Skipped +} + +func FixURLPrefix(dbConf *data.Database, opts *FixURLPrefixOptions) error { + if opts == nil { + return fmt.Errorf("fix options is nil") + } + if len(opts.SrcPrefix) == 0 { + return fmt.Errorf("SRC_PREFIX must not be empty") + } + if len(opts.DstPrefix) == 0 { + return fmt.Errorf("DST_PREFIX must not be empty") + } + if opts.SrcPrefix == opts.DstPrefix { + return fmt.Errorf("SRC_PREFIX and DST_PREFIX must be different") + } + + db, err := data.NewDB(false, dbConf) + if err != nil { + return fmt.Errorf("connect database failed: %w", err) + } + defer func() { + _ = db.Close() + }() + + fixTypes := resolveFixTypes(opts.FixType) + if len(fixTypes) == 0 { + return fmt.Errorf("unknown fix type %q, must be one of: %s, %s, %s, %s", opts.FixType, fixTypeAll, fixTypeBranding, fixTypeAvatar, fixTypePost) + } + + results := make(map[string]*fixResult, len(fixTypes)) + state := newFixRunState(opts.Limit) + + for _, fixType := range fixTypes { + if state.exhausted() { + break + } + + var result fixResult + switch fixType { + case fixTypeBranding: + result, err = fixBranding(db, opts, state) + case fixTypeAvatar: + result, err = fixAvatar(db, opts, state) + case fixTypePost: + result, err = fixPost(db, opts, state) + } + if err != nil { + return fmt.Errorf("fix %s failed: %w", fixType, err) + } + + results[fixType] = &result + } + + printFixSummary(fixTypes, results) + + if !opts.DryRun { + invalidateSiteInfoCache(opts.CacheFilePath, fixTypes) + } + return nil +} + +func resolveFixTypes(fixType string) []string { + switch fixType { + case fixTypeAll: + return []string{fixTypeBranding, fixTypeAvatar, fixTypePost} + case fixTypeBranding, fixTypeAvatar, fixTypePost: + return []string{fixType} + default: + return nil + } +} + +func printFixSummary(fixTypes []string, results map[string]*fixResult) { + fmt.Println() + fmt.Printf("%-12s %10s %10s %10s\n", "TYPE", "AFFECTED", "FIXED", "SKIPPED") + fmt.Println(strings.Repeat("-", 46)) + + var total fixResult + for _, fixType := range fixTypes { + result := results[fixType] + if result == nil { + continue + } + fmt.Printf("%-12s %10d %10d %10d\n", fixType, result.Affected, result.Fixed, result.Skipped) + total.add(*result) + } + + fmt.Println(strings.Repeat("-", 46)) + fmt.Printf("%-12s %10d %10d %10d\n", "TOTAL", total.Affected, total.Fixed, total.Skipped) +} + +// URL context markers used to anchor prefix matching/replacement to actual URL +// positions in text, preventing accidental rewrites of plain text content. +var ( + // textURLMarkers precede URLs in raw markdown [text](URL) and HTML attr="URL" + textURLMarkers = []string{"](", `="`} + // jsonURLMarkers precede URLs in JSON-encoded text (revision content stores + // the post entity as JSON, so HTML quotes are escaped as \") + jsonURLMarkers = []string{"](", `=\"`} +) + +// containsURLPrefix reports whether text contains marker+prefix for any marker. +func containsURLPrefix(text, prefix string, markers []string) bool { + for _, m := range markers { + if strings.Contains(text, m+prefix) { + return true + } + } + return false +} + +// prefixMatch classifies how stored data relates to the src/dst prefixes. +type prefixMatch int + +const ( + matchNone prefixMatch = iota // neither prefix present → nothing to do + matchFix // src prefix present → rewrite to dst + matchSkip // already on dst prefix → skip + matchBoth // both prefixes present → ambiguous, manual fix +) + +// replaceURLPrefix replaces marker+src with marker+dst for each marker. +func replaceURLPrefix(text, src, dst string, markers []string) string { + for _, m := range markers { + text = strings.ReplaceAll(text, m+src, m+dst) + } + return text +} + +// containsPrefixNotShadowed reports whether `prefix` occurs as a URL prefix +// that is not merely the head of the longer `shadow` prefix. Example: with +// prefix="/uploads/" and shadow="/uploads/new/", a URL "/uploads/new/x.png" +// does NOT count, but a standalone "/uploads/x.png" does. +func containsPrefixNotShadowed(text, prefix, shadow string, markers []string) bool { + if !containsURLPrefix(text, prefix, markers) { + return false + } + if len(shadow) > len(prefix) && strings.HasPrefix(shadow, prefix) { + scrubbed := text + for _, m := range markers { + scrubbed = strings.ReplaceAll(scrubbed, m+shadow, "") + } + return containsURLPrefix(scrubbed, prefix, markers) + } + return true +} + +func anyContainsPrefixNotShadowed(texts []string, prefix, shadow string, markers []string) bool { + for _, t := range texts { + if containsPrefixNotShadowed(t, prefix, shadow, markers) { + return true + } + } + return false +} + +// classifyValue classifies a stored value whose entire content is the URL, so +// prefixes are matched at the start. It never returns matchBoth. +func classifyValue(val, src, dst string) prefixMatch { + matchesSrc := strings.HasPrefix(val, src) + matchesDst := strings.HasPrefix(val, dst) + // When both match, prefer the longer prefix so a value already on dst is + // skipped even if dst and src share a common head. + if matchesDst && (!matchesSrc || len(dst) > len(src)) { + return matchSkip + } + if matchesSrc { + return matchFix + } + return matchNone +} + +// classifyText classifies free text that may embed URLs at marker positions. +func classifyText(texts []string, src, dst string, markers []string) prefixMatch { + hasSrc := anyContainsPrefixNotShadowed(texts, src, dst, markers) + hasDst := anyContainsPrefixNotShadowed(texts, dst, src, markers) + switch { + case hasSrc && hasDst: + return matchBoth + case hasDst: + return matchSkip + case hasSrc: + return matchFix + default: + return matchNone + } +} + +// fixRunState centralizes limit handling for all fix sub-routines. +type fixRunState struct { + limit int64 + remaining int64 +} + +func newFixRunState(limit int64) *fixRunState { + return &fixRunState{limit: limit, remaining: limit} +} + +func (s *fixRunState) exhausted() bool { + return s.limit > 0 && s.remaining <= 0 +} + +func (s *fixRunState) markAffected(result *fixResult) { + result.Affected++ + if s.limit > 0 { + s.remaining-- + } +} + +func applyValueFix(result *fixResult, opts *FixURLPrefixOptions, state *fixRunState, + label, target, current string, update func(newValue string) error, +) error { + switch classifyValue(current, opts.SrcPrefix, opts.DstPrefix) { + case matchSkip: + fmt.Printf("[%s] skip %s: already has DST_PREFIX: %s\n", label, target, current) + result.Skipped++ + return nil + case matchNone: + return nil + } + + newValue := opts.DstPrefix + strings.TrimPrefix(current, opts.SrcPrefix) + state.markAffected(result) + fmt.Printf("[%s] %s: %s -> %s\n", label, target, current, newValue) + + if opts.DryRun { + return nil + } + if err := update(newValue); err != nil { + return err + } + result.Fixed++ + return nil +} + +func runBatchedFix[T any](state *fixRunState, fetch func(offset int) ([]T, error), process func(item T) error) error { + offset := 0 + for { + if state.exhausted() { + return nil + } + + items, err := fetch(offset) + if err != nil { + return err + } + if len(items) == 0 { + return nil + } + + for _, item := range items { + if state.exhausted() { + return nil + } + if err := process(item); err != nil { + return err + } + } + + if len(items) < fixBatchSize { + return nil + } + offset += fixBatchSize + } +} diff --git a/internal/cli/fix_avatar.go b/internal/cli/fix_avatar.go new file mode 100644 index 000000000..74de49dc2 --- /dev/null +++ b/internal/cli/fix_avatar.go @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cli + +import ( + "encoding/json" + "fmt" + + "github.com/apache/answer/internal/base/constant" + "github.com/apache/answer/internal/entity" + "github.com/apache/answer/plugin" + "xorm.io/xorm" +) + +// avatarData is the JSON structure stored in the user.avatar column. +type avatarData struct { + Type string `json:"type"` + Gravatar string `json:"gravatar,omitempty"` + Custom string `json:"custom,omitempty"` +} + +func fixAvatar(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState) (fixResult, error) { + result := fixResult{} + + err := runBatchedFix(state, + func(offset int) ([]*entity.User, error) { return queryCustomAvatarUsers(x, offset) }, + func(user *entity.User) error { return processAvatarUser(x, &result, opts, state, user) }) + if err != nil { + return fixResult{}, err + } + + frResult, err := fixFileRecordBySources(x, opts, state, + []string{string(plugin.UserAvatar)}, fixTypeAvatar) + if err != nil { + return fixResult{}, err + } + result.add(frResult) + + return result, nil +} + +func queryCustomAvatarUsers(x *xorm.Engine, offset int) ([]*entity.User, error) { + users := make([]*entity.User, 0) + err := x.Select("id, avatar"). + Where("avatar LIKE ?", `%"custom":"%`). + OrderBy("id"). + Limit(fixBatchSize, offset). + Find(&users) + if err != nil { + return nil, fmt.Errorf("query user failed: %w", err) + } + return users, nil +} + +func processAvatarUser(x *xorm.Engine, result *fixResult, opts *FixURLPrefixOptions, state *fixRunState, user *entity.User) error { + avatar := &avatarData{} + if err := json.Unmarshal([]byte(user.Avatar), avatar); err != nil { + return nil + } + if avatar.Type != constant.AvatarTypeCustom || len(avatar.Custom) == 0 { + return nil + } + return applyValueFix(result, opts, state, fixTypeAvatar, fmt.Sprintf("user %s", user.ID), avatar.Custom, + func(newCustom string) error { + avatar.Custom = newCustom + newJSON, err := json.Marshal(avatar) + if err != nil { + return fmt.Errorf("marshal avatar JSON (user=%s) failed: %w", user.ID, err) + } + if _, err := x.ID(user.ID).Cols("avatar").NoAutoTime().Update(&entity.User{Avatar: string(newJSON)}); err != nil { + return fmt.Errorf("update user avatar (id=%s) failed: %w", user.ID, err) + } + return nil + }) +} diff --git a/internal/cli/fix_branding.go b/internal/cli/fix_branding.go new file mode 100644 index 000000000..937fdc836 --- /dev/null +++ b/internal/cli/fix_branding.go @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cli + +import ( + "encoding/json" + "fmt" + + "github.com/apache/answer/internal/base/constant" + "github.com/apache/answer/internal/entity" + "github.com/apache/answer/plugin" + "xorm.io/xorm" +) + +func fixBranding(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState) (fixResult, error) { + siteInfo := &entity.SiteInfo{Type: constant.SiteTypeBranding} + exist, err := x.Get(siteInfo) + if err != nil { + return fixResult{}, fmt.Errorf("get site info failed: %w", err) + } + if !exist { + fmt.Printf("[%s] no branding row found, skipping\n", fixTypeBranding) + return fixResult{}, nil + } + + type brandingContent struct { + Logo string `json:"logo"` + MobileLogo string `json:"mobile_logo"` + SquareIcon string `json:"square_icon"` + Favicon string `json:"favicon"` + } + + content := &brandingContent{} + if err = json.Unmarshal([]byte(siteInfo.Content), content); err != nil { + return fixResult{}, fmt.Errorf("parse branding content failed: %w", err) + } + + type brandingField struct { + name string + value *string + } + + fields := []brandingField{ + {name: "logo", value: &content.Logo}, + {name: "mobile_logo", value: &content.MobileLogo}, + {name: "square_icon", value: &content.SquareIcon}, + {name: "favicon", value: &content.Favicon}, + } + + result := fixResult{} + changed := false + + for _, field := range fields { + if state.exhausted() { + break + } + if len(*field.value) == 0 { + continue + } + if err := applyValueFix(&result, opts, state, fixTypeBranding, field.name, *field.value, func(newValue string) error { + *field.value = newValue + changed = true + return nil + }); err != nil { + return fixResult{}, err + } + } + + if changed { + newContent, err := json.Marshal(content) + if err != nil { + return fixResult{}, fmt.Errorf("marshal branding content failed: %w", err) + } + _, err = x.ID(siteInfo.ID).Cols("content").NoAutoTime().Update(&entity.SiteInfo{Content: string(newContent)}) + if err != nil { + return fixResult{}, fmt.Errorf("update site info failed: %w", err) + } + } + + frResult, err := fixFileRecordBySources(x, opts, state, + []string{string(plugin.AdminBranding)}, fixTypeBranding) + if err != nil { + return fixResult{}, err + } + result.add(frResult) + + return result, nil +} diff --git a/internal/cli/fix_cache.go b/internal/cli/fix_cache.go new file mode 100644 index 000000000..d0a646ca7 --- /dev/null +++ b/internal/cli/fix_cache.go @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cli + +import ( + "context" + "fmt" + "slices" + + "github.com/apache/answer/internal/base/constant" + "github.com/segmentfault/pacman/contrib/cache/memory" +) + +func invalidateSiteInfoCache(cacheFilePath string, fixTypes []string) { + if !slices.Contains(fixTypes, fixTypeBranding) { + return + } + + // The branding site-info cache only lives in the file-backed memory cache. + // When a cache plugin (e.g. Redis) is used, no file path is configured, so + // warn the admin to flush it manually instead of silently doing nothing. + if len(cacheFilePath) == 0 { + fmt.Println("[cache] no file cache configured; flush your cache backend or restart the application to refresh branding") + return + } + + memCache := memory.NewCache() + if err := memory.Load(memCache, cacheFilePath); err != nil { + fmt.Printf("[cache] cannot load cache file %s: %v (skipping)\n", cacheFilePath, err) + return + } + + key := constant.SiteInfoCacheKey + constant.SiteTypeBranding + if err := memCache.Del(context.Background(), key); err != nil { + fmt.Printf("[cache] failed to delete key %s: %v\n", key, err) + } else { + fmt.Printf("[cache] invalidated %s\n", key) + } + + if err := memory.Save(memCache, cacheFilePath); err != nil { + fmt.Printf("[cache] failed to save cache file: %v\n", err) + return + } + + fmt.Println("[cache] cache file updated, restart the application to take full effect") +} diff --git a/internal/cli/fix_filerecord.go b/internal/cli/fix_filerecord.go new file mode 100644 index 000000000..439f81fe4 --- /dev/null +++ b/internal/cli/fix_filerecord.go @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cli + +import ( + "fmt" + + "github.com/apache/answer/internal/entity" + "xorm.io/xorm" +) + +func fixFileRecordBySources(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState, sources []string, label string) (fixResult, error) { + result := fixResult{} + + err := runBatchedFix(state, func(offset int) ([]*entity.FileRecord, error) { + records := make([]*entity.FileRecord, 0) + err := x.Select("id, file_url"). + In("source", sources). + OrderBy("id"). + Limit(fixBatchSize, offset). + Find(&records) + if err != nil { + return nil, fmt.Errorf("query %s file_record failed: %w", label, err) + } + return records, nil + }, func(record *entity.FileRecord) error { + return applyValueFix(&result, opts, state, label, fmt.Sprintf("file_record id=%d", record.ID), record.FileURL, func(newURL string) error { + _, err := x.ID(record.ID).Cols("file_url").NoAutoTime().Update(&entity.FileRecord{FileURL: newURL}) + if err != nil { + return fmt.Errorf("update %s file_record (id=%d) failed: %w", label, record.ID, err) + } + return nil + }) + }) + if err != nil { + return fixResult{}, err + } + + return result, nil +} diff --git a/internal/cli/fix_post.go b/internal/cli/fix_post.go new file mode 100644 index 000000000..c743be462 --- /dev/null +++ b/internal/cli/fix_post.go @@ -0,0 +1,188 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cli + +import ( + "fmt" + + "github.com/apache/answer/internal/base/constant" + "github.com/apache/answer/internal/entity" + "github.com/apache/answer/plugin" + "xorm.io/xorm" +) + +func fixPost(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState) (fixResult, error) { + result := fixResult{} + + // Fix question and answer body text. + for _, tableName := range []string{"question", "answer"} { + r, err := fixPostTable(x, opts, state, tableName) + if err != nil { + return fixResult{}, err + } + result.add(r) + } + + // Fix revisions for questions and answers. + for _, objectType := range []int{ + constant.ObjectTypeStrMapping[constant.QuestionObjectType], + constant.ObjectTypeStrMapping[constant.AnswerObjectType], + } { + r, err := fixRevisionByObjectType(x, opts, state, objectType, fixTypePost) + if err != nil { + return fixResult{}, err + } + result.add(r) + } + + // Fix file records. + fileResult, err := fixFileRecordBySources(x, opts, state, + []string{string(plugin.UserPost), string(plugin.UserPostAttachment)}, fixTypePost) + if err != nil { + return fixResult{}, err + } + result.add(fileResult) + + return result, nil +} + +// postRow represents a question or answer row with its text fields. +type postRow struct { + ID string `xorm:"id"` + OriginalText string `xorm:"original_text"` + ParsedText string `xorm:"parsed_text"` +} + +func fixPostTable(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState, tableName string) (fixResult, error) { + result := fixResult{} + err := runBatchedFix(state, + func(offset int) ([]*postRow, error) { return queryPostRows(x, tableName, offset) }, + func(row *postRow) error { + return applyTextFix(&result, opts, state, fixTypePost, fmt.Sprintf("%s id=%s", tableName, row.ID), + []string{row.OriginalText, row.ParsedText}, textURLMarkers, + func() error { return updatePostText(x, tableName, row, opts) }) + }) + if err != nil { + return fixResult{}, err + } + return result, nil +} + +func queryPostRows(x *xorm.Engine, tableName string, offset int) ([]*postRow, error) { + rows := make([]*postRow, 0) + err := x.Table(tableName).Select("id, original_text, parsed_text"). + OrderBy("id"). + Limit(fixBatchSize, offset). + Find(&rows) + if err != nil { + return nil, fmt.Errorf("query %s failed: %w", tableName, err) + } + return rows, nil +} + +func updatePostText(x *xorm.Engine, tableName string, row *postRow, opts *FixURLPrefixOptions) error { + origText := replaceURLPrefix(row.OriginalText, opts.SrcPrefix, opts.DstPrefix, textURLMarkers) + parsedText := replaceURLPrefix(row.ParsedText, opts.SrcPrefix, opts.DstPrefix, textURLMarkers) + + var bean interface{} + switch tableName { + case "question": + bean = &entity.Question{OriginalText: origText, ParsedText: parsedText} + case "answer": + bean = &entity.Answer{OriginalText: origText, ParsedText: parsedText} + default: + return fmt.Errorf("unknown post table %q", tableName) + } + + if _, err := x.ID(row.ID).Cols("original_text", "parsed_text").NoAutoTime().Update(bean); err != nil { + return fmt.Errorf("update %s (id=%s) failed: %w", tableName, row.ID, err) + } + return nil +} + +func fixRevisionByObjectType(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState, objectType int, label string) (fixResult, error) { + result := fixResult{} + err := runBatchedFix(state, + func(offset int) ([]*entity.Revision, error) { + return queryRevisionsByObjectType(x, objectType, label, offset) + }, + func(revision *entity.Revision) error { + return applyTextFix(&result, opts, state, label, fmt.Sprintf("revision id=%s", revision.ID), + []string{revision.Content}, jsonURLMarkers, + func() error { return updateRevisionContent(x, revision, opts, label) }) + }) + if err != nil { + return fixResult{}, err + } + return result, nil +} + +func queryRevisionsByObjectType(x *xorm.Engine, objectType int, label string, offset int) ([]*entity.Revision, error) { + revisions := make([]*entity.Revision, 0) + err := x.Select("id, content"). + Where("object_type = ?", objectType). + OrderBy("id"). + Limit(fixBatchSize, offset). + Find(&revisions) + if err != nil { + return nil, fmt.Errorf("query %s revision failed: %w", label, err) + } + return revisions, nil +} + +func updateRevisionContent(x *xorm.Engine, revision *entity.Revision, opts *FixURLPrefixOptions, label string) error { + updated := &entity.Revision{ + Content: replaceURLPrefix(revision.Content, opts.SrcPrefix, opts.DstPrefix, jsonURLMarkers), + } + _, err := x.ID(revision.ID).Cols("content").NoAutoTime().Update(updated) + if err != nil { + return fmt.Errorf("update %s revision (id=%s) failed: %w", label, revision.ID, err) + } + return nil +} + +func applyTextFix(result *fixResult, opts *FixURLPrefixOptions, state *fixRunState, + label, target string, texts []string, markers []string, update func() error, +) error { + switch classifyText(texts, opts.SrcPrefix, opts.DstPrefix, markers) { + case matchBoth: + fmt.Printf("[%s] WARN %s: has BOTH prefixes, skipping (manual fix needed)\n", label, target) + result.Skipped++ + return nil + case matchSkip: + fmt.Printf("[%s] skip %s: already has DST_PREFIX\n", label, target) + result.Skipped++ + return nil + case matchNone: + return nil + } + + state.markAffected(result) + fmt.Printf("[%s] %s: replacing %q with %q\n", label, target, opts.SrcPrefix, opts.DstPrefix) + + if opts.DryRun { + return nil + } + if err := update(); err != nil { + return err + } + result.Fixed++ + return nil +} From ac543242ec690f2b02321f349248e3f650beafce Mon Sep 17 00:00:00 2001 From: Shaobiao Lin Date: Sat, 17 Oct 2026 18:57:40 +0800 Subject: [PATCH 2/2] fix(cli): make URL prefix fix safe for partial and repeated runs Fix each post with its revisions in one transaction, classify text fields independently, cover comments and notification avatars, keep file records in sync, page by id, and report invalid or ambiguous data instead of skipping it. Co-Authored-By: Claude Opus 5.5 --- cmd/command.go | 29 +- internal/cli/fix.go | 183 ++++++--- internal/cli/fix_avatar.go | 97 +++-- internal/cli/fix_branding.go | 25 +- internal/cli/fix_cache.go | 15 +- internal/cli/fix_filerecord.go | 143 +++++++- internal/cli/fix_post.go | 267 +++++++++----- internal/cli/fix_test.go | 651 +++++++++++++++++++++++++++++++++ 8 files changed, 1198 insertions(+), 212 deletions(-) create mode 100644 internal/cli/fix_test.go diff --git a/cmd/command.go b/cmd/command.go index 00470efea..47a82b8c5 100644 --- a/cmd/command.go +++ b/cmd/command.go @@ -90,7 +90,7 @@ func init() { resetPasswordCmd.Flags().StringVarP(&resetPasswordPassword, "password", "p", "", "new password (not recommended, will be recorded in shell history)") fixCmd.Flags().BoolVar(&fixDryRun, "dry-run", false, "show what would be changed without modifying the database") - fixCmd.Flags().Int64Var(&fixLimit, "limit", 0, "stop after fixing NUM rows total across all selected fix types (0 = unlimited)") + fixCmd.Flags().Int64Var(&fixLimit, "limit", 0, "stop after fixing NUM rows total across all selected fix types; a post and its revisions count as one row, file records are not counted (default: unlimited)") for _, cmd := range []*cobra.Command{initCmd, checkCmd, runCmd, dumpCmd, upgradeCmd, buildCmd, pluginCmd, configCmd, i18nCmd, resetPasswordCmd, fixCmd} { rootCmd.AddCommand(cmd) @@ -343,24 +343,31 @@ To run answer, use: fixCmd = &cobra.Command{ Use: "fix [all|branding|avatar|post] SRC_PREFIX DST_PREFIX", Short: "Fix stored URL prefixes in the database (stop Answer and back up data first)", - Long: `Fix stored URL prefixes for branding, avatars, and post content. + Long: `Fix stored URL prefixes of uploaded files: + branding logo, mobile logo, square icon and favicon + avatar custom avatars, including copies in notifications + post questions, answers, tags (with revisions) and comments -WARNING: Stop Answer and back up your database before running this command. +WARNING: Stop Answer and back up your database first. Values already on DST_PREFIX +are skipped; ambiguous ones are reported for manual fix. -Rows already using DST_PREFIX are skipped so the command can be rerun safely. -If a text field contains both prefixes, it will be skipped and reported. - -After fixing branding data, the related site info cache will be invalidated.`, +In text, only URLs after "](" or '="' are fixed. Not fixed: site URL, user +bios, legal pages, custom CSS/HTML, plugin settings, close messages and +plain-URL avatars from old versions.`, Example: ` # Preview what would change - answer fix all /uploads/ /uploads/new/ --dry-run + answer fix all https://old.example.com/uploads/ https://new.example.com/uploads/ --dry-run # Fix up to 50 rows - answer fix all /uploads/ /cdn/ --limit 50 + answer fix all https://old.example.com/uploads/ https://cdn.example.com/uploads/ --limit 50 # Fix only avatars - answer fix avatar /uploads/ /cdn/`, + answer fix avatar https://old.example.com/uploads/ https://cdn.example.com/uploads/`, Args: cobra.ExactArgs(3), - Run: func(_ *cobra.Command, args []string) { + Run: func(cmd *cobra.Command, args []string) { + if cmd.Flags().Changed("limit") && fixLimit <= 0 { + fmt.Fprintf(os.Stderr, "--limit must be greater than 0, got %d\n", fixLimit) + os.Exit(1) + } path.FormatAllPath(dataDirPath) c, err := conf.ReadConfig(path.GetConfigFilePath()) if err != nil { diff --git a/internal/cli/fix.go b/internal/cli/fix.go index bc2a5e0c9..5d4fc1582 100644 --- a/internal/cli/fix.go +++ b/internal/cli/fix.go @@ -21,6 +21,7 @@ package cli import ( "fmt" + "strconv" "strings" "github.com/apache/answer/internal/base/data" @@ -42,11 +43,10 @@ type FixURLPrefixOptions struct { SrcPrefix string DstPrefix string DryRun bool - // Limit is the max total number of rows fixed across all selected types. - // 0 means unlimited. + // Limit caps fixed rows across all types (a post with its revisions is one + // row; file records are not counted). 0 means unlimited. Limit int64 - // CacheFilePath is the path to the file-backed cache. - // If non-empty, affected cache entries are cleared after DB writes. + // CacheFilePath is the file cache whose branding entry is cleared, if set. CacheFilePath string } @@ -75,6 +75,17 @@ func FixURLPrefix(dbConf *data.Database, opts *FixURLPrefixOptions) error { if opts.SrcPrefix == opts.DstPrefix { return fmt.Errorf("SRC_PREFIX and DST_PREFIX must be different") } + // A slash on one side only joins path segments: /uploads/a.png -> /cdna.png. + if strings.HasSuffix(opts.SrcPrefix, "/") != strings.HasSuffix(opts.DstPrefix, "/") { + return fmt.Errorf("SRC_PREFIX and DST_PREFIX must both end with \"/\" or both not") + } + if opts.Limit < 0 { + return fmt.Errorf("limit must not be negative (0 means unlimited), got %d", opts.Limit) + } + fixTypes := resolveFixTypes(opts.FixType) + if len(fixTypes) == 0 { + return fmt.Errorf("unknown fix type %q, must be one of: %s, %s, %s, %s", opts.FixType, fixTypeAll, fixTypeBranding, fixTypeAvatar, fixTypePost) + } db, err := data.NewDB(false, dbConf) if err != nil { @@ -84,16 +95,31 @@ func FixURLPrefix(dbConf *data.Database, opts *FixURLPrefixOptions) error { _ = db.Close() }() - fixTypes := resolveFixTypes(opts.FixType) - if len(fixTypes) == 0 { - return fmt.Errorf("unknown fix type %q, must be one of: %s, %s, %s, %s", opts.FixType, fixTypeAll, fixTypeBranding, fixTypeAvatar, fixTypePost) + if !strings.HasSuffix(opts.SrcPrefix, "/") { + fmt.Printf("WARN: SRC_PREFIX does not end with \"/\", so it also matches longer paths (e.g. %q matches %q)\n", + opts.SrcPrefix, opts.SrcPrefix+"-other/x.png") + } + if strings.HasPrefix(opts.SrcPrefix, opts.DstPrefix) { + fmt.Printf("WARN: SRC_PREFIX starts with DST_PREFIX, so URLs already moved under %q cannot be told apart from old ones; do not rerun after a successful run\n", + opts.SrcPrefix) + } + if opts.DryRun && opts.Limit > 0 { + fmt.Println("NOTE: the preview counts post file records as if --limit did not apply, so a real run may fix fewer of them") } results := make(map[string]*fixResult, len(fixTypes)) + var postFiles *fixResult + var notStarted []string state := newFixRunState(opts.Limit) - for _, fixType := range fixTypes { + if !opts.DryRun { + // Deferred so it also runs when a later fix type fails. + defer invalidateSiteInfoCache(opts.CacheFilePath, fixTypes) + } + + for i, fixType := range fixTypes { if state.exhausted() { + notStarted = fixTypes[i:] break } @@ -104,7 +130,9 @@ func FixURLPrefix(dbConf *data.Database, opts *FixURLPrefixOptions) error { case fixTypeAvatar: result, err = fixAvatar(db, opts, state) case fixTypePost: - result, err = fixPost(db, opts, state) + var files fixResult + result, files, err = fixPost(db, opts, state) + postFiles = &files } if err != nil { return fmt.Errorf("fix %s failed: %w", fixType, err) @@ -113,10 +141,16 @@ func FixURLPrefix(dbConf *data.Database, opts *FixURLPrefixOptions) error { results[fixType] = &result } - printFixSummary(fixTypes, results) - - if !opts.DryRun { - invalidateSiteInfoCache(opts.CacheFilePath, fixTypes) + printFixSummary(fixTypes, results, postFiles) + if state.exhausted() { + if opts.DryRun { + fmt.Printf("\n--limit %d reached in preview; a real run would stop here too.\n", opts.Limit) + } else { + fmt.Printf("\n--limit %d reached; run the command again to fix the rest.\n", opts.Limit) + } + if len(notStarted) > 0 { + fmt.Printf("Not started: %s\n", strings.Join(notStarted, ", ")) + } } return nil } @@ -132,7 +166,8 @@ func resolveFixTypes(fixType string) []string { } } -func printFixSummary(fixTypes []string, results map[string]*fixResult) { +// printFixSummary prints the rows counted by --limit, then post file records. +func printFixSummary(fixTypes []string, results map[string]*fixResult, postFiles *fixResult) { fmt.Println() fmt.Printf("%-12s %10s %10s %10s\n", "TYPE", "AFFECTED", "FIXED", "SKIPPED") fmt.Println(strings.Repeat("-", 46)) @@ -149,15 +184,19 @@ func printFixSummary(fixTypes []string, results map[string]*fixResult) { fmt.Println(strings.Repeat("-", 46)) fmt.Printf("%-12s %10d %10d %10d\n", "TOTAL", total.Affected, total.Fixed, total.Skipped) + + if postFiles != nil { + fmt.Println() + fmt.Println("File records (not counted by --limit):") + fmt.Printf("%-12s %10d %10d %10d\n", fixTypePost, postFiles.Affected, postFiles.Fixed, postFiles.Skipped) + } } -// URL context markers used to anchor prefix matching/replacement to actual URL -// positions in text, preventing accidental rewrites of plain text content. +// URL markers anchor prefix matching to URL positions, so plain text is kept. var ( - // textURLMarkers precede URLs in raw markdown [text](URL) and HTML attr="URL" + // textURLMarkers precede URLs in markdown [text](URL) and HTML attr="URL". textURLMarkers = []string{"](", `="`} - // jsonURLMarkers precede URLs in JSON-encoded text (revision content stores - // the post entity as JSON, so HTML quotes are escaped as \") + // jsonURLMarkers are the same in JSON-encoded revision content. jsonURLMarkers = []string{"](", `=\"`} ) @@ -171,14 +210,14 @@ func containsURLPrefix(text, prefix string, markers []string) bool { return false } -// prefixMatch classifies how stored data relates to the src/dst prefixes. +// prefixMatch is how stored data relates to the src/dst prefixes. type prefixMatch int const ( - matchNone prefixMatch = iota // neither prefix present → nothing to do - matchFix // src prefix present → rewrite to dst - matchSkip // already on dst prefix → skip - matchBoth // both prefixes present → ambiguous, manual fix + matchNone prefixMatch = iota // neither prefix + matchFix // has src: rewrite + matchSkip // only dst: skip + matchBoth // ambiguous: manual fix ) // replaceURLPrefix replaces marker+src with marker+dst for each marker. @@ -189,10 +228,14 @@ func replaceURLPrefix(text, src, dst string, markers []string) string { return text } -// containsPrefixNotShadowed reports whether `prefix` occurs as a URL prefix -// that is not merely the head of the longer `shadow` prefix. Example: with -// prefix="/uploads/" and shadow="/uploads/new/", a URL "/uploads/new/x.png" -// does NOT count, but a standalone "/uploads/x.png" does. +// swapPrefix replaces the leading from prefix of url with to. +func swapPrefix(url, from, to string) string { + return to + strings.TrimPrefix(url, from) +} + +// containsPrefixNotShadowed reports whether prefix occurs other than as the +// head of a longer shadow: for /uploads/ shadowed by /uploads/new/, +// /uploads/x.png counts but /uploads/new/x.png does not. func containsPrefixNotShadowed(text, prefix, shadow string, markers []string) bool { if !containsURLPrefix(text, prefix, markers) { return false @@ -207,22 +250,12 @@ func containsPrefixNotShadowed(text, prefix, shadow string, markers []string) bo return true } -func anyContainsPrefixNotShadowed(texts []string, prefix, shadow string, markers []string) bool { - for _, t := range texts { - if containsPrefixNotShadowed(t, prefix, shadow, markers) { - return true - } - } - return false -} - -// classifyValue classifies a stored value whose entire content is the URL, so -// prefixes are matched at the start. It never returns matchBoth. +// classifyValue classifies a value that is a whole URL. It never returns +// matchBoth. func classifyValue(val, src, dst string) prefixMatch { matchesSrc := strings.HasPrefix(val, src) matchesDst := strings.HasPrefix(val, dst) - // When both match, prefer the longer prefix so a value already on dst is - // skipped even if dst and src share a common head. + // When both match, the longer prefix wins. if matchesDst && (!matchesSrc || len(dst) > len(src)) { return matchSkip } @@ -232,13 +265,18 @@ func classifyValue(val, src, dst string) prefixMatch { return matchNone } -// classifyText classifies free text that may embed URLs at marker positions. -func classifyText(texts []string, src, dst string, markers []string) prefixMatch { - hasSrc := anyContainsPrefixNotShadowed(texts, src, dst, markers) - hasDst := anyContainsPrefixNotShadowed(texts, dst, src, markers) +// classifyText classifies text embedding URLs at marker positions. Text with +// both prefixes is ambiguous only if dst starts with src, as replacing src +// would then nest dst URLs (/uploads/cdn/ -> /uploads/cdn/cdn/). +func classifyText(text, src, dst string, markers []string) prefixMatch { + hasSrc := containsPrefixNotShadowed(text, src, dst, markers) + hasDst := containsPrefixNotShadowed(text, dst, src, markers) switch { case hasSrc && hasDst: - return matchBoth + if strings.HasPrefix(dst, src) { + return matchBoth + } + return matchFix case hasDst: return matchSkip case hasSrc: @@ -248,7 +286,26 @@ func classifyText(texts []string, src, dst string, markers []string) prefixMatch } } -// fixRunState centralizes limit handling for all fix sub-routines. +// classifyFields combines per-field results: any ambiguous field wins, then +// any fixable one. Callers rewrite only matchFix fields (see fixTextField). +func classifyFields(texts []string, src, dst string, markers []string) prefixMatch { + result := matchNone + for _, text := range texts { + switch classifyText(text, src, dst, markers) { + case matchBoth: + return matchBoth + case matchFix: + result = matchFix + case matchSkip: + if result == matchNone { + result = matchSkip + } + } + } + return result +} + +// fixRunState tracks --limit across all fix types. type fixRunState struct { limit int64 remaining int64 @@ -281,7 +338,7 @@ func applyValueFix(result *fixResult, opts *FixURLPrefixOptions, state *fixRunSt return nil } - newValue := opts.DstPrefix + strings.TrimPrefix(current, opts.SrcPrefix) + newValue := swapPrefix(current, opts.SrcPrefix, opts.DstPrefix) state.markAffected(result) fmt.Printf("[%s] %s: %s -> %s\n", label, target, current, newValue) @@ -295,20 +352,19 @@ func applyValueFix(result *fixResult, opts *FixURLPrefixOptions, state *fixRunSt return nil } -func runBatchedFix[T any](state *fixRunState, fetch func(offset int) ([]T, error), process func(item T) error) error { - offset := 0 +// runBatchedFix pages by id rather than OFFSET. fetch returns rows with id > +// afterID in id order, and the last row's id. +func runBatchedFix[T any](state *fixRunState, fetch func(afterID int64) (items []T, lastID int64, err error), process func(item T) error) error { + var afterID int64 for { if state.exhausted() { return nil } - items, err := fetch(offset) + items, lastID, err := fetch(afterID) if err != nil { return err } - if len(items) == 0 { - return nil - } for _, item := range items { if state.exhausted() { @@ -322,6 +378,23 @@ func runBatchedFix[T any](state *fixRunState, fetch func(offset int) ([]T, error if len(items) < fixBatchSize { return nil } - offset += fixBatchSize + afterID = lastID + } +} + +// parseID parses a BIGINT id held as a string, for numeric SQL comparison. +func parseID(id string) (int64, error) { + n, err := strconv.ParseInt(id, 10, 64) + if err != nil { + return 0, fmt.Errorf("parse id %q failed: %w", id, err) + } + return n, nil +} + +// lastStringID returns the last item's parsed id, or 0 if there is none. +func lastStringID[T any](items []T, id func(T) string) (int64, error) { + if len(items) == 0 { + return 0, nil } + return parseID(id(items[len(items)-1])) } diff --git a/internal/cli/fix_avatar.go b/internal/cli/fix_avatar.go index 74de49dc2..ceb03fc9b 100644 --- a/internal/cli/fix_avatar.go +++ b/internal/cli/fix_avatar.go @@ -23,61 +23,60 @@ import ( "encoding/json" "fmt" - "github.com/apache/answer/internal/base/constant" "github.com/apache/answer/internal/entity" + "github.com/apache/answer/internal/schema" "github.com/apache/answer/plugin" "xorm.io/xorm" ) -// avatarData is the JSON structure stored in the user.avatar column. -type avatarData struct { - Type string `json:"type"` - Gravatar string `json:"gravatar,omitempty"` - Custom string `json:"custom,omitempty"` -} - func fixAvatar(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState) (fixResult, error) { result := fixResult{} - - err := runBatchedFix(state, - func(offset int) ([]*entity.User, error) { return queryCustomAvatarUsers(x, offset) }, - func(user *entity.User) error { return processAvatarUser(x, &result, opts, state, user) }) + records, err := newFileRecordSync(x, opts, fixTypeAvatar, string(plugin.UserAvatar)) if err != nil { return fixResult{}, err } - frResult, err := fixFileRecordBySources(x, opts, state, - []string{string(plugin.UserAvatar)}, fixTypeAvatar) + err = runBatchedFix(state, + func(afterID int64) ([]*entity.User, int64, error) { return queryCustomAvatarUsers(x, afterID) }, + func(user *entity.User) error { return processAvatarUser(x, &result, opts, state, records, user) }) if err != nil { return fixResult{}, err } - result.add(frResult) + if err := fixNotificationAvatars(x, &result, opts, state); err != nil { + return fixResult{}, err + } return result, nil } -func queryCustomAvatarUsers(x *xorm.Engine, offset int) ([]*entity.User, error) { +func queryCustomAvatarUsers(x *xorm.Engine, afterID int64) ([]*entity.User, int64, error) { users := make([]*entity.User, 0) err := x.Select("id, avatar"). - Where("avatar LIKE ?", `%"custom":"%`). + Where("id > ?", afterID). + And("avatar LIKE ?", `%"custom":"%`). OrderBy("id"). - Limit(fixBatchSize, offset). + Limit(fixBatchSize). Find(&users) if err != nil { - return nil, fmt.Errorf("query user failed: %w", err) + return nil, 0, fmt.Errorf("query user failed: %w", err) } - return users, nil + lastID, err := lastStringID(users, func(u *entity.User) string { return u.ID }) + return users, lastID, err } -func processAvatarUser(x *xorm.Engine, result *fixResult, opts *FixURLPrefixOptions, state *fixRunState, user *entity.User) error { - avatar := &avatarData{} +func processAvatarUser(x *xorm.Engine, result *fixResult, opts *FixURLPrefixOptions, state *fixRunState, records *fileRecordSync, user *entity.User) error { + avatar := &schema.AvatarInfo{} if err := json.Unmarshal([]byte(user.Avatar), avatar); err != nil { + fmt.Printf("[%s] WARN user %s: invalid avatar JSON, skipping (manual fix needed): %v\n", fixTypeAvatar, user.ID, err) + result.Skipped++ return nil } - if avatar.Type != constant.AvatarTypeCustom || len(avatar.Custom) == 0 { + // Fixed whatever the current type: the UI keeps custom after switching to + // another type, and switching back would bring the old URL back. + if len(avatar.Custom) == 0 { return nil } - return applyValueFix(result, opts, state, fixTypeAvatar, fmt.Sprintf("user %s", user.ID), avatar.Custom, + err := applyValueFix(result, opts, state, fixTypeAvatar, fmt.Sprintf("user %s", user.ID), avatar.Custom, func(newCustom string) error { avatar.Custom = newCustom newJSON, err := json.Marshal(avatar) @@ -89,4 +88,54 @@ func processAvatarUser(x *xorm.Engine, result *fixResult, opts *FixURLPrefixOpti } return nil }) + if err != nil { + return err + } + return records.sync(avatar.Custom) +} + +// fixNotificationAvatars fixes the sender avatar URL that a notification +// copies into its content when it is created and shows as is afterwards. +// Notifications not on SRC_PREFIX are left out silently, as there can be many. +func fixNotificationAvatars(x *xorm.Engine, result *fixResult, opts *FixURLPrefixOptions, state *fixRunState) error { + return runBatchedFix(state, + func(afterID int64) ([]*entity.Notification, int64, error) { + notifications := make([]*entity.Notification, 0) + err := x.Select("id, content"). + Where("id > ?", afterID). + And("content LIKE ?", `%"avatar":"`+opts.SrcPrefix+`%`). + OrderBy("id"). + Limit(fixBatchSize). + Find(¬ifications) + if err != nil { + return nil, 0, fmt.Errorf("query notification failed: %w", err) + } + lastID, err := lastStringID(notifications, func(n *entity.Notification) string { return n.ID }) + return notifications, lastID, err + }, + func(n *entity.Notification) error { + target := fmt.Sprintf("notification %s", n.ID) + content := &schema.NotificationContent{} + if err := json.Unmarshal([]byte(n.Content), content); err != nil { + fmt.Printf("[%s] WARN %s: invalid content JSON, skipping (manual fix needed): %v\n", fixTypeAvatar, target, err) + result.Skipped++ + return nil + } + if content.UserInfo == nil || classifyValue(content.UserInfo.Avatar, opts.SrcPrefix, opts.DstPrefix) != matchFix { + return nil + } + return applyValueFix(result, opts, state, fixTypeAvatar, target, content.UserInfo.Avatar, + func(newURL string) error { + content.UserInfo.Avatar = newURL + newJSON, err := json.Marshal(content) + if err != nil { + return fmt.Errorf("marshal notification content (id=%s) failed: %w", n.ID, err) + } + _, err = x.ID(n.ID).Cols("content").NoAutoTime().Update(&entity.Notification{Content: string(newJSON)}) + if err != nil { + return fmt.Errorf("update notification content (id=%s) failed: %w", n.ID, err) + } + return nil + }) + }) } diff --git a/internal/cli/fix_branding.go b/internal/cli/fix_branding.go index 937fdc836..11be4c075 100644 --- a/internal/cli/fix_branding.go +++ b/internal/cli/fix_branding.go @@ -22,9 +22,11 @@ package cli import ( "encoding/json" "fmt" + "slices" "github.com/apache/answer/internal/base/constant" "github.com/apache/answer/internal/entity" + "github.com/apache/answer/internal/schema" "github.com/apache/answer/plugin" "xorm.io/xorm" ) @@ -40,14 +42,7 @@ func fixBranding(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState) return fixResult{}, nil } - type brandingContent struct { - Logo string `json:"logo"` - MobileLogo string `json:"mobile_logo"` - SquareIcon string `json:"square_icon"` - Favicon string `json:"favicon"` - } - - content := &brandingContent{} + content := &schema.SiteBrandingReq{} if err = json.Unmarshal([]byte(siteInfo.Content), content); err != nil { return fixResult{}, fmt.Errorf("parse branding content failed: %w", err) } @@ -94,12 +89,20 @@ func fixBranding(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState) } } - frResult, err := fixFileRecordBySources(x, opts, state, - []string{string(plugin.AdminBranding)}, fixTypeBranding) + records, err := newFileRecordSync(x, opts, fixTypeBranding, string(plugin.AdminBranding)) if err != nil { return fixResult{}, err } - result.add(frResult) + for _, field := range fields { + // --limit may leave another field on the shared old URL. + oldURL := swapPrefix(*field.value, opts.DstPrefix, opts.SrcPrefix) + if slices.ContainsFunc(fields, func(f brandingField) bool { return *f.value == oldURL }) { + continue + } + if err := records.sync(*field.value); err != nil { + return fixResult{}, err + } + } return result, nil } diff --git a/internal/cli/fix_cache.go b/internal/cli/fix_cache.go index d0a646ca7..2692060ba 100644 --- a/internal/cli/fix_cache.go +++ b/internal/cli/fix_cache.go @@ -33,11 +33,15 @@ func invalidateSiteInfoCache(cacheFilePath string, fixTypes []string) { return } - // The branding site-info cache only lives in the file-backed memory cache. - // When a cache plugin (e.g. Redis) is used, no file path is configured, so - // warn the admin to flush it manually instead of silently doing nothing. + // The app uses an enabled cache plugin (e.g. Redis) instead of the file + // cache, even when a file path is configured, and the CLI cannot tell + // whether one is enabled. So always remind the admin about it. + key := constant.SiteInfoCacheKey + constant.SiteTypeBranding + defer fmt.Printf("[cache] if a cache plugin (e.g. Redis) is enabled, delete %s there, or branding refreshes when it expires (up to %s)\n", + key, constant.SiteInfoCacheTime) + if len(cacheFilePath) == 0 { - fmt.Println("[cache] no file cache configured; flush your cache backend or restart the application to refresh branding") + fmt.Println("[cache] no file cache configured") return } @@ -47,11 +51,10 @@ func invalidateSiteInfoCache(cacheFilePath string, fixTypes []string) { return } - key := constant.SiteInfoCacheKey + constant.SiteTypeBranding if err := memCache.Del(context.Background(), key); err != nil { fmt.Printf("[cache] failed to delete key %s: %v\n", key, err) } else { - fmt.Printf("[cache] invalidated %s\n", key) + fmt.Printf("[cache] invalidated %s in the cache file\n", key) } if err := memory.Save(memCache, cacheFilePath); err != nil { diff --git a/internal/cli/fix_filerecord.go b/internal/cli/fix_filerecord.go index 439f81fe4..87debcd20 100644 --- a/internal/cli/fix_filerecord.go +++ b/internal/cli/fix_filerecord.go @@ -21,27 +21,47 @@ package cli import ( "fmt" + "strings" "github.com/apache/answer/internal/entity" + "xorm.io/builder" "xorm.io/xorm" ) -func fixFileRecordBySources(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState, sources []string, label string) (fixResult, error) { - result := fixResult{} - - err := runBatchedFix(state, func(offset int) ([]*entity.FileRecord, error) { +// scanFileRecords visits matching file records in id order, ignoring --limit. +func scanFileRecords(x *xorm.Engine, label string, cond builder.Cond, visit func(*entity.FileRecord) error) error { + return runBatchedFix(newFixRunState(0), func(afterID int64) ([]*entity.FileRecord, int64, error) { records := make([]*entity.FileRecord, 0) - err := x.Select("id, file_url"). - In("source", sources). - OrderBy("id"). - Limit(fixBatchSize, offset). - Find(&records) + err := x.Select("id, file_url").Where(builder.Gt{"id": afterID}.And(cond)).OrderBy("id").Limit(fixBatchSize).Find(&records) if err != nil { - return nil, fmt.Errorf("query %s file_record failed: %w", label, err) + return nil, 0, fmt.Errorf("query %s file_record failed: %w", label, err) + } + var lastID int64 + if len(records) > 0 { + lastID = int64(records[len(records)-1].ID) + } + return records, lastID, nil + }, visit) +} + +// fixFileRecordBySources ignores --limit and keeps unreferenced records (see +// findUnreferencedRecords) on their old URL. +func fixFileRecordBySources(x *xorm.Engine, opts *FixURLPrefixOptions, sources []string, label string) (fixResult, error) { + held, err := findUnreferencedRecords(x, opts, sources, label) + if err != nil { + return fixResult{}, err + } + + result := fixResult{} + state := newFixRunState(0) + err = scanFileRecords(x, label, builder.In("source", sources), func(record *entity.FileRecord) error { + target := fmt.Sprintf("file_record id=%d", record.ID) + if held[record.ID] { + fmt.Printf("[%s] WARN %s: no revision references the new URL yet, keeping: %s\n", label, target, record.FileURL) + result.Skipped++ + return nil } - return records, nil - }, func(record *entity.FileRecord) error { - return applyValueFix(&result, opts, state, label, fmt.Sprintf("file_record id=%d", record.ID), record.FileURL, func(newURL string) error { + return applyValueFix(&result, opts, state, label, target, record.FileURL, func(newURL string) error { _, err := x.ID(record.ID).Cols("file_url").NoAutoTime().Update(&entity.FileRecord{FileURL: newURL}) if err != nil { return fmt.Errorf("update %s file_record (id=%d) failed: %w", label, record.ID, err) @@ -52,6 +72,101 @@ func fixFileRecordBySources(x *xorm.Engine, opts *FixURLPrefixOptions, state *fi if err != nil { return fixResult{}, err } - return result, nil } + +// findUnreferencedRecords returns unlinked records whose new URL is in no +// revision yet; the cleaner would delete them. Dry runs assume revisions fixed. +func findUnreferencedRecords(x *xorm.Engine, opts *FixURLPrefixOptions, sources []string, label string) (map[int]bool, error) { + pending := make(map[string][]int) // new URL -> record IDs + maxLen := 0 + cond := builder.In("source", sources).And(builder.Eq{"object_id": 0, "status": entity.FileRecordStatusAvailable}) + err := scanFileRecords(x, label, cond, func(record *entity.FileRecord) error { + if classifyValue(record.FileURL, opts.SrcPrefix, opts.DstPrefix) == matchFix { + newURL := swapPrefix(record.FileURL, opts.SrcPrefix, opts.DstPrefix) + pending[newURL] = append(pending[newURL], record.ID) + maxLen = max(maxLen, len(newURL)) + } + return nil + }) + if err != nil || len(pending) == 0 { + return nil, err + } + + err = runBatchedFix(newFixRunState(0), func(afterID int64) ([]*entity.Revision, int64, error) { + revisions := make([]*entity.Revision, 0) + err := x.Select("id, content").Where("id > ?", afterID).OrderBy("id").Limit(fixBatchSize).Find(&revisions) + if err != nil { + return nil, 0, fmt.Errorf("query revisions for %s file_record failed: %w", label, err) + } + lastID, err := lastStringID(revisions, func(r *entity.Revision) string { return r.ID }) + return revisions, lastID, err + }, func(revision *entity.Revision) error { + content := revision.Content + if opts.DryRun && classifyText(content, opts.SrcPrefix, opts.DstPrefix, jsonURLMarkers) == matchFix { + content = replaceURLPrefix(content, opts.SrcPrefix, opts.DstPrefix, jsonURLMarkers) + } + // Pending URLs start with DST_PREFIX: try each such substring up to maxLen. + for rest := content; ; rest = rest[1:] { + i := strings.Index(rest, opts.DstPrefix) + if i < 0 { + break + } + rest = rest[i:] + for n := len(opts.DstPrefix) + 1; n <= min(maxLen, len(rest)); n++ { + delete(pending, rest[:n]) + } + } + return nil + }) + if err != nil { + return nil, err + } + + held := make(map[int]bool) + for _, ids := range pending { + for _, id := range ids { + held[id] = true + } + } + return held, nil +} + +// fileRecordSync moves a source's records off SRC_PREFIX once their value is on +// DST_PREFIX, which also repairs records left behind by earlier runs. +type fileRecordSync struct { + x *xorm.Engine + opts *FixURLPrefixOptions + label string + ids map[string][]int // SRC_PREFIX URL -> record IDs; file_url is not indexed +} + +func newFileRecordSync(x *xorm.Engine, opts *FixURLPrefixOptions, label, source string) (*fileRecordSync, error) { + s := &fileRecordSync{x: x, opts: opts, label: label, ids: make(map[string][]int)} + if opts.DryRun { + return s, nil + } + cond := builder.Eq{"source": source}.And(builder.Expr("file_url LIKE ?", opts.SrcPrefix+"%")) + err := scanFileRecords(x, label, cond, func(record *entity.FileRecord) error { + s.ids[record.FileURL] = append(s.ids[record.FileURL], record.ID) + return nil + }) + return s, err +} + +func (s *fileRecordSync) sync(value string) error { + if s.opts.DryRun || classifyValue(value, s.opts.SrcPrefix, s.opts.DstPrefix) != matchSkip { + return nil + } + oldURL := swapPrefix(value, s.opts.DstPrefix, s.opts.SrcPrefix) + ids, ok := s.ids[oldURL] + if !ok { + return nil + } + _, err := s.x.In("id", ids).Cols("file_url").NoAutoTime().Update(&entity.FileRecord{FileURL: value}) + if err != nil { + return fmt.Errorf("update %s file_record (url=%s) failed: %w", s.label, oldURL, err) + } + delete(s.ids, oldURL) + return nil +} diff --git a/internal/cli/fix_post.go b/internal/cli/fix_post.go index c743be462..8a3139837 100644 --- a/internal/cli/fix_post.go +++ b/internal/cli/fix_post.go @@ -28,56 +28,83 @@ import ( "xorm.io/xorm" ) -func fixPost(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState) (fixResult, error) { - result := fixResult{} +// postTable is a table with original_text/parsed_text; objectType 0 means its +// rows have no revisions. +type postTable struct { + name string + objectType int +} - // Fix question and answer body text. - for _, tableName := range []string{"question", "answer"} { - r, err := fixPostTable(x, opts, state, tableName) - if err != nil { - return fixResult{}, err - } - result.add(r) +// postTables lists the tables that may link uploads. Comments have no upload +// button but keep pasted image URLs. +func postTables() []postTable { + return []postTable{ + {"question", constant.ObjectTypeStrMapping[constant.QuestionObjectType]}, + {"answer", constant.ObjectTypeStrMapping[constant.AnswerObjectType]}, + {"tag", constant.ObjectTypeStrMapping[constant.TagObjectType]}, + {"comment", 0}, } +} - // Fix revisions for questions and answers. - for _, objectType := range []int{ - constant.ObjectTypeStrMapping[constant.QuestionObjectType], - constant.ObjectTypeStrMapping[constant.AnswerObjectType], - } { - r, err := fixRevisionByObjectType(x, opts, state, objectType, fixTypePost) +// fixPost fixes each post with its revisions as one unit, so --limit never +// leaves old revisions that approving a pending edit would write back. It +// returns the units, counted by --limit, and the post file records, not counted. +func fixPost(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState) (units, files fixResult, err error) { + for _, t := range postTables() { + r, err := fixPostTable(x, opts, state, t) if err != nil { - return fixResult{}, err + return fixResult{}, fixResult{}, err + } + units.add(r) + if r, err = fixOrphanRevisions(x, opts, state, t); err != nil { + return fixResult{}, fixResult{}, err } - result.add(r) + units.add(r) } - // Fix file records. - fileResult, err := fixFileRecordBySources(x, opts, state, + // Must run after revisions, which it checks. + files, err = fixFileRecordBySources(x, opts, []string{string(plugin.UserPost), string(plugin.UserPostAttachment)}, fixTypePost) if err != nil { - return fixResult{}, err + return fixResult{}, fixResult{}, err } - result.add(fileResult) - - return result, nil + return units, files, nil } -// postRow represents a question or answer row with its text fields. type postRow struct { ID string `xorm:"id"` OriginalText string `xorm:"original_text"` ParsedText string `xorm:"parsed_text"` } -func fixPostTable(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState, tableName string) (fixResult, error) { +type postUnit struct { + row *postRow + revisions []*entity.Revision +} + +func fixPostTable(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState, t postTable) (fixResult, error) { result := fixResult{} err := runBatchedFix(state, - func(offset int) ([]*postRow, error) { return queryPostRows(x, tableName, offset) }, - func(row *postRow) error { - return applyTextFix(&result, opts, state, fixTypePost, fmt.Sprintf("%s id=%s", tableName, row.ID), - []string{row.OriginalText, row.ParsedText}, textURLMarkers, - func() error { return updatePostText(x, tableName, row, opts) }) + func(afterID int64) ([]*postUnit, int64, error) { return queryPostUnits(x, t, afterID) }, + func(u *postUnit) error { + items := []textFix{{ + target: fmt.Sprintf("%s id=%s", t.name, u.row.ID), + texts: []string{u.row.OriginalText, u.row.ParsedText}, + markers: textURLMarkers, + update: func(db xorm.Interface) error { + _, err := db.Table(t.name).Where("id = ?", u.row.ID).Cols("original_text", "parsed_text"). + Update(&postRow{ + OriginalText: fixTextField(u.row.OriginalText, opts, textURLMarkers), + ParsedText: fixTextField(u.row.ParsedText, opts, textURLMarkers), + }) + if err != nil { + return fmt.Errorf("update %s (id=%s) failed: %w", t.name, u.row.ID, err) + } + return nil + }, + }} + items = append(items, revisionFixes(opts, u.revisions)...) + return applyUnitFix(x, &result, opts, state, items) }) if err != nil { return fixResult{}, err @@ -85,48 +112,75 @@ func fixPostTable(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState, return result, nil } -func queryPostRows(x *xorm.Engine, tableName string, offset int) ([]*postRow, error) { +// queryPostUnits loads a page of posts and, in one more query, their revisions. +func queryPostUnits(x *xorm.Engine, t postTable, afterID int64) ([]*postUnit, int64, error) { rows := make([]*postRow, 0) - err := x.Table(tableName).Select("id, original_text, parsed_text"). + err := x.Table(t.name).Select("id, original_text, parsed_text"). + Where("id > ?", afterID). OrderBy("id"). - Limit(fixBatchSize, offset). + Limit(fixBatchSize). Find(&rows) if err != nil { - return nil, fmt.Errorf("query %s failed: %w", tableName, err) + return nil, 0, fmt.Errorf("query %s failed: %w", t.name, err) + } + lastID, err := lastStringID(rows, func(r *postRow) string { return r.ID }) + if err != nil || len(rows) == 0 { + return nil, lastID, err } - return rows, nil -} - -func updatePostText(x *xorm.Engine, tableName string, row *postRow, opts *FixURLPrefixOptions) error { - origText := replaceURLPrefix(row.OriginalText, opts.SrcPrefix, opts.DstPrefix, textURLMarkers) - parsedText := replaceURLPrefix(row.ParsedText, opts.SrcPrefix, opts.DstPrefix, textURLMarkers) - var bean interface{} - switch tableName { - case "question": - bean = &entity.Question{OriginalText: origText, ParsedText: parsedText} - case "answer": - bean = &entity.Answer{OriginalText: origText, ParsedText: parsedText} - default: - return fmt.Errorf("unknown post table %q", tableName) + units := make([]*postUnit, len(rows)) + byID := make(map[string]*postUnit, len(rows)) + ids := make([]string, len(rows)) + for i, row := range rows { + units[i] = &postUnit{row: row} + byID[row.ID] = units[i] + ids[i] = row.ID + } + if t.objectType == 0 { + return units, lastID, nil } - if _, err := x.ID(row.ID).Cols("original_text", "parsed_text").NoAutoTime().Update(bean); err != nil { - return fmt.Errorf("update %s (id=%s) failed: %w", tableName, row.ID, err) + revisions := make([]*entity.Revision, 0) + err = x.Select("id, object_id, content"). + Where("object_type = ?", t.objectType). + In("object_id", ids). + OrderBy("id"). + Find(&revisions) + if err != nil { + return nil, 0, fmt.Errorf("query %s revision failed: %w", t.name, err) } - return nil + for _, rev := range revisions { + if u := byID[rev.ObjectID]; u != nil { + u.revisions = append(u.revisions, rev) + } + } + return units, lastID, nil } -func fixRevisionByObjectType(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState, objectType int, label string) (fixResult, error) { +// fixOrphanRevisions fixes revisions whose post row is gone, one unit each. +func fixOrphanRevisions(x *xorm.Engine, opts *FixURLPrefixOptions, state *fixRunState, t postTable) (fixResult, error) { result := fixResult{} + if t.objectType == 0 { + return result, nil + } err := runBatchedFix(state, - func(offset int) ([]*entity.Revision, error) { - return queryRevisionsByObjectType(x, objectType, label, offset) + func(afterID int64) ([]*entity.Revision, int64, error) { + revisions := make([]*entity.Revision, 0) + err := x.Select("id, content"). + Where("id > ?", afterID). + And("object_type = ?", t.objectType). + And(fmt.Sprintf("object_id NOT IN (SELECT id FROM %s)", t.name)). + OrderBy("id"). + Limit(fixBatchSize). + Find(&revisions) + if err != nil { + return nil, 0, fmt.Errorf("query orphan %s revision failed: %w", t.name, err) + } + lastID, err := lastStringID(revisions, func(r *entity.Revision) string { return r.ID }) + return revisions, lastID, err }, - func(revision *entity.Revision) error { - return applyTextFix(&result, opts, state, label, fmt.Sprintf("revision id=%s", revision.ID), - []string{revision.Content}, jsonURLMarkers, - func() error { return updateRevisionContent(x, revision, opts, label) }) + func(rev *entity.Revision) error { + return applyUnitFix(x, &result, opts, state, revisionFixes(opts, []*entity.Revision{rev})) }) if err != nil { return fixResult{}, err @@ -134,53 +188,84 @@ func fixRevisionByObjectType(x *xorm.Engine, opts *FixURLPrefixOptions, state *f return result, nil } -func queryRevisionsByObjectType(x *xorm.Engine, objectType int, label string, offset int) ([]*entity.Revision, error) { - revisions := make([]*entity.Revision, 0) - err := x.Select("id, content"). - Where("object_type = ?", objectType). - OrderBy("id"). - Limit(fixBatchSize, offset). - Find(&revisions) - if err != nil { - return nil, fmt.Errorf("query %s revision failed: %w", label, err) +// fixTextField leaves fields not classified as fixable alone, so a field on a +// DST_PREFIX that starts with SRC_PREFIX is not nested (e.g. /uploads/cdn/cdn/). +func fixTextField(text string, opts *FixURLPrefixOptions, markers []string) string { + if classifyText(text, opts.SrcPrefix, opts.DstPrefix, markers) != matchFix { + return text } - return revisions, nil + return replaceURLPrefix(text, opts.SrcPrefix, opts.DstPrefix, markers) } -func updateRevisionContent(x *xorm.Engine, revision *entity.Revision, opts *FixURLPrefixOptions, label string) error { - updated := &entity.Revision{ - Content: replaceURLPrefix(revision.Content, opts.SrcPrefix, opts.DstPrefix, jsonURLMarkers), - } - _, err := x.ID(revision.ID).Cols("content").NoAutoTime().Update(updated) - if err != nil { - return fmt.Errorf("update %s revision (id=%s) failed: %w", label, revision.ID, err) +func revisionFixes(opts *FixURLPrefixOptions, revisions []*entity.Revision) []textFix { + items := make([]textFix, 0, len(revisions)) + for _, rev := range revisions { + items = append(items, textFix{ + target: fmt.Sprintf("revision id=%s", rev.ID), + texts: []string{rev.Content}, + markers: jsonURLMarkers, + update: func(db xorm.Interface) error { + content := replaceURLPrefix(rev.Content, opts.SrcPrefix, opts.DstPrefix, jsonURLMarkers) + if _, err := db.ID(rev.ID).Cols("content").NoAutoTime().Update(&entity.Revision{Content: content}); err != nil { + return fmt.Errorf("update revision (id=%s) failed: %w", rev.ID, err) + } + return nil + }, + }) } - return nil + return items } -func applyTextFix(result *fixResult, opts *FixURLPrefixOptions, state *fixRunState, - label, target string, texts []string, markers []string, update func() error, -) error { - switch classifyText(texts, opts.SrcPrefix, opts.DstPrefix, markers) { - case matchBoth: - fmt.Printf("[%s] WARN %s: has BOTH prefixes, skipping (manual fix needed)\n", label, target) - result.Skipped++ - return nil - case matchSkip: - fmt.Printf("[%s] skip %s: already has DST_PREFIX\n", label, target) +// textFix is one record of a unit: its text fields and how to write them back. +type textFix struct { + target string + texts []string + markers []string + update func(db xorm.Interface) error +} + +// applyUnitFix writes a unit's fixable records in one transaction and counts +// the unit once: affected if any record is fixable, skipped if a record is +// ambiguous (left for manual fix; the rest is still fixed) or none is fixable +// but some are already on DST_PREFIX. +func applyUnitFix(x *xorm.Engine, result *fixResult, opts *FixURLPrefixOptions, state *fixRunState, items []textFix) error { + var fixable []textFix + ambiguous, onDst := false, false + for _, item := range items { + switch classifyFields(item.texts, opts.SrcPrefix, opts.DstPrefix, item.markers) { + case matchBoth: + fmt.Printf("[%s] WARN %s: a field has BOTH prefixes and DST_PREFIX starts with SRC_PREFIX, skipping (manual fix needed)\n", fixTypePost, item.target) + ambiguous = true + case matchSkip: + fmt.Printf("[%s] skip %s: already has DST_PREFIX\n", fixTypePost, item.target) + onDst = true + case matchFix: + fixable = append(fixable, item) + } + } + if ambiguous || (len(fixable) == 0 && onDst) { result.Skipped++ - return nil - case matchNone: + } + if len(fixable) == 0 { return nil } state.markAffected(result) - fmt.Printf("[%s] %s: replacing %q with %q\n", label, target, opts.SrcPrefix, opts.DstPrefix) - + for _, item := range fixable { + fmt.Printf("[%s] %s: replacing %q with %q\n", fixTypePost, item.target, opts.SrcPrefix, opts.DstPrefix) + } if opts.DryRun { return nil } - if err := update(); err != nil { + _, err := x.Transaction(func(session *xorm.Session) (interface{}, error) { + for _, item := range fixable { + if err := item.update(session); err != nil { + return nil, err + } + } + return nil, nil + }) + if err != nil { return err } result.Fixed++ diff --git a/internal/cli/fix_test.go b/internal/cli/fix_test.go new file mode 100644 index 000000000..a20683b0e --- /dev/null +++ b/internal/cli/fix_test.go @@ -0,0 +1,651 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cli + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/apache/answer/internal/base/constant" + "github.com/apache/answer/internal/entity" + "github.com/apache/answer/internal/schema" + "github.com/apache/answer/plugin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "xorm.io/xorm" +) + +const ( + testSrc = "https://old.example.com/uploads/" + testDst = "https://new.example.com/uploads/" +) + +func TestClassifyText(t *testing.T) { + tests := []struct { + name string + text string + src, dst string + want prefixMatch + }{ + {"none", "plain text https://old.example.com/uploads/x.png", testSrc, testDst, matchNone}, + {"markdown src", "![](https://old.example.com/uploads/a.png)", testSrc, testDst, matchFix}, + {"html dst", ``, testSrc, testDst, matchSkip}, + {"both, disjoint prefixes", "[a](https://old.example.com/uploads/a) [b](https://new.example.com/uploads/b)", testSrc, testDst, matchFix}, + {"dst nested under src is not src", "![](/uploads/new/a.png)", "/uploads/", "/uploads/new/", matchSkip}, + {"src nested under dst is not dst", "![](/uploads/new/a.png)", "/uploads/new/", "/uploads/", matchFix}, + {"both, dst starts with src", "![](/uploads/a.png) ![](/uploads/new/b.png)", "/uploads/", "/uploads/new/", matchBoth}, + {"both, src starts with dst", "![](/uploads/new/a.png) ![](/uploads/b.png)", "/uploads/new/", "/uploads/", matchFix}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, classifyText(tt.text, tt.src, tt.dst, textURLMarkers)) + }) + } +} + +func TestClassifyFields(t *testing.T) { + src := "![](" + testSrc + "a.png)" + dst := "![](" + testDst + "a.png)" + const nestedSrc, nestedDst = "/uploads/", "/uploads/cdn/" + nsrc := "![](" + nestedSrc + "a.png)" + ndst := "![](" + nestedDst + "a.png)" + tests := []struct { + name string + fields []string + src, dst string + want prefixMatch + }{ + {"none", []string{"x", "y"}, testSrc, testDst, matchNone}, + {"all dst", []string{dst, dst}, testSrc, testDst, matchSkip}, + {"src and dst in different fields", []string{src, dst}, testSrc, testDst, matchFix}, + {"dst then src", []string{dst, src}, testSrc, testDst, matchFix}, + {"both in one field, disjoint prefixes", []string{src + dst, dst}, testSrc, testDst, matchFix}, + {"both in one field, nested prefixes", []string{nsrc + ndst, nsrc}, nestedSrc, nestedDst, matchBoth}, + {"both in a later field, nested prefixes", []string{nsrc, ndst + nsrc}, nestedSrc, nestedDst, matchBoth}, + {"nested prefixes across fields", []string{nsrc, ndst}, nestedSrc, nestedDst, matchFix}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, classifyFields(tt.fields, tt.src, tt.dst, textURLMarkers)) + }) + } +} + +func TestClassifyValue(t *testing.T) { + assert.Equal(t, matchFix, classifyValue(testSrc+"a.png", testSrc, testDst)) + assert.Equal(t, matchSkip, classifyValue(testDst+"a.png", testSrc, testDst)) + assert.Equal(t, matchNone, classifyValue("https://other/a.png", testSrc, testDst)) + assert.Equal(t, matchSkip, classifyValue("/uploads/new/a.png", "/uploads/", "/uploads/new/")) + assert.Equal(t, matchFix, classifyValue("/uploads/new/a.png", "/uploads/new/", "/uploads/")) +} + +func newFixTestDB(t *testing.T) *xorm.Engine { + t.Helper() + x, err := xorm.NewEngine("sqlite", ":memory:") + require.NoError(t, err) + t.Cleanup(func() { _ = x.Close() }) + require.NoError(t, x.Sync(new(entity.Question), new(entity.Answer), new(entity.Tag), new(entity.Comment), + new(entity.Revision), new(entity.FileRecord), new(entity.User), new(entity.SiteInfo), new(entity.Notification))) + return x +} + +func TestFixPost(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + + // Prefixes mixed across fields are fixed field by field. + _, err := x.Insert(&entity.Question{ID: "1", + OriginalText: "![](" + testSrc + "q.png)", + ParsedText: ``}) + require.NoError(t, err) + // Both prefixes in one field are fixed, since the prefixes do not overlap. + _, err = x.Insert(&entity.Answer{ID: "5", + OriginalText: "![](" + testSrc + "a.png) ![](" + testDst + "b.png)", + ParsedText: ``}) + require.NoError(t, err) + _, err = x.Insert(&entity.Tag{ID: "2", SlugName: "go", + OriginalText: "![](" + testSrc + "t.png)", + ParsedText: ``}) + require.NoError(t, err) + + tagObjectType := constant.ObjectTypeStrMapping[constant.TagObjectType] + migratable, _ := json.Marshal(map[string]any{ + "ID": "2", "OriginalText": "![](" + testSrc + "t.png)", "ParsedText": ``}) + mixed, _ := json.Marshal(map[string]any{ + "ID": "3", "OriginalText": "![](" + testSrc + "keep.png) ![](" + testDst + "other.png)"}) + _, err = x.Insert(&entity.Revision{ID: "11", ObjectType: tagObjectType, ObjectID: "2", Content: string(migratable)}) + require.NoError(t, err) + _, err = x.Insert(&entity.Revision{ID: "12", ObjectType: tagObjectType, ObjectID: "3", Content: string(mixed)}) + require.NoError(t, err) + + _, err = x.Insert(&entity.FileRecord{ID: 1, FileURL: testSrc + "t.png", ObjectID: "0", Status: entity.FileRecordStatusAvailable, Source: string(plugin.UserPost)}) + require.NoError(t, err) + _, err = x.Insert(&entity.FileRecord{ID: 2, FileURL: testSrc + "keep.png", ObjectID: "0", Status: entity.FileRecordStatusAvailable, Source: string(plugin.UserPost)}) + require.NoError(t, err) + // Linked records are not checked against revisions. + _, err = x.Insert(&entity.FileRecord{ID: 3, FileURL: testSrc + "linked.png", ObjectID: "100", Source: string(plugin.UserPost)}) + require.NoError(t, err) + // Deleted records are not checked against revisions either. + _, err = x.Insert(&entity.FileRecord{ID: 4, FileURL: testSrc + "deleted.png", ObjectID: "0", + Status: entity.FileRecordStatusDeleted, Source: string(plugin.UserPost)}) + require.NoError(t, err) + + rows, files, err := fixPost(x, opts, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, int64(4), rows.Fixed) // question 1, answer 5, tag 2 with revision 11, orphan revision 12 + assert.Equal(t, int64(0), rows.Skipped) + assert.Equal(t, int64(4), files.Fixed) // file_record 1 to 4 + assert.Equal(t, int64(0), files.Skipped) + + q := &entity.Question{} + _, err = x.ID("1").Get(q) + require.NoError(t, err) + assert.Equal(t, "![]("+testDst+"q.png)", q.OriginalText) + assert.Equal(t, ``, q.ParsedText) + + a := &entity.Answer{} + _, err = x.ID("5").Get(a) + require.NoError(t, err) + assert.Equal(t, "![]("+testDst+"a.png) ![]("+testDst+"b.png)", a.OriginalText) + assert.Equal(t, ``, a.ParsedText) + + tag := &entity.Tag{} + _, err = x.ID("2").Get(tag) + require.NoError(t, err) + assert.Equal(t, "![]("+testDst+"t.png)", tag.OriginalText) + assert.Equal(t, ``, tag.ParsedText) + + rev := &entity.Revision{} + _, err = x.ID("11").Get(rev) + require.NoError(t, err) + var doc map[string]any + require.NoError(t, json.Unmarshal([]byte(rev.Content), &doc)) + assert.Equal(t, "2", doc["ID"]) + assert.Equal(t, "![]("+testDst+"t.png)", doc["OriginalText"]) + assert.Equal(t, ``, doc["ParsedText"]) + + rev = &entity.Revision{} + _, err = x.ID("12").Get(rev) + require.NoError(t, err) + doc = map[string]any{} + require.NoError(t, json.Unmarshal([]byte(rev.Content), &doc)) + assert.Equal(t, "![]("+testDst+"keep.png) ![]("+testDst+"other.png)", doc["OriginalText"]) + + assertFileURL(t, x, 1, testDst+"t.png") + // Referenced by the fixed mixed revision, so it follows. + assertFileURL(t, x, 2, testDst+"keep.png") + assertFileURL(t, x, 3, testDst+"linked.png") + assertFileURL(t, x, 4, testDst+"deleted.png") +} + +// assertFileURL asserts the file_url of file record id. +func assertFileURL(t *testing.T, x *xorm.Engine, id int, want string) { + t.Helper() + fr := &entity.FileRecord{} + _, err := x.ID(id).Get(fr) + require.NoError(t, err) + assert.Equal(t, want, fr.FileURL) +} + +func TestFixAvatarLimitKeepsFileRecordsInSync(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + + for i, name := range []string{"a.png", "b.png"} { + _, err := x.Insert(&entity.User{ID: fmt.Sprint(i + 1), Username: name, + Avatar: `{"type":"custom","custom":"` + testSrc + name + `"}`}) + require.NoError(t, err) + _, err = x.Insert(&entity.FileRecord{ID: i + 1, FileURL: testSrc + name, Source: string(plugin.UserAvatar)}) + require.NoError(t, err) + } + + result, err := fixAvatar(x, opts, newFixRunState(1)) + require.NoError(t, err) + assert.Equal(t, int64(1), result.Fixed) + + // Only the migrated user's file record follows; the other stays on the old URL. + assertFileURL(t, x, 1, testDst+"a.png") + assertFileURL(t, x, 2, testSrc+"b.png") +} + +func TestFixAvatarRepairsStaleFileRecord(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + + // The avatar was already moved (e.g. by hand), but its file record was not. + _, err := x.Insert(&entity.User{ID: "1", Username: "u1", + Avatar: `{"type":"custom","custom":"` + testDst + `a.png"}`}) + require.NoError(t, err) + _, err = x.Insert(&entity.FileRecord{ID: 1, FileURL: testSrc + "a.png", Source: string(plugin.UserAvatar)}) + require.NoError(t, err) + + _, err = fixAvatar(x, &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst, DryRun: true}, newFixRunState(0)) + require.NoError(t, err) + assertFileURL(t, x, 1, testSrc+"a.png") + + result, err := fixAvatar(x, opts, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, int64(0), result.Fixed) + assert.Equal(t, int64(1), result.Skipped) + assertFileURL(t, x, 1, testDst+"a.png") +} + +func TestFixBrandingLimitKeepsFileRecordsInSync(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + + // logo and mobile_logo share one file record. + _, err := x.Insert(&entity.SiteInfo{Type: constant.SiteTypeBranding, + Content: `{"logo":"` + testSrc + `logo.png","mobile_logo":"` + testSrc + `logo.png","favicon":"` + testSrc + `favicon.png"}`}) + require.NoError(t, err) + _, err = x.Insert(&entity.FileRecord{ID: 1, FileURL: testSrc + "logo.png", Source: string(plugin.AdminBranding)}) + require.NoError(t, err) + _, err = x.Insert(&entity.FileRecord{ID: 2, FileURL: testSrc + "favicon.png", Source: string(plugin.AdminBranding)}) + require.NoError(t, err) + + result, err := fixBranding(x, opts, newFixRunState(1)) + require.NoError(t, err) + assert.Equal(t, int64(1), result.Fixed) + // mobile_logo still uses the old URL, so the shared record must keep it. + assertFileURL(t, x, 1, testSrc+"logo.png") + assertFileURL(t, x, 2, testSrc+"favicon.png") + + result, err = fixBranding(x, opts, newFixRunState(1)) + require.NoError(t, err) + assert.Equal(t, int64(1), result.Fixed) + assertFileURL(t, x, 1, testDst+"logo.png") + assertFileURL(t, x, 2, testSrc+"favicon.png") +} + +func TestFixPostDryRunMatchesRealRun(t *testing.T) { + x := newFixTestDB(t) + + content, _ := json.Marshal(map[string]any{"ID": "1", "OriginalText": "![](" + testSrc + "a.png)"}) + _, err := x.Insert(&entity.Revision{ID: "11", ObjectType: constant.ObjectTypeStrMapping[constant.QuestionObjectType], + ObjectID: "1", Content: string(content)}) + require.NoError(t, err) + _, err = x.Insert(&entity.FileRecord{ID: 1, FileURL: testSrc + "a.png", ObjectID: "0", Status: entity.FileRecordStatusAvailable, Source: string(plugin.UserPost)}) + require.NoError(t, err) + + // A revision mixing disjoint prefixes is fixed, so a dry run must count its file record as referenced. + mixed, _ := json.Marshal(map[string]any{ + "ID": "2", "OriginalText": "![](" + testSrc + "b.png) ![](" + testDst + "c.png)"}) + _, err = x.Insert(&entity.Revision{ID: "12", ObjectType: constant.ObjectTypeStrMapping[constant.QuestionObjectType], + ObjectID: "2", Content: string(mixed)}) + require.NoError(t, err) + _, err = x.Insert(&entity.FileRecord{ID: 2, FileURL: testSrc + "b.png", ObjectID: "0", Status: entity.FileRecordStatusAvailable, Source: string(plugin.UserPost)}) + require.NoError(t, err) + + // d.png is a bare URL, so fixing the revision's other link does not rewrite it. + bare, _ := json.Marshal(map[string]any{ + "ID": "3", "OriginalText": "![](" + testSrc + "e.png) " + testSrc + "d.png"}) + _, err = x.Insert(&entity.Revision{ID: "13", ObjectType: constant.ObjectTypeStrMapping[constant.QuestionObjectType], + ObjectID: "3", Content: string(bare)}) + require.NoError(t, err) + _, err = x.Insert(&entity.FileRecord{ID: 3, FileURL: testSrc + "d.png", ObjectID: "0", Status: entity.FileRecordStatusAvailable, Source: string(plugin.UserPost)}) + require.NoError(t, err) + + dryRows, dryFiles, err := fixPost(x, &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst, DryRun: true}, newFixRunState(0)) + require.NoError(t, err) + assertFileURL(t, x, 1, testSrc+"a.png") + + rows, files, err := fixPost(x, &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst}, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, rows.Affected, dryRows.Affected) + assert.Equal(t, rows.Skipped, dryRows.Skipped) + assert.Equal(t, files.Affected, dryFiles.Affected) + assert.Equal(t, files.Skipped, dryFiles.Skipped) + assertFileURL(t, x, 1, testDst+"a.png") + assertFileURL(t, x, 2, testDst+"b.png") + assertFileURL(t, x, 3, testSrc+"d.png") +} + +func TestFixPostSkipsBothWithOverlappingPrefixes(t *testing.T) { + x := newFixTestDB(t) + const src, dst = "/uploads/", "/uploads/cdn/" + + // Rewriting src here would also turn /uploads/cdn/ into /uploads/cdn/cdn/. + mixedText := "![](" + src + "a.png) ![](" + dst + "b.png)" + _, err := x.Insert(&entity.Answer{ID: "5", OriginalText: mixedText, ParsedText: ``}) + require.NoError(t, err) + mixed, _ := json.Marshal(map[string]any{"ID": "5", "OriginalText": mixedText}) + _, err = x.Insert(&entity.Revision{ID: "12", ObjectType: constant.ObjectTypeStrMapping[constant.AnswerObjectType], + ObjectID: "5", Content: string(mixed)}) + require.NoError(t, err) + // Referenced only by the skipped revision, so it must keep the old URL. + _, err = x.Insert(&entity.FileRecord{ID: 1, FileURL: src + "a.png", ObjectID: "0", Status: entity.FileRecordStatusAvailable, Source: string(plugin.UserPost)}) + require.NoError(t, err) + + dryRows, dryFiles, err := fixPost(x, &FixURLPrefixOptions{SrcPrefix: src, DstPrefix: dst, DryRun: true}, newFixRunState(0)) + require.NoError(t, err) + rows, files, err := fixPost(x, &FixURLPrefixOptions{SrcPrefix: src, DstPrefix: dst}, newFixRunState(0)) + require.NoError(t, err) + + assert.Equal(t, fixResult{Skipped: 1}, rows) // answer 5 with revision 12 + assert.Equal(t, fixResult{Skipped: 1}, files) + assert.Equal(t, rows, dryRows) + assert.Equal(t, files, dryFiles) + + a := &entity.Answer{} + _, err = x.ID("5").Get(a) + require.NoError(t, err) + assert.Equal(t, mixedText, a.OriginalText) + rev := &entity.Revision{} + _, err = x.ID("12").Get(rev) + require.NoError(t, err) + assert.Equal(t, string(mixed), rev.Content) + assertFileURL(t, x, 1, src+"a.png") +} + +func TestFixAvatarSkipsInvalidJSON(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + + // Matches the custom-avatar query but cannot be parsed. + invalid := `{"custom":"` + testSrc + `a.png"` + _, err := x.Insert(&entity.User{ID: "1", Username: "u1", Avatar: invalid}) + require.NoError(t, err) + // A valid row after the invalid one must still be fixed. + _, err = x.Insert(&entity.User{ID: "2", Username: "u2", + Avatar: `{"type":"custom","custom":"` + testSrc + `b.png"}`}) + require.NoError(t, err) + + result, err := fixAvatar(x, opts, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, int64(1), result.Fixed) + assert.Equal(t, int64(1), result.Skipped) + + u := &entity.User{} + _, err = x.ID("1").Get(u) + require.NoError(t, err) + assert.Equal(t, invalid, u.Avatar) + + u = &entity.User{} + _, err = x.ID("2").Get(u) + require.NoError(t, err) + // Stored in the same format the app writes. + assert.Equal(t, schema.CustomAvatar(testDst+"b.png").ToJsonString(), u.Avatar) +} + +func TestFixPostMixedFieldsWithNestedDst(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: "/uploads/", DstPrefix: "/uploads/cdn/"} + + // parsed_text is already on DST, which starts with SRC; it must not be rewritten again. + _, err := x.Insert(&entity.Question{ID: "1", + OriginalText: "![](/uploads/q.png)", + ParsedText: ``}) + require.NoError(t, err) + + rows, _, err := fixPost(x, opts, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, int64(1), rows.Fixed) + + q := &entity.Question{} + _, err = x.ID("1").Get(q) + require.NoError(t, err) + assert.Equal(t, "![](/uploads/cdn/q.png)", q.OriginalText) + assert.Equal(t, ``, q.ParsedText) +} + +func TestFixPostLimitStillSyncsFileRecords(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst, Limit: 1} + + content, _ := json.Marshal(map[string]any{"ID": "1", "OriginalText": "![](" + testSrc + "a.png)"}) + _, err := x.Insert(&entity.Revision{ID: "11", ObjectType: constant.ObjectTypeStrMapping[constant.QuestionObjectType], + ObjectID: "1", Content: string(content)}) + require.NoError(t, err) + _, err = x.Insert(&entity.FileRecord{ID: 1, FileURL: testSrc + "a.png", ObjectID: "0", Status: entity.FileRecordStatusAvailable, Source: string(plugin.UserPost)}) + require.NoError(t, err) + + state := newFixRunState(opts.Limit) + rows, files, err := fixPost(x, opts, state) + require.NoError(t, err) + assert.True(t, state.exhausted()) + // The file record is reported apart from the rows --limit counts. + assert.Equal(t, int64(1), rows.Affected) + assert.Equal(t, int64(1), files.Affected) + assertFileURL(t, x, 1, testDst+"a.png") +} + +// insertQuestionWithRevision inserts a question and one revision of it, both +// linking name under prefix. +func insertQuestionWithRevision(t *testing.T, x *xorm.Engine, id, revID, prefix, name string) { + t.Helper() + _, err := x.Insert(&entity.Question{ID: id, + OriginalText: "![](" + prefix + name + ")", + ParsedText: ``}) + require.NoError(t, err) + content, _ := json.Marshal(map[string]any{"ID": id, "OriginalText": "![](" + prefix + name + ")"}) + _, err = x.Insert(&entity.Revision{ID: revID, ObjectType: constant.ObjectTypeStrMapping[constant.QuestionObjectType], + ObjectID: id, Content: string(content)}) + require.NoError(t, err) +} + +// assertRevisionHas asserts that the content of revision id contains want. +func assertRevisionHas(t *testing.T, x *xorm.Engine, id, want string) { + t.Helper() + rev := &entity.Revision{} + _, err := x.ID(id).Get(rev) + require.NoError(t, err) + assert.Contains(t, rev.Content, want) +} + +func TestFixPostLimitFixesPostWithItsRevisions(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst, Limit: 1} + insertQuestionWithRevision(t, x, "1", "11", testSrc, "a.png") + insertQuestionWithRevision(t, x, "2", "12", testSrc, "b.png") + + rows, _, err := fixPost(x, opts, newFixRunState(opts.Limit)) + require.NoError(t, err) + assert.Equal(t, fixResult{Affected: 1, Fixed: 1}, rows) + + // The post within the limit is fixed with its revision; the other keeps both on SRC. + q := &entity.Question{} + _, err = x.ID("1").Get(q) + require.NoError(t, err) + assert.Equal(t, "![]("+testDst+"a.png)", q.OriginalText) + assertRevisionHas(t, x, "11", "]("+testDst+"a.png)") + + q = &entity.Question{} + _, err = x.ID("2").Get(q) + require.NoError(t, err) + assert.Equal(t, "![]("+testSrc+"b.png)", q.OriginalText) + assertRevisionHas(t, x, "12", "]("+testSrc+"b.png)") +} + +func TestFixPostFixesOldRevisionsOfPostOnDst(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + // The post is already on DST, but one of its revisions is not. + insertQuestionWithRevision(t, x, "1", "11", testDst, "a.png") + old, _ := json.Marshal(map[string]any{"ID": "1", "OriginalText": "![](" + testSrc + "old.png)"}) + _, err := x.Insert(&entity.Revision{ID: "10", ObjectType: constant.ObjectTypeStrMapping[constant.QuestionObjectType], + ObjectID: "1", Content: string(old)}) + require.NoError(t, err) + + rows, _, err := fixPost(x, opts, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, fixResult{Affected: 1, Fixed: 1}, rows) + assertRevisionHas(t, x, "10", "]("+testDst+"old.png)") + assertRevisionHas(t, x, "11", "]("+testDst+"a.png)") +} + +func TestFixPostPagesByID(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + // More than two pages; ids 1..n also order differently as strings ("10" < "9"). + n := 2*fixBatchSize + 1 + for i := 1; i <= n; i++ { + insertQuestionWithRevision(t, x, fmt.Sprint(i), fmt.Sprint(1000+i), testSrc, fmt.Sprintf("%d.png", i)) + } + + rows, _, err := fixPost(x, opts, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, fixResult{Affected: int64(n), Fixed: int64(n)}, rows) + + count, err := x.Where("original_text LIKE ?", "%"+testSrc+"%").Count(new(entity.Question)) + require.NoError(t, err) + assert.Zero(t, count) + count, err = x.Where("content LIKE ?", "%"+testSrc+"%").Count(new(entity.Revision)) + require.NoError(t, err) + assert.Zero(t, count) +} + +func TestFixAvatarFixesCustomURLOfOtherType(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + + // The user switched to gravatar; the old custom URL is kept for switching back. + _, err := x.Insert(&entity.User{ID: "1", Username: "u1", + Avatar: `{"type":"gravatar","gravatar":"https://gravatar.example/x","custom":"` + testSrc + `a.png"}`}) + require.NoError(t, err) + _, err = x.Insert(&entity.FileRecord{ID: 1, FileURL: testSrc + "a.png", Source: string(plugin.UserAvatar)}) + require.NoError(t, err) + + result, err := fixAvatar(x, opts, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, int64(1), result.Fixed) + + u := &entity.User{} + _, err = x.ID("1").Get(u) + require.NoError(t, err) + var avatar schema.AvatarInfo + require.NoError(t, json.Unmarshal([]byte(u.Avatar), &avatar)) + assert.Equal(t, schema.AvatarInfo{Type: "gravatar", Gravatar: "https://gravatar.example/x", Custom: testDst + "a.png"}, avatar) + assertFileURL(t, x, 1, testDst+"a.png") +} + +func TestFixPostFixesComments(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + _, err := x.Insert(&entity.Comment{ID: "1", UserID: "1", ObjectID: "1", + OriginalText: "see ![](" + testSrc + "c.png)", + ParsedText: `

see

`}) + require.NoError(t, err) + + rows, _, err := fixPost(x, opts, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, fixResult{Affected: 1, Fixed: 1}, rows) + + c := &entity.Comment{} + _, err = x.ID("1").Get(c) + require.NoError(t, err) + assert.Equal(t, "see ![]("+testDst+"c.png)", c.OriginalText) + assert.Equal(t, `

see

`, c.ParsedText) +} + +func TestFixAvatarFixesNotificationAvatars(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + + // Encoded the way the app stores notification content. + notificationContent := func(avatar string) string { + content, _ := json.Marshal(&schema.NotificationContent{ + UserInfo: &schema.UserBasicInfo{ID: "7", Username: "u7", Avatar: avatar}, + ObjectInfo: schema.ObjectInfo{Title: "q", ObjectID: "1", ObjectType: "question"}, + NotificationAction: "notification.action.answered_question", + }) + return string(content) + } + for id, avatar := range map[string]string{ + "1": testSrc + "a.png", + "2": testDst + "a.png", + "3": "https://gravatar.example/x", + } { + _, err := x.Insert(&entity.Notification{ID: id, UserID: "8", ObjectID: "1", Content: notificationContent(avatar)}) + require.NoError(t, err) + } + + result, err := fixAvatar(x, opts, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, fixResult{Affected: 1, Fixed: 1}, result) + + for id, want := range map[string]string{ + "1": notificationContent(testDst + "a.png"), + "2": notificationContent(testDst + "a.png"), + "3": notificationContent("https://gravatar.example/x"), + } { + n := &entity.Notification{} + _, err = x.ID(id).Get(n) + require.NoError(t, err) + assert.Equal(t, want, n.Content, "notification %s", id) + } +} + +func TestFixPostRollsBackUnitOnFailedWrite(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: testSrc, DstPrefix: testDst} + insertQuestionWithRevision(t, x, "1", "11", testSrc, "a.png") + // The post row is written first; failing its revision must undo that write. + _, err := x.Exec("CREATE TRIGGER fail_revision BEFORE UPDATE ON revision BEGIN SELECT RAISE(ABORT, 'boom'); END") + require.NoError(t, err) + + _, _, err = fixPost(x, opts, newFixRunState(0)) + require.ErrorContains(t, err, "boom") + + q := &entity.Question{} + _, err = x.ID("1").Get(q) + require.NoError(t, err) + assert.Equal(t, "![]("+testSrc+"a.png)", q.OriginalText) + assertRevisionHas(t, x, "11", "]("+testSrc+"a.png)") +} + +func TestFixPostMixedFieldWhenSrcStartsWithDst(t *testing.T) { + x := newFixTestDB(t) + opts := &FixURLPrefixOptions{SrcPrefix: "/uploads/cdn/", DstPrefix: "/uploads/"} + // Replacing /uploads/cdn/ cannot touch a URL that is only /uploads/. + _, err := x.Insert(&entity.Answer{ID: "5", + OriginalText: "![a](/uploads/cdn/a.png) ![b](/uploads/b.png)", + ParsedText: ``}) + require.NoError(t, err) + + rows, _, err := fixPost(x, opts, newFixRunState(0)) + require.NoError(t, err) + assert.Equal(t, fixResult{Affected: 1, Fixed: 1}, rows) + + a := &entity.Answer{} + _, err = x.ID("5").Get(a) + require.NoError(t, err) + assert.Equal(t, "![a](/uploads/a.png) ![b](/uploads/b.png)", a.OriginalText) + assert.Equal(t, ``, a.ParsedText) +} + +func TestFixURLPrefixRejectsMismatchedTrailingSlash(t *testing.T) { + err := FixURLPrefix(nil, &FixURLPrefixOptions{FixType: fixTypeAll, SrcPrefix: "/uploads/", DstPrefix: "/cdn"}) + require.Error(t, err) + assert.Contains(t, err.Error(), `end with "/"`) +} + +func TestFixURLPrefixRejectsNegativeLimit(t *testing.T) { + err := FixURLPrefix(nil, &FixURLPrefixOptions{FixType: fixTypeAll, SrcPrefix: testSrc, DstPrefix: testDst, Limit: -1}) + require.Error(t, err) + assert.Contains(t, err.Error(), "limit must not be negative") +} + +func TestFixURLPrefixRejectsUnknownTypeBeforeConnecting(t *testing.T) { + // A nil database config would fail to connect, so this only passes if the type is checked first. + err := FixURLPrefix(nil, &FixURLPrefixOptions{FixType: "avtar", SrcPrefix: testSrc, DstPrefix: testDst}) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown fix type") +}