Make repo migration cancelable and fix various bugs (#24605)

Replace #12917

Close #24601
Close #12845


![image](https://github.com/go-gitea/gitea/assets/2114189/39378118-064d-40fb-8396-4579ebf33917)


![image](https://github.com/go-gitea/gitea/assets/2114189/faf37418-191c-46a6-90a8-353141e00e2d)


![image](https://github.com/go-gitea/gitea/assets/2114189/fdc8ee4d-125f-4737-9990-89bcdf9eb388)


![image](https://github.com/go-gitea/gitea/assets/2114189/9a3bd2c2-fe20-4011-81f0-990ed869d139)

---------

Co-authored-by: Yarden Shoham <git@yardenshoham.com>
Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: Giteabot <teabot@gitea.io>

models/admin/task.go

@@ -17,8 +17,6 @@ import (
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/util"
-
-	"xorm.io/builder"
 )
 
 // Task represents a task
@@ -35,7 +33,7 @@ type Task struct {
 	StartTime      timeutil.TimeStamp
 	EndTime        timeutil.TimeStamp
 	PayloadContent string             `xorm:"TEXT"`
-	Message        string             `xorm:"TEXT"` // if task failed, saved the error reason
+	Message        string             `xorm:"TEXT"` // if task failed, saved the error reason, it could be a JSON string of TranslatableMessage or a plain message
 	Created        timeutil.TimeStamp `xorm:"created"`
 }
 
@@ -185,14 +183,6 @@ func GetMigratingTask(repoID int64) (*Task, error) {
 	return &task, nil
 }
 
-// HasFinishedMigratingTask returns if a finished migration task exists for the repo.
-func HasFinishedMigratingTask(repoID int64) (bool, error) {
-	return db.GetEngine(db.DefaultContext).
-		Where("repo_id=? AND type=? AND status=?", repoID, structs.TaskTypeMigrateRepo, structs.TaskStatusFinished).
-		Table("task").
-		Exist()
-}
-
 // GetMigratingTaskByID returns the migrating task by repo's id
 func GetMigratingTaskByID(id, doerID int64) (*Task, *migration.MigrateOptions, error) {
 	task := Task{
@@ -214,27 +204,6 @@ func GetMigratingTaskByID(id, doerID int64) (*Task, *migration.MigrateOptions, e
 	return &task, &opts, nil
 }
 
-// FindTaskOptions find all tasks
-type FindTaskOptions struct {
-	Status int
-}
-
-// ToConds generates conditions for database operation.
-func (opts FindTaskOptions) ToConds() builder.Cond {
-	cond := builder.NewCond()
-	if opts.Status >= 0 {
-		cond = cond.And(builder.Eq{"status": opts.Status})
-	}
-	return cond
-}
-
-// FindTasks find all tasks
-func FindTasks(opts FindTaskOptions) ([]*Task, error) {
-	tasks := make([]*Task, 0, 10)
-	err := db.GetEngine(db.DefaultContext).Where(opts.ToConds()).Find(&tasks)
-	return tasks, err
-}
-
 // CreateTask creates a task on database
 func CreateTask(task *Task) error {
 	return db.Insert(db.DefaultContext, task)

modules/structs/task.go

@@ -6,10 +6,7 @@ package structs
 // TaskType defines task type
 type TaskType int
 
-// all kinds of task types
+const TaskTypeMigrateRepo TaskType = iota // migrate repository from external or local disk
-const (
-	TaskTypeMigrateRepo TaskType = iota // migrate repository from external or local disk
-)
 
 // Name returns the task type name
 func (taskType TaskType) Name() string {
@@ -25,9 +22,9 @@ type TaskStatus int
 
 // enumerate all the kinds of task status
 const (
-	TaskStatusQueue    TaskStatus = iota // 0 task is queue
+	TaskStatusQueued   TaskStatus = iota // 0 task is queued
 	TaskStatusRunning                    // 1 task is running
-	TaskStatusStopped                    // 2 task is stopped
+	TaskStatusStopped                    // 2 task is stopped (never used)
 	TaskStatusFailed                     // 3 task is failed
 	TaskStatusFinished                   // 4 task is finished
 )

options/locale/locale_en-US.ini

@@ -1038,7 +1038,7 @@ migrated_from_fake = Migrated From %[1]s
 migrate.migrate = Migrate From %s
 migrate.migrating = Migrating from <b>%s</b> ...
 migrate.migrating_failed = Migrating from <b>%s</b> failed.
-migrate.migrating_failed.error = Error: %s
+migrate.migrating_failed.error = Failed to migrate: %s
 migrate.migrating_failed_no_addr = Migration failed.
 migrate.github.description = Migrate data from github.com or other GitHub instances.
 migrate.git.description = Migrate a repository only from any Git service.
@@ -1055,6 +1055,8 @@ migrate.migrating_labels = Migrating Labels
 migrate.migrating_releases = Migrating Releases
 migrate.migrating_issues = Migrating Issues
 migrate.migrating_pulls = Migrating Pull Requests
+migrate.cancel_migrating_title = Cancel Migration
+migrate.cancel_migrating_confirm = Do you want to cancel this migration?
 
 mirror_from = mirror of
 forked_from = forked from

routers/web/repo/migrate.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"code.gitea.io/gitea/models"
+	admin_model "code.gitea.io/gitea/models/admin"
 	"code.gitea.io/gitea/models/db"
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
@@ -257,3 +258,20 @@ func setMigrationContextData(ctx *context.Context, serviceType structs.GitServic
 	ctx.Data["Services"] = append([]structs.GitServiceType{structs.PlainGitService}, structs.SupportedFullGitService...)
 	ctx.Data["service"] = serviceType
 }
+
+func MigrateCancelPost(ctx *context.Context) {
+	migratingTask, err := admin_model.GetMigratingTask(ctx.Repo.Repository.ID)
+	if err != nil {
+		log.Error("GetMigratingTask: %v", err)
+		ctx.Redirect(ctx.Repo.Repository.Link())
+		return
+	}
+	if migratingTask.Status == structs.TaskStatusRunning {
+		taskUpdate := &admin_model.Task{ID: migratingTask.ID, Status: structs.TaskStatusFailed, Message: "canceled"}
+		if err = taskUpdate.UpdateCols("status", "message"); err != nil {
+			ctx.ServerError("task.UpdateCols", err)
+			return
+		}
+	}
+	ctx.Redirect(ctx.Repo.Repository.Link())
+}

routers/web/web.go

@@ -939,6 +939,7 @@ func registerRoutes(m *web.Route) {
 				addSettingsRunnersRoutes()
 				addSettingsSecretsRoutes()
 			}, actions.MustEnableActions)
+			m.Post("/migrate/cancel", repo.MigrateCancelPost) // this handler must be under "settings", otherwise this incomplete repo can't be accessed
 		}, ctxDataSet("PageIsRepoSettings", true, "LFSStartServer", setting.LFS.StartServer))
 	}, reqSignIn, context.RepoAssignment, context.UnitTypes(), reqRepoAdmin, context.RepoRef())
 

services/migrations/gitea_uploader.go

@@ -923,9 +923,8 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
 func (g *GiteaLocalUploader) Rollback() error {
 	if g.repo != nil && g.repo.ID > 0 {
 		g.gitRepo.Close()
-		if err := models.DeleteRepository(g.doer, g.repo.OwnerID, g.repo.ID); err != nil {
+
-			return err
+		// do not delete the repository, otherwise the end users won't be able to see the last error message
-		}
 	}
 	return nil
 }

services/task/migrate.go

@@ -7,8 +7,8 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"time"
 
-	"code.gitea.io/gitea/models"
 	admin_model "code.gitea.io/gitea/models/admin"
 	"code.gitea.io/gitea/models/db"
 	repo_model "code.gitea.io/gitea/models/repo"
@@ -28,13 +28,13 @@ import (
 func handleCreateError(owner *user_model.User, err error) error {
 	switch {
 	case repo_model.IsErrReachLimitOfRepo(err):
-		return fmt.Errorf("You have already reached your limit of %d repositories", owner.MaxCreationLimit())
+		return fmt.Errorf("you have already reached your limit of %d repositories", owner.MaxCreationLimit())
 	case repo_model.IsErrRepoAlreadyExist(err):
-		return errors.New("The repository name is already used")
+		return errors.New("the repository name is already used")
 	case db.IsErrNameReserved(err):
-		return fmt.Errorf("The repository name '%s' is reserved", err.(db.ErrNameReserved).Name)
+		return fmt.Errorf("the repository name '%s' is reserved", err.(db.ErrNameReserved).Name)
 	case db.IsErrNamePatternNotAllowed(err):
-		return fmt.Errorf("The pattern '%s' is not allowed in a repository name", err.(db.ErrNamePatternNotAllowed).Pattern)
+		return fmt.Errorf("the pattern '%s' is not allowed in a repository name", err.(db.ErrNamePatternNotAllowed).Pattern)
 	default:
 		return err
 	}
@@ -57,22 +57,17 @@ func runMigrateTask(t *admin_model.Task) (err error) {
 			log.Error("FinishMigrateTask[%d] by DoerID[%d] to RepoID[%d] for OwnerID[%d] failed: %v", t.ID, t.DoerID, t.RepoID, t.OwnerID, err)
 		}
 
+		log.Error("runMigrateTask[%d] by DoerID[%d] to RepoID[%d] for OwnerID[%d] failed: %v", t.ID, t.DoerID, t.RepoID, t.OwnerID, err)
+
 		t.EndTime = timeutil.TimeStampNow()
 		t.Status = structs.TaskStatusFailed
 		t.Message = err.Error()
-		// Ensure that the repo loaded before we zero out the repo ID from the task - thus ensuring that we can delete it
-		_ = t.LoadRepo()
 
-		t.RepoID = 0
+		if err := t.UpdateCols("status", "message", "end_time"); err != nil {
-		if err := t.UpdateCols("status", "errors", "repo_id", "end_time"); err != nil {
 			log.Error("Task UpdateCols failed: %v", err)
 		}
 
-		if t.Repo != nil {
+		// then, do not delete the repository, otherwise the users won't be able to see the last error
-			if errDelete := models.DeleteRepository(t.Doer, t.OwnerID, t.Repo.ID); errDelete != nil {
-				log.Error("DeleteRepository: %v", errDelete)
-			}
-		}
 	}()
 
 	if err = t.LoadRepo(); err != nil {
@@ -100,7 +95,7 @@ func runMigrateTask(t *admin_model.Task) (err error) {
 	opts.MigrateToRepoID = t.RepoID
 
 	pm := process.GetManager()
-	ctx, _, finished := pm.AddContext(graceful.GetManager().ShutdownContext(), fmt.Sprintf("MigrateTask: %s/%s", t.Owner.Name, opts.RepoName))
+	ctx, cancel, finished := pm.AddContext(graceful.GetManager().ShutdownContext(), fmt.Sprintf("MigrateTask: %s/%s", t.Owner.Name, opts.RepoName))
 	defer finished()
 
 	t.StartTime = timeutil.TimeStampNow()
@@ -109,6 +104,23 @@ func runMigrateTask(t *admin_model.Task) (err error) {
 		return
 	}
 
+	// check whether the task should be canceled, this goroutine is also managed by process manager
+	go func() {
+		for {
+			select {
+			case <-time.After(2 * time.Second):
+			case <-ctx.Done():
+				return
+			}
+			task, _ := admin_model.GetMigratingTask(t.RepoID)
+			if task != nil && task.Status != structs.TaskStatusRunning {
+				log.Debug("MigrateTask[%d] by DoerID[%d] to RepoID[%d] for OwnerID[%d] is canceled due to status is not 'running'", t.ID, t.DoerID, t.RepoID, t.OwnerID)
+				cancel()
+				return
+			}
+		}
+	}()
+
 	t.Repo, err = migrations.MigrateRepository(ctx, t.Doer, t.Owner.Name, *opts, func(format string, args ...interface{}) {
 		message := admin_model.TranslatableMessage{
 			Format: format,
@@ -118,13 +130,14 @@ func runMigrateTask(t *admin_model.Task) (err error) {
 		t.Message = string(bs)
 		_ = t.UpdateCols("message")
 	})
+
 	if err == nil {
 		log.Trace("Repository migrated [%d]: %s/%s", t.Repo.ID, t.Owner.Name, t.Repo.Name)
 		return
 	}
 
 	if repo_model.IsErrRepoAlreadyExist(err) {
-		err = errors.New("The repository name is already used")
+		err = errors.New("the repository name is already used")
 		return
 	}
 
@@ -132,9 +145,9 @@ func runMigrateTask(t *admin_model.Task) (err error) {
 	err = util.SanitizeErrorCredentialURLs(err)
 	if strings.Contains(err.Error(), "Authentication failed") ||
 		strings.Contains(err.Error(), "could not read Username") {
-		return fmt.Errorf("Authentication failed: %w", err)
+		return fmt.Errorf("authentication failed: %w", err)
 	} else if strings.Contains(err.Error(), "fatal:") {
-		return fmt.Errorf("Migration failed: %w", err)
+		return fmt.Errorf("migration failed: %w", err)
 	}
 
 	// do not be tempted to coalesce this line with the return

services/task/task.go

@@ -95,7 +95,7 @@ func CreateMigrateTask(doer, u *user_model.User, opts base.MigrateOptions) (*adm
 		DoerID:         doer.ID,
 		OwnerID:        u.ID,
 		Type:           structs.TaskTypeMigrateRepo,
-		Status:         structs.TaskStatusQueue,
+		Status:         structs.TaskStatusQueued,
 		PayloadContent: string(bs),
 	}
 

templates/repo/migrate/migrating.tmpl

@@ -7,7 +7,7 @@
 				{{template "base/alert" .}}
 				<div class="home">
 					<div class="ui stackable middle very relaxed page grid">
-						<div id="repo_migrating" class="sixteen wide center aligned centered column" task="{{.MigrateTask.ID}}">
+						<div id="repo_migrating" class="sixteen wide center aligned centered column" data-migrating-task-id="{{.MigrateTask.ID}}">
 							<div>
 								<img src="{{AssetUrlPrefix}}/img/loading.png">
 							</div>
@@ -32,10 +32,14 @@
 								{{end}}
 								<p id="repo_migrating_failed_error"></p>
 							</div>
-							{{if and .Failed .Permission.IsAdmin}}
+							{{if .Permission.IsAdmin}}
 								<div class="ui divider"></div>
 								<div class="item">
+									{{if .Failed}}
 									<button class="ui basic red show-modal button" data-modal="#delete-repo-modal">{{.locale.Tr "repo.settings.delete"}}</button>
+									{{else}}
+									<button class="ui basic red show-modal button" data-modal="#cancel-repo-modal">{{.locale.Tr "cancel"}}</button>
+									{{end}}
 								</div>
 							{{end}}
 						</div>
@@ -45,6 +49,7 @@
 		</div>
 	</div>
 </div>
+
 <div class="ui small modal" id="delete-repo-modal">
 	<div class="header">
 		{{.locale.Tr "repo.settings.delete"}}
@@ -78,4 +83,18 @@
 		</form>
 	</div>
 </div>
+
+<div class="ui g-modal-confirm modal" id="cancel-repo-modal">
+	<div class="header">
+		{{.locale.Tr "repo.migrate.cancel_migrating_title"}}
+	</div>
+	<form action="{{.Link}}/settings/migrate/cancel" method="post">
+		{{.CsrfTokenHtml}}
+		<div class="content">
+			{{.locale.Tr "repo.migrate.cancel_migrating_confirm"}}
+		</div>
+		{{template "base/modal_actions_confirm" .}}
+	</form>
+</div>
+
 {{template "base/footer" .}}

web_src/js/features/repo-migrate.js

@@ -1,51 +1,55 @@
 import $ from 'jquery';
 import {hideElem, showElem} from '../utils/dom.js';
 
-const {appSubUrl, csrfToken} = window.config;
+const {appSubUrl} = window.config;
 
 export function initRepoMigrationStatusChecker() {
-  const migrating = $('#repo_migrating');
+  const $repoMigrating = $('#repo_migrating');
-  hideElem($('#repo_migrating_failed'));
+  if (!$repoMigrating.length) return;
-  hideElem($('#repo_migrating_failed_image'));
+
-  hideElem($('#repo_migrating_progress_message'));
+  const task = $repoMigrating.attr('data-migrating-task-id');
-  if (migrating) {
+
-    const task = migrating.attr('task');
+  // returns true if the refresh still need to be called after a while
-    if (task === undefined) {
+  const refresh = async () => {
-      return;
+    const res = await fetch(`${appSubUrl}/user/task/${task}`);
+    if (res.status !== 200) return true; // continue to refresh if network error occurs
+
+    const data = await res.json();
+
+    // for all status
+    if (data.message) {
+      $('#repo_migrating_progress_message').text(data.message);
     }
-    $.ajax({
+
-      type: 'GET',
+    // TaskStatusFinished
-      url: `${appSubUrl}/user/task/${task}`,
+    if (data.status === 4) {
-      data: {
-        _csrf: csrfToken,
-      },
-      complete(xhr) {
-        if (xhr.status === 200 && xhr.responseJSON) {
-          if (xhr.responseJSON.status === 4) {
       window.location.reload();
-            return;
+      return false;
-          } else if (xhr.responseJSON.status === 3) {
-            hideElem($('#repo_migrating_progress'));
-            hideElem($('#repo_migrating'));
-            showElem($('#repo_migrating_failed'));
-            showElem($('#repo_migrating_failed_image'));
-            $('#repo_migrating_failed_error').text(xhr.responseJSON.message);
-            return;
     }
-          if (xhr.responseJSON.message) {
+
-            showElem($('#repo_migrating_progress_message'));
+    // TaskStatusFailed
-            $('#repo_migrating_progress_message').text(xhr.responseJSON.message);
+    if (data.status === 3) {
-          }
+      hideElem('#repo_migrating_progress');
-          setTimeout(() => {
+      hideElem('#repo_migrating');
-            initRepoMigrationStatusChecker();
+      showElem('#repo_migrating_failed');
-          }, 2000);
+      showElem('#repo_migrating_failed_image');
-          return;
+      $('#repo_migrating_failed_error').text(data.message);
+      return false;
     }
-        hideElem($('#repo_migrating_progress'));
+
-        hideElem($('#repo_migrating'));
+    return true; // continue to refresh
-        showElem($('#repo_migrating_failed'));
+  };
-        showElem($('#repo_migrating_failed_image'));
+
+  const syncTaskStatus = async () => {
+    let doNextRefresh = true;
+    try {
+      doNextRefresh = await refresh();
+    } finally {
+      if (doNextRefresh) {
+        setTimeout(syncTaskStatus, 2000);
       }
-    });
     }
+  };
+
+  syncTaskStatus(); // no await
 }