mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-18 16:56:35 +00:00
chore: tidy up uploading migration code (#12577)
- Validate and sanitize topics. - Cap topics at 25 (limit used elsewhere, now unified constant). - Add more details and rephrase common user-facing error messages. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12577
This commit is contained in:
Gusted
Gusted
parent
f4450f5015
commit
aae19e6c19
5 changed files with 123 additions and 100 deletions
|
|
@ -20,6 +20,9 @@ func init() {
|
|||
db.RegisterModel(new(RepoTopic))
|
||||
}
|
||||
|
||||
// MaxTopicsPerRepo caps the amount of topics allowed per repository.
|
||||
const MaxTopicsPerRepo = 25
|
||||
|
||||
var topicPattern = regexp.MustCompile(`^[a-z0-9][-.a-z0-9]*$`)
|
||||
|
||||
// Topic represents a topic of repositories
|
||||
|
|
|
|||
|
|
@ -104,7 +104,7 @@ func UpdateTopics(ctx *context.APIContext) {
|
|||
topicNames := form.Topics
|
||||
validTopics, invalidTopics := repo_model.SanitizeAndValidateTopics(topicNames)
|
||||
|
||||
if len(validTopics) > 25 {
|
||||
if len(validTopics) > repo_model.MaxTopicsPerRepo {
|
||||
ctx.JSON(http.StatusUnprocessableEntity, map[string]any{
|
||||
"invalidTopics": nil,
|
||||
"message": "Exceeding maximum number of topics per repo",
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ func TopicsPost(ctx *context.Context) {
|
|||
|
||||
validTopics, invalidTopics := repo_model.SanitizeAndValidateTopics(topics)
|
||||
|
||||
if len(validTopics) > 25 {
|
||||
if len(validTopics) > repo_model.MaxTopicsPerRepo {
|
||||
ctx.JSON(http.StatusUnprocessableEntity, map[string]any{
|
||||
"invalidTopics": nil,
|
||||
"message": ctx.Tr("repo.topic.count_prompt"),
|
||||
|
|
|
|||
|
|
@ -161,18 +161,18 @@ func (g *GiteaLocalUploader) Close() {
|
|||
|
||||
// CreateTopics creates topics
|
||||
func (g *GiteaLocalUploader) CreateTopics(topics ...string) error {
|
||||
// Ignore topics too long for the db
|
||||
c := 0
|
||||
for _, topic := range topics {
|
||||
if len(topic) > 50 {
|
||||
continue
|
||||
}
|
||||
|
||||
topics[c] = topic
|
||||
c++
|
||||
validTopics, invalidTopics := repo_model.SanitizeAndValidateTopics(topics)
|
||||
if len(invalidTopics) > 0 {
|
||||
log.Warn("Invalid topics: %v in migration to %s/%s", invalidTopics, g.repoOwner, g.repoName)
|
||||
}
|
||||
topics = topics[:c]
|
||||
return repo_model.SaveTopics(g.ctx, g.repo.ID, topics...)
|
||||
|
||||
// Truncate if it exceeds the maximum number of topics per repo.
|
||||
if len(validTopics) > repo_model.MaxTopicsPerRepo {
|
||||
validTopics = validTopics[:repo_model.MaxTopicsPerRepo]
|
||||
log.Warn("More than %[0]d topics, truncated to first %[0]d topics in migration to %s/%s", repo_model.MaxTopicsPerRepo, g.repoOwner, g.repoName)
|
||||
}
|
||||
|
||||
return repo_model.SaveTopics(g.ctx, g.repo.ID, validTopics...)
|
||||
}
|
||||
|
||||
// CreateMilestones creates milestones
|
||||
|
|
@ -459,7 +459,7 @@ func (g *GiteaLocalUploader) CreateComments(comments ...*base.Comment) error {
|
|||
var issue *issues_model.Issue
|
||||
issue, ok := g.issues[comment.IssueIndex]
|
||||
if !ok {
|
||||
return fmt.Errorf("comment references non existent IssueIndex %d", comment.IssueIndex)
|
||||
return fmt.Errorf("comment[index=%d,poster_id=%d] belongs to a issue[index=%d] that was not migrated", comment.Index, comment.PosterID, comment.IssueIndex)
|
||||
}
|
||||
|
||||
if comment.Created.IsZero() {
|
||||
|
|
@ -566,7 +566,7 @@ func (g *GiteaLocalUploader) updateGitForPullRequest(pr *base.PullRequest) (head
|
|||
// SECURITY: this pr must have been must have been ensured safe
|
||||
if !pr.EnsuredSafe {
|
||||
log.Error("PR #%d in %s/%s has not been checked for safety.", pr.Number, g.repoOwner, g.repoName)
|
||||
return "", fmt.Errorf("the PR[%d] was not checked for safety", pr.Number)
|
||||
return "", fmt.Errorf("pull request[index=%d] was not checked for safety", pr.Number)
|
||||
}
|
||||
|
||||
// Anonymous function to download the patch file (allows us to use defer)
|
||||
|
|
@ -845,7 +845,7 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
|
|||
var issue *issues_model.Issue
|
||||
issue, ok := g.issues[review.IssueIndex]
|
||||
if !ok {
|
||||
return fmt.Errorf("review references non existent IssueIndex %d", review.IssueIndex)
|
||||
return fmt.Errorf("review[index=%d,reviewer_id=%d] belongs to a issue[index=%d] that was not migrated", review.ID, review.ReviewerID, review.IssueIndex)
|
||||
}
|
||||
if review.CreatedAt.IsZero() {
|
||||
review.CreatedAt = time.Unix(int64(issue.CreatedUnix), 0)
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ import (
|
|||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestCommentUpload(t *testing.T) {
|
||||
func TestUpload(t *testing.T) {
|
||||
unittest.PrepareTestEnv(t)
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
|
||||
var (
|
||||
|
|
@ -61,96 +61,116 @@ func TestCommentUpload(t *testing.T) {
|
|||
|
||||
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: user.ID, Name: repoName})
|
||||
|
||||
// Create and Test Issues Uploading
|
||||
issueA := &base.Issue{
|
||||
Title: "First issue",
|
||||
Number: 0,
|
||||
PosterID: 37243484,
|
||||
PosterName: "PatDyn",
|
||||
PosterEmail: "",
|
||||
Content: "Mock Content",
|
||||
Milestone: "Mock Milestone",
|
||||
State: "open",
|
||||
Created: time.Date(2025, 8, 7, 12, 44, 7, 0, time.UTC),
|
||||
Updated: time.Date(2025, 8, 7, 12, 44, 47, 0, time.UTC),
|
||||
Labels: nil,
|
||||
Reactions: nil,
|
||||
Closed: nil,
|
||||
IsLocked: false,
|
||||
Assignees: nil,
|
||||
ForeignIndex: 0,
|
||||
}
|
||||
t.Run("Topic", func(t *testing.T) {
|
||||
logChecker, cleanup := test.NewLogChecker(log.DEFAULT, log.INFO)
|
||||
logChecker.StopMark("Invalid topics: [verylongtopicthatistoolongforgejomigration a b c d e f] in migration to user1/test_repo")
|
||||
defer cleanup()
|
||||
|
||||
issueB := &base.Issue{
|
||||
Title: "Second Issue",
|
||||
Number: 1,
|
||||
PosterID: 37243484,
|
||||
PosterName: "PatDyn",
|
||||
PosterEmail: "",
|
||||
Content: "Mock Content",
|
||||
Milestone: "Mock Milestone",
|
||||
State: "open",
|
||||
Created: time.Date(2025, 8, 7, 12, 45, 44, 0, time.UTC),
|
||||
Updated: time.Date(2025, 8, 7, 13, 7, 25, 0, time.UTC),
|
||||
Labels: nil,
|
||||
Reactions: nil,
|
||||
Closed: nil,
|
||||
IsLocked: false,
|
||||
Assignees: nil,
|
||||
ForeignIndex: 1,
|
||||
}
|
||||
require.NoError(t, uploader.CreateTopics("verylongtopicthatistoolongforgejomigration", "go ", " security", "a b c d e f"))
|
||||
|
||||
err := uploader.CreateIssues(issueA, issueB)
|
||||
require.NoError(t, err)
|
||||
_, logStopped := logChecker.Check(time.Second)
|
||||
assert.True(t, logStopped)
|
||||
|
||||
issues, err := issues_model.Issues(db.DefaultContext, &issues_model.IssuesOptions{
|
||||
RepoIDs: []int64{repo.ID},
|
||||
IsPull: optional.Some(false),
|
||||
SortType: "newest",
|
||||
unittest.AssertExistsIf(t, false, &repo_model.Topic{Name: "verylongtopicthatistoolongforgejomigration"})
|
||||
unittest.AssertExistsIf(t, true, &repo_model.Topic{Name: "go"})
|
||||
unittest.AssertExistsIf(t, true, &repo_model.Topic{Name: "security"})
|
||||
unittest.AssertExistsIf(t, false, &repo_model.Topic{Name: "a b c d e f"})
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, issues, 2)
|
||||
|
||||
// Create and Test Comment Uploading
|
||||
issueAComment := &base.Comment{
|
||||
IssueIndex: 0,
|
||||
Index: 0,
|
||||
CommentType: "comment",
|
||||
PosterID: 37243484,
|
||||
PosterName: "PatDyn",
|
||||
PosterEmail: "",
|
||||
Created: time.Date(2025, 8, 7, 12, 44, 24, 0, time.UTC),
|
||||
Updated: time.Date(2025, 8, 7, 12, 44, 24, 0, time.UTC),
|
||||
Content: "First Mock Comment",
|
||||
Reactions: nil,
|
||||
Meta: nil,
|
||||
}
|
||||
issueBComment := &base.Comment{
|
||||
IssueIndex: 1,
|
||||
Index: 1,
|
||||
CommentType: "comment",
|
||||
PosterID: 37243484,
|
||||
PosterName: "PatDyn",
|
||||
PosterEmail: "",
|
||||
Created: time.Date(2025, 8, 7, 13, 7, 25, 0, time.UTC),
|
||||
Updated: time.Date(2025, 8, 7, 13, 7, 25, 0, time.UTC),
|
||||
Content: "Second Mock Comment",
|
||||
Reactions: nil,
|
||||
Meta: nil,
|
||||
}
|
||||
require.NoError(t, uploader.CreateComments(issueBComment, issueAComment))
|
||||
t.Run("Issue", func(t *testing.T) {
|
||||
// Create and Test Issues Uploading
|
||||
issueA := &base.Issue{
|
||||
Title: "First issue",
|
||||
Number: 0,
|
||||
PosterID: 37243484,
|
||||
PosterName: "PatDyn",
|
||||
PosterEmail: "",
|
||||
Content: "Mock Content",
|
||||
Milestone: "Mock Milestone",
|
||||
State: "open",
|
||||
Created: time.Date(2025, 8, 7, 12, 44, 7, 0, time.UTC),
|
||||
Updated: time.Date(2025, 8, 7, 12, 44, 47, 0, time.UTC),
|
||||
Labels: nil,
|
||||
Reactions: nil,
|
||||
Closed: nil,
|
||||
IsLocked: false,
|
||||
Assignees: nil,
|
||||
ForeignIndex: 0,
|
||||
}
|
||||
|
||||
issues, err = issues_model.Issues(db.DefaultContext, &issues_model.IssuesOptions{
|
||||
RepoIDs: []int64{repo.ID},
|
||||
IsPull: optional.Some(false),
|
||||
SortType: "newest",
|
||||
issueB := &base.Issue{
|
||||
Title: "Second Issue",
|
||||
Number: 1,
|
||||
PosterID: 37243484,
|
||||
PosterName: "PatDyn",
|
||||
PosterEmail: "",
|
||||
Content: "Mock Content",
|
||||
Milestone: "Mock Milestone",
|
||||
State: "open",
|
||||
Created: time.Date(2025, 8, 7, 12, 45, 44, 0, time.UTC),
|
||||
Updated: time.Date(2025, 8, 7, 13, 7, 25, 0, time.UTC),
|
||||
Labels: nil,
|
||||
Reactions: nil,
|
||||
Closed: nil,
|
||||
IsLocked: false,
|
||||
Assignees: nil,
|
||||
ForeignIndex: 1,
|
||||
}
|
||||
|
||||
err := uploader.CreateIssues(issueA, issueB)
|
||||
require.NoError(t, err)
|
||||
|
||||
issues, err := issues_model.Issues(db.DefaultContext, &issues_model.IssuesOptions{
|
||||
RepoIDs: []int64{repo.ID},
|
||||
IsPull: optional.Some(false),
|
||||
SortType: "newest",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, issues, 2)
|
||||
})
|
||||
|
||||
t.Run("Comments", func(t *testing.T) {
|
||||
// Create and Test Comment Uploading
|
||||
issueAComment := &base.Comment{
|
||||
IssueIndex: 0,
|
||||
Index: 0,
|
||||
CommentType: "comment",
|
||||
PosterID: 37243484,
|
||||
PosterName: "PatDyn",
|
||||
PosterEmail: "",
|
||||
Created: time.Date(2025, 8, 7, 12, 44, 24, 0, time.UTC),
|
||||
Updated: time.Date(2025, 8, 7, 12, 44, 24, 0, time.UTC),
|
||||
Content: "First Mock Comment",
|
||||
Reactions: nil,
|
||||
Meta: nil,
|
||||
}
|
||||
issueBComment := &base.Comment{
|
||||
IssueIndex: 1,
|
||||
Index: 1,
|
||||
CommentType: "comment",
|
||||
PosterID: 37243484,
|
||||
PosterName: "PatDyn",
|
||||
PosterEmail: "",
|
||||
Created: time.Date(2025, 8, 7, 13, 7, 25, 0, time.UTC),
|
||||
Updated: time.Date(2025, 8, 7, 13, 7, 25, 0, time.UTC),
|
||||
Content: "Second Mock Comment",
|
||||
Reactions: nil,
|
||||
Meta: nil,
|
||||
}
|
||||
require.NoError(t, uploader.CreateComments(issueBComment, issueAComment))
|
||||
|
||||
issues, err := issues_model.Issues(db.DefaultContext, &issues_model.IssuesOptions{
|
||||
RepoIDs: []int64{repo.ID},
|
||||
IsPull: optional.Some(false),
|
||||
SortType: "newest",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, issues, 2)
|
||||
require.NoError(t, issues[0].LoadDiscussComments(db.DefaultContext))
|
||||
require.NoError(t, issues[1].LoadDiscussComments(db.DefaultContext))
|
||||
assert.Len(t, issues[0].Comments, 1)
|
||||
assert.Len(t, issues[1].Comments, 1)
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, issues, 2)
|
||||
require.NoError(t, issues[0].LoadDiscussComments(db.DefaultContext))
|
||||
require.NoError(t, issues[1].LoadDiscussComments(db.DefaultContext))
|
||||
assert.Len(t, issues[0].Comments, 1)
|
||||
assert.Len(t, issues[1].Comments, 1)
|
||||
}
|
||||
|
||||
func TestGiteaUploadRepo(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue