Skip to content

Fix wrong xorm get usage on migration #27111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 18, 2023
Merged

Fix wrong xorm get usage on migration #27111

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6239288
Fix wrong xomr get usage on migration
lunny Sep 18, 2023
adbd89a
Update models/migrations/v1_21/v276.go
lunny Sep 18, 2023
5ee5025
Fix error
lunny Sep 18, 2023
2ac38f6
Add error track
lunny Sep 18, 2023
5c63864
Merge branch 'main' into lunny/fix_migration_get
jolheiser Sep 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 14 additions & 19 deletions models/migrations/v1_21/v276.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ func migratePullMirrors(x *xorm.Engine) error {
ID int64 `xorm:"pk autoincr"`
RepoID int64 `xorm:"INDEX"`
RemoteAddress string `xorm:"VARCHAR(2048)"`
RepoOwner string
RepoName string
}

sess := x.NewSession()
Expand All @@ -59,7 +61,9 @@ func migratePullMirrors(x *xorm.Engine) error {

for {
var mirrors []Mirror
if err := sess.Limit(limit, start).Find(&mirrors); err != nil {
if err := sess.Select("mirror.id, mirror.repo_id, mirror.remote_address, repository.owner_name as repo_owner, repository.name as repo_name").
Join("INNER", "repository", "repository.id = mirror.repo_id").
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Join("INNER", "repository", "repository.id = mirror.repo_id").
Join("INNER", "repository", "repository.id = mirror.repo_id").
Where("mirror.remote_address IS NULL")

Limit(limit, start).Find(&mirrors); err != nil {
return err
}

Expand All @@ -69,7 +73,7 @@ func migratePullMirrors(x *xorm.Engine) error {
start += len(mirrors)

for _, m := range mirrors {
remoteAddress, err := getRemoteAddress(sess, m.RepoID, "origin")
remoteAddress, err := getRemoteAddress(m.RepoOwner, m.RepoName, "origin")
if err != nil {
return err
}
Expand Down Expand Up @@ -100,6 +104,8 @@ func migratePushMirrors(x *xorm.Engine) error {
RepoID int64 `xorm:"INDEX"`
RemoteName string
RemoteAddress string `xorm:"VARCHAR(2048)"`
RepoOwner string
RepoName string
}

sess := x.NewSession()
Expand All @@ -118,7 +124,9 @@ func migratePushMirrors(x *xorm.Engine) error {

for {
var mirrors []PushMirror
if err := sess.Limit(limit, start).Find(&mirrors); err != nil {
if err := sess.Select("push_mirror.id, push_mirror.repo_id, push_mirror.remote_name, push_mirror.remote_address, repository.owner_name as repo_owner, repository.name as repo_name").
Join("INNER", "repository", "repository.id = push_mirror.repo_id").
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are already at it, can we also use

Suggested change
Join("INNER", "repository", "repository.id = push_mirror.repo_id").
Join("INNER", "repository", "repository.id = push_mirror.repo_id").
Where("push_mirror.remote_address IS NULL")

?
That way, the migration at least becomes symmetric.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since remote_address is not an index column and the second update will not affect anything, I don't think it's necessary for this change.

Limit(limit, start).Find(&mirrors); err != nil {
return err
}

Expand All @@ -128,7 +136,7 @@ func migratePushMirrors(x *xorm.Engine) error {
start += len(mirrors)

for _, m := range mirrors {
remoteAddress, err := getRemoteAddress(sess, m.RepoID, m.RemoteName)
remoteAddress, err := getRemoteAddress(m.RepoOwner, m.RepoName, m.RemoteName)
if err != nil {
return err
}
Expand All @@ -153,25 +161,12 @@ func migratePushMirrors(x *xorm.Engine) error {
return sess.Commit()
}

func getRemoteAddress(sess *xorm.Session, repoID int64, remoteName string) (string, error) {
var ownerName string
var repoName string
has, err := sess.
Table("repository").
Cols("owner_name", "lower_name").
Where("id=?", repoID).
Get(&ownerName, &repoName)
if err != nil {
return "", err
} else if !has {
return "", fmt.Errorf("repository [%v] not found", repoID)
}

func getRemoteAddress(ownerName, repoName, remoteName string) (string, error) {
repoPath := filepath.Join(setting.RepoRootPath, strings.ToLower(ownerName), strings.ToLower(repoName)+".git")

remoteURL, err := git.GetRemoteAddress(context.Background(), repoPath, remoteName)
if err != nil {
return "", err
return "", fmt.Errorf("get remote %s's address of %s/%s failed: %v", remoteName, ownerName, repoName, err)
}

u, err := giturl.Parse(remoteURL)
Expand Down