Merge pull request #9859 from ywk253100/191113_subresource_1.10

Refine the implementation of replication execution API

src/core/api/replication_execution.go

@@ -30,6 +30,8 @@ import (
 // ReplicationOperationAPI handles the replication operation requests
 type ReplicationOperationAPI struct {
 	BaseController
+	execution *models.Execution
+	task      *models.Task
 }
 
 // Prepare ...
@@ -45,6 +47,46 @@ func (r *ReplicationOperationAPI) Prepare() {
 		r.SendForbiddenError(errors.New(r.SecurityCtx.GetUsername()))
 		return
 	}
+
+	// check the existence of execution if execution ID is provided in the request path
+	executionIDStr := r.GetStringFromPath(":id")
+	if len(executionIDStr) > 0 {
+		executionID, err := strconv.ParseInt(executionIDStr, 10, 64)
+		if err != nil || executionID <= 0 {
+			r.SendBadRequestError(fmt.Errorf("invalid execution ID: %s", executionIDStr))
+			return
+		}
+		execution, err := replication.OperationCtl.GetExecution(executionID)
+		if err != nil {
+			r.SendInternalServerError(fmt.Errorf("failed to get execution %d: %v", executionID, err))
+			return
+		}
+		if execution == nil {
+			r.SendNotFoundError(fmt.Errorf("execution %d not found", executionID))
+			return
+		}
+		r.execution = execution
+
+		// check the existence of task if task ID is provided in the request path
+		taskIDStr := r.GetStringFromPath(":tid")
+		if len(taskIDStr) > 0 {
+			taskID, err := strconv.ParseInt(taskIDStr, 10, 64)
+			if err != nil || taskID <= 0 {
+				r.SendBadRequestError(fmt.Errorf("invalid task ID: %s", taskIDStr))
+				return
+			}
+			task, err := replication.OperationCtl.GetTask(taskID)
+			if err != nil {
+				r.SendInternalServerError(fmt.Errorf("failed to get task %d: %v", taskID, err))
+				return
+			}
+			if task == nil || task.ExecutionID != executionID {
+				r.SendNotFoundError(fmt.Errorf("task %d not found", taskID))
+				return
+			}
+			r.task = task
+		}
+	}
 }
 
 // The API is open only for system admin currently, we can use
