From 05ffb7a3c5b070fa02e509f163b546a80e907385 Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Wed, 13 Nov 2019 13:28:23 +0800 Subject: [PATCH] Refine the implementation of replication execution API Remove the duplicated code in replication execution API Signed-off-by: Wenkai Yin --- src/core/api/replication_execution.go | 136 ++++++++------------- src/core/api/replication_execution_test.go | 18 ++- 2 files changed, 66 insertions(+), 88 deletions(-) diff --git a/src/core/api/replication_execution.go b/src/core/api/replication_execution.go index 440b7c38c..2719cdafa 100644 --- a/src/core/api/replication_execution.go +++ b/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") - 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) + r.WriteJSONData(r.execution) } // StopExecution stops one execution of the replication func (r *ReplicationOperationAPI) StopExecution() { - 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 - } - - if err := replication.OperationCtl.StopReplication(executionID); err != nil { - r.SendInternalServerError(fmt.Errorf("failed to stop execution %d: %v", executionID, err)) + if err := replication.OperationCtl.StopReplication(r.execution.ID); err != nil { + r.SendInternalServerError(fmt.Errorf("failed to stop execution %d: %v", r.execution.ID, 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") - 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) + logBytes, err := replication.OperationCtl.GetTaskLog(r.task.ID) 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 } } diff --git a/src/core/api/replication_execution_test.go b/src/core/api/replication_execution_test.go index a4d8409cd..2b7687d21 100644 --- a/src/core/api/replication_execution_test.go +++ b/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{