@@ -146,68 +188,21 @@ func (r *ReplicationOperationAPI) CreateExecution() {
 
 // GetExecution gets one execution of the replication
 func (r *ReplicationOperationAPI) GetExecution() {
-	executionID, err := r.GetInt64FromPath(":id")
+	r.WriteJSONData(r.execution)
-	if err != nil || executionID <= 0 {
-		r.SendBadRequestError(errors.New("invalid execution ID"))
-		return
-	}
-	execution, err := replication.OperationCtl.GetExecution(executionID)
-	if err != nil {
-		r.SendInternalServerError(fmt.Errorf("failed to get execution %d: %v", executionID, err))
-		return
-	}
-
-	if execution == nil {
-		r.SendNotFoundError(fmt.Errorf("execution %d not found", executionID))
-		return
-	}
-	r.WriteJSONData(execution)
 }
 
 // StopExecution stops one execution of the replication
 func (r *ReplicationOperationAPI) StopExecution() {
-	executionID, err := r.GetInt64FromPath(":id")
+	if err := replication.OperationCtl.StopReplication(r.execution.ID); err != nil {
-	if err != nil || executionID <= 0 {
+		r.SendInternalServerError(fmt.Errorf("failed to stop execution %d: %v", r.execution.ID, err))
-		r.SendBadRequestError(errors.New("invalid execution ID"))
-		return
-	}
-	execution, err := replication.OperationCtl.GetExecution(executionID)
-	if err != nil {
-		r.SendInternalServerError(fmt.Errorf("failed to get execution %d: %v", executionID, err))
-		return
-	}
-
-	if execution == nil {
-		r.SendNotFoundError(fmt.Errorf("execution %d not found", executionID))
-		return
-	}
-
-	if err := replication.OperationCtl.StopReplication(executionID); err != nil {
-		r.SendInternalServerError(fmt.Errorf("failed to stop execution %d: %v", executionID, err))
 		return
 	}
 }
 
 // ListTasks ...
 func (r *ReplicationOperationAPI) ListTasks() {
-	executionID, err := r.GetInt64FromPath(":id")
-	if err != nil || executionID <= 0 {
-		r.SendBadRequestError(errors.New("invalid execution ID"))
-		return
-	}
-
-	execution, err := replication.OperationCtl.GetExecution(executionID)
-	if err != nil {
-		r.SendInternalServerError(fmt.Errorf("failed to get execution %d: %v", executionID, err))
-		return
-	}
-	if execution == nil {
-		r.SendNotFoundError(fmt.Errorf("execution %d not found", executionID))
-		return
-	}
-
 	query := &models.TaskQuery{
-		ExecutionID:  executionID,
+		ExecutionID:  r.execution.ID,
 		ResourceType: r.GetString("resource_type"),
 	}
 	status := r.GetString("status")
@@ -232,53 +227,22 @@ func (r *ReplicationOperationAPI) ListTasks() {
 
 // GetTaskLog ...
 func (r *ReplicationOperationAPI) GetTaskLog() {
-	executionID, err := r.GetInt64FromPath(":id")
+	logBytes, err := replication.OperationCtl.GetTaskLog(r.task.ID)
-	if err != nil || executionID <= 0 {
-		r.SendBadRequestError(errors.New("invalid execution ID"))
-		return
-	}
-
-	execution, err := replication.OperationCtl.GetExecution(executionID)
-	if err != nil {
-		r.SendInternalServerError(fmt.Errorf("failed to get execution %d: %v", executionID, err))
-		return
-	}
-	if execution == nil {
-		r.SendNotFoundError(fmt.Errorf("execution %d not found", executionID))
-		return
-	}
-
-	taskID, err := r.GetInt64FromPath(":tid")
-	if err != nil || taskID <= 0 {
-		r.SendBadRequestError(errors.New("invalid task ID"))
-		return
-	}
-	task, err := replication.OperationCtl.GetTask(taskID)
-	if err != nil {
-		r.SendInternalServerError(fmt.Errorf("failed to get task %d: %v", taskID, err))
-		return
-	}
-	if task == nil {
-		r.SendNotFoundError(fmt.Errorf("task %d not found", taskID))
-		return
-	}
-
-	logBytes, err := replication.OperationCtl.GetTaskLog(taskID)
 	if err != nil {
 		if httpErr, ok := err.(*common_http.Error); ok {
 			if ok && httpErr.Code == http.StatusNotFound {
-				r.SendNotFoundError(fmt.Errorf("the log of task %d not found", taskID))
+				r.SendNotFoundError(fmt.Errorf("the log of task %d not found", r.task.ID))
 				return
 			}
 		}
-		r.SendInternalServerError(fmt.Errorf("failed to get log of task %d: %v", taskID, err))
+		r.SendInternalServerError(fmt.Errorf("failed to get log of task %d: %v", r.task.ID, err))
 		return
 	}
 	r.Ctx.ResponseWriter.Header().Set(http.CanonicalHeaderKey("Content-Length"), strconv.Itoa(len(logBytes)))
 	r.Ctx.ResponseWriter.Header().Set(http.CanonicalHeaderKey("Content-Type"), "text/plain")
 	_, err = r.Ctx.ResponseWriter.Write(logBytes)
 	if err != nil {
-		r.SendInternalServerError(fmt.Errorf("failed to write log of task %d: %v", taskID, err))
+		r.SendInternalServerError(fmt.Errorf("failed to write log of task %d: %v", r.task.ID, err))
 		return
 	}
 }

src/core/api/replication_execution_test.go

@@ -19,7 +19,6 @@ import (
 	"testing"
 
 	"github.com/goharbor/harbor/src/replication"
-
 	"github.com/goharbor/harbor/src/replication/dao/models"
 	"github.com/goharbor/harbor/src/replication/model"
 )
@@ -50,7 +49,7 @@ func (f *fakedOperationController) GetExecution(id int64) (*models.Execution, er
 	return nil, nil
 }
 func (f *fakedOperationController) ListTasks(...*models.TaskQuery) (int64, []*models.Task, error) {
-	return 1, []*models.Task{
+	return 2, []*models.Task{
 		{
 			ID:          1,
 			ExecutionID: 1,
@@ -64,6 +63,12 @@ func (f *fakedOperationController) GetTask(id int64) (*models.Task, error) {
 			ExecutionID: 1,
 		}, nil
 	}
+	if id == 3 {
+		return &models.Task{
+			ID:          3,
+			ExecutionID: 2,
+		}, nil
+	}
 	return nil, nil
 }
 func (f *fakedOperationController) UpdateTaskStatus(id int64, status string, statusRevision int64, statusCondition ...string) error {
@@ -414,6 +419,15 @@ func TestGetTaskLog(t *testing.T) {
 			},
 			code: http.StatusNotFound,
 		},
+		// 404, task not found
+		{
+			request: &testingRequest{
+				method:     http.MethodGet,
+				url:        "/api/replication/executions/1/tasks/3/log",
+				credential: sysAdmin,
+			},
+			code: http.StatusNotFound,
+		},
 		// 200
 		{
 			request: &testingRequest{