Moved utility logics of progress bar to inspection/progress

Related to #250

Author: kyasbal

pkg/core/inspection/runner.go

@@ -260,15 +260,15 @@ func (i *InspectionTaskRunner) Run(ctx context.Context, req *inspection_task.Ins
 		resultSize := 0
 		if result, err := i.runner.Result(); err != nil {
 			if errors.Is(cancelableCtx.Err(), context.Canceled) {
-				progress.Cancel()
+				progress.MarkCancelled()
 				status = "cancel"
 			} else {
-				progress.Error()
+				progress.MarkError()
 				status = "error"
 			}
 			slog.WarnContext(runCtx, fmt.Sprintf("task %s was finished with an error\n%s", i.ID, err))
 		} else {
-			progress.Done()
+			progress.MarkDone()
 			status = "done"
 
 			history, found := typedmap.Get(result, typedmap.NewTypedKey[inspectiondata.Store](serializer.SerializerTaskID.ReferenceIDString()))

pkg/inspection/form/fileform.go

@@ -52,10 +52,10 @@ func (b *FileFormTaskBuilder) WithDescription(description string) *FileFormTaskB
 }
 
 func (b *FileFormTaskBuilder) Build(labelOpts ...common_task.LabelOpt) common_task.Task[upload.UploadResult] {
-	return common_task.NewTask(b.id, b.dependencies, func(ctx context.Context) (upload.UploadResult, error) {
+	return common_task.NewTask(b.FormTaskBuilderBase.id, b.FormTaskBuilderBase.dependencies, func(ctx context.Context) (upload.UploadResult, error) {
 		metadata := khictx.MustGetValue(ctx, inspectioncontract.InspectionRunMetadata)
 
-		token := upload.DefaultUploadFileStore.GetUploadToken(upload.GenerateUploadIDWithTaskContext(ctx, b.id.ReferenceIDString()), b.verifier)
+		token := upload.DefaultUploadFileStore.GetUploadToken(upload.GenerateUploadIDWithTaskContext(ctx, b.FormTaskBuilderBase.id.ReferenceIDString()), b.verifier)
 		uploadResult, err := upload.DefaultUploadFileStore.GetResult(token)
 		if err != nil {
 			return upload.UploadResult{}, err
@@ -69,7 +69,7 @@ func (b *FileFormTaskBuilder) Build(labelOpts ...common_task.LabelOpt) common_ta
 			Token:  token,
 			Status: uploadResult.Status,
 		}
-		b.SetupBaseFormField(&field.ParameterFormFieldBase)
+		b.FormTaskBuilderBase.SetupBaseFormField(&field.ParameterFormFieldBase)
 
 		field = setFormHintsFromUploadResult(uploadResult, field)
 		formFields, found := typedmap.Get(metadata, form_metadata.FormFieldSetMetadataKey)
@@ -78,7 +78,7 @@ func (b *FileFormTaskBuilder) Build(labelOpts ...common_task.LabelOpt) common_ta
 		}
 		err = formFields.SetField(field)
 		if err != nil {
-			return upload.UploadResult{}, fmt.Errorf("failed to configure the form metadata in task `%s`\n%v", b.id, err)
+			return upload.UploadResult{}, fmt.Errorf("failed to configure the form metadata in task `%s`\n%v", b.FormTaskBuilderBase.id, err)
 		}
 
 		return uploadResult, nil

pkg/inspection/metadata/progress/conformance_test.go

@@ -25,8 +25,8 @@ import (
 
 func newProgressforConformanceTest() metadata.Metadata {
 	progress := NewProgress()
-	progress.GetTaskProgress("foo")
-	progress.GetTaskProgress("bar")
+	progress.GetOrCreateTaskProgress("foo")
+	progress.GetOrCreateTaskProgress("bar")
 	return progress
 }
 

pkg/inspection/metadata/progress/progress.go

@@ -22,13 +22,27 @@ import (
 	"github.com/GoogleCloudPlatform/khi/pkg/inspection/metadata"
 )
 
+// ProgressMetadataKey is the key used to store and retrieve Progress metadata
+// from a context or metadata map.
 var ProgressMetadataKey = metadata.NewMetadataKey[*Progress]("progress")
 
-const TASK_PHASE_RUNNING = "RUNNING"
-const TASK_PHASE_DONE = "DONE"
-const TASK_PHASE_ERROR = "ERROR"
-const TASK_PHASE_CANCELLED = "CANCELLED"
+// TaskProgressPhase represents the lifecycle phase of a task's progress.
+type TaskProgressPhase string
+
+// Constants defining the possible phases of a task's progress.
+const (
+	// TaskPhaseRunning indicates that the task is currently executing.
+	TaskPhaseRunning TaskProgressPhase = "RUNNING"
+	// TaskPhaseDone indicates that the task has completed successfully.
+	TaskPhaseDone = "DONE"
+	// TaskPhaseError indicates that the task terminated with an error.
+	TaskPhaseError = "ERROR"
+	// TaskPhaseCancelled indicates that the task was cancelled before completion.
+	TaskPhaseCancelled = "CANCELLED"
+)
 
+// TaskProgress represents the progress of a single task within an inspection.
+// It includes an ID, a human-readable label, a status message, and completion percentage.
 type TaskProgress struct {
 	Id            string  `json:"id"`
 	Label         string  `json:"label"`
@@ -37,6 +51,7 @@ type TaskProgress struct {
 	Indeterminate bool    `json:"indeterminate"`
 }
 
+// NewTaskProgress creates and initializes a new TaskProgress object with the given ID.
 func NewTaskProgress(id string) *TaskProgress {
 	return &TaskProgress{
 		Id:            id,
@@ -60,18 +75,21 @@ func (tp *TaskProgress) MarkIndeterminate() {
 	tp.Percentage = 0
 }
 
+// Progress aggregates the progress of all tasks in an inspection run.
+// It tracks the overall phase, total progress, and the progress of individual active tasks.
 type Progress struct {
-	Phase             string          `json:"phase"`
-	TotalProgress     *TaskProgress   `json:"totalProgress"`
-	TaskProgresses    []*TaskProgress `json:"progresses"`
-	totalTaskCount    int             `json:"-"`
-	resolvedTaskCount int             `json:"-"`
-	lock              sync.Mutex      `json:"-"`
+	Phase             TaskProgressPhase `json:"phase"`
+	TotalProgress     *TaskProgress     `json:"totalProgress"`
+	TaskProgresses    []*TaskProgress   `json:"progresses"`
+	totalTaskCount    int               `json:"-"`
+	resolvedTaskCount int               `json:"-"`
+	lock              sync.Mutex        `json:"-"`
 }
 
+// NewProgress creates and initializes a new Progress object.
 func NewProgress() *Progress {
 	return &Progress{
-		Phase:             TASK_PHASE_RUNNING,
+		Phase:             TaskPhaseRunning,
 		TaskProgresses:    make([]*TaskProgress, 0),
 		TotalProgress:     NewTaskProgress("Total"),
 		lock:              sync.Mutex{},
@@ -92,15 +110,20 @@ func (p *Progress) ToSerializable() interface{} {
 	return p
 }
 
+// SetTotalTaskCount sets the total number of tasks that will be tracked.
+// This is used to calculate the overall progress percentage.
 func (p *Progress) SetTotalTaskCount(count int) {
 	p.totalTaskCount = count
 	p.updateTotalTaskProgress()
 }
 
-func (p *Progress) GetTaskProgress(id string) (*TaskProgress, error) {
+// GetOrCreateTaskProgress retrieves the TaskProgress for a given task ID.
+// If no progress object exists for the ID, a new one is created and added to the list.
+// It returns an error if the overall progress is no longer in the RUNNING phase.
+func (p *Progress) GetOrCreateTaskProgress(id string) (*TaskProgress, error) {
 	p.lock.Lock()
 	defer p.lock.Unlock()
-	if p.Phase != TASK_PHASE_RUNNING {
+	if p.Phase != TaskPhaseRunning {
 		return nil, fmt.Errorf("the current progress phase is not RUNNING but %s", p.Phase)
 	}
 	for _, progress := range p.TaskProgresses {
@@ -113,10 +136,13 @@ func (p *Progress) GetTaskProgress(id string) (*TaskProgress, error) {
 	return taskProgress, nil
 }
 
+// ResolveTask marks a task as resolved by removing it from the active progress list
+// and increments the count of resolved tasks.
+// It returns an error if the overall progress is no longer in the RUNNING phase.
 func (p *Progress) ResolveTask(id string) error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
-	if p.Phase != TASK_PHASE_RUNNING {
+	if p.Phase != TaskPhaseRunning {
 		return fmt.Errorf("the current progress phase is not RUNNING but %s", p.Phase)
 	}
 	newTaskProgress := make([]*TaskProgress, 0)
@@ -131,37 +157,46 @@ func (p *Progress) ResolveTask(id string) error {
 	return nil
 }
 
-func (p *Progress) Done() error {
+// MarkDone transitions the overall progress to the DONE phase.
+// It clears all active task progresses and marks the total progress as 100% complete.
+// It returns an error if the overall progress is no longer in the RUNNING phase.
+func (p *Progress) MarkDone() error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
-	if p.Phase != TASK_PHASE_RUNNING {
+	if p.Phase != TaskPhaseRunning {
 		return fmt.Errorf("the current progress phase is not RUNNING but %s", p.Phase)
 	}
-	p.Phase = TASK_PHASE_DONE
+	p.Phase = TaskPhaseDone
 	p.resolvedTaskCount = p.totalTaskCount
 	p.TaskProgresses = make([]*TaskProgress, 0)
 	p.updateTotalTaskProgress()
 	return nil
 }
 
-func (p *Progress) Cancel() error {
+// MarkCancelled transitions the overall progress to the CANCELLED phase.
+// It clears all active task progresses.
+// It returns an error if the overall progress is no longer in the RUNNING phase.
+func (p *Progress) MarkCancelled() error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
-	if p.Phase != TASK_PHASE_RUNNING {
+	if p.Phase != TaskPhaseRunning {
 		return fmt.Errorf("the current progress phase is not RUNNING but %s", p.Phase)
 	}
-	p.Phase = TASK_PHASE_CANCELLED
+	p.Phase = TaskPhaseCancelled
 	p.TaskProgresses = make([]*TaskProgress, 0)
 	return nil
 }
 
-func (p *Progress) Error() error {
+// MarkError transitions the overall progress to the ERROR phase.
+// It clears all active task progresses.
+// It returns an error if the overall progress is no longer in the RUNNING phase.
+func (p *Progress) MarkError() error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
-	if p.Phase != TASK_PHASE_RUNNING {
+	if p.Phase != TaskPhaseRunning {
 		return fmt.Errorf("the current progress phase is not RUNNING but %s", p.Phase)
 	}
-	p.Phase = TASK_PHASE_ERROR
+	p.Phase = TaskPhaseError
 	p.TaskProgresses = make([]*TaskProgress, 0)
 	return nil
 }

pkg/inspection/metadata/progress/progress_test.go

@@ -25,7 +25,7 @@ import (
 
 func TestGetTaskProgress(t *testing.T) {
 	progress := NewProgress()
-	tp, err := progress.GetTaskProgress("foo")
+	tp, err := progress.GetOrCreateTaskProgress("foo")
 	if err != nil {
 		t.Errorf("unexpected error %s", err)
 	}
@@ -41,7 +41,7 @@ func TestGetTaskProgress(t *testing.T) {
 		t.Errorf("generated task progress is not containing the expected state\n%s", diff)
 	}
 
-	tp2, err := progress.GetTaskProgress("foo")
+	tp2, err := progress.GetOrCreateTaskProgress("foo")
 	if err != nil {
 		t.Errorf("unexpected error %s", err)
 	}
@@ -54,8 +54,8 @@ func TestGetTaskProgress(t *testing.T) {
 func TestResolveTasks(t *testing.T) {
 	progress := NewProgress()
 	progress.SetTotalTaskCount(2)
-	progress.GetTaskProgress("foo")
-	progress.GetTaskProgress("bar")
+	progress.GetOrCreateTaskProgress("foo")
+	progress.GetOrCreateTaskProgress("bar")
 	progress.ResolveTask("foo")
 
 	if diff := cmp.Diff(&Progress{
@@ -75,9 +75,9 @@ func TestResolveTasks(t *testing.T) {
 func TestDoneClearTasks(t *testing.T) {
 	progress := NewProgress()
 	progress.SetTotalTaskCount(2)
-	progress.GetTaskProgress("foo")
-	progress.GetTaskProgress("bar")
-	progress.Done()
+	progress.GetOrCreateTaskProgress("foo")
+	progress.GetOrCreateTaskProgress("bar")
+	progress.MarkDone()
 
 	if diff := cmp.Diff(&Progress{
 		Phase:          "DONE",
@@ -91,9 +91,9 @@ func TestDoneClearTasks(t *testing.T) {
 func TestCancelClearTasks(t *testing.T) {
 	progress := NewProgress()
 	progress.SetTotalTaskCount(2)
-	progress.GetTaskProgress("foo")
-	progress.GetTaskProgress("bar")
-	progress.Cancel()
+	progress.GetOrCreateTaskProgress("foo")
+	progress.GetOrCreateTaskProgress("bar")
+	progress.MarkCancelled()
 
 	if diff := cmp.Diff(&Progress{
 		Phase:          "CANCELLED",

…tadata/progress/indeterminate_updator.go …on/progressutil/indeterminate_updator.gopkg/inspection/metadata/progress/indeterminate_updator.go renamed to pkg/inspection/progressutil/indeterminate_updator.go pkg/inspection/metadata/progress/indeterminate_updator.go renamed to pkg/inspection/progressutil/indeterminate_updator.go

@@ -12,23 +12,29 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package progress
+package progressutil
 
 import (
 	"context"
 	"fmt"
 	"time"
+
+	"github.com/GoogleCloudPlatform/khi/pkg/inspection/metadata/progress"
 )
 
-// IndeterminateUpdator updates progress bar during a procedure can't report its progress.
+// IndeterminateUpdator updates a progress bar for a task that cannot report
+// its progress as a percentage. It shows a message with an animated indicator
+// to signify that the task is running.
 type IndeterminateUpdator struct {
-	Progress *TaskProgress
+	Progress *progress.TaskProgress
 	Interval time.Duration
 	context  context.Context
 	cancel   func()
 }
 
-func NewIndeterminateUpdator(progress *TaskProgress, interval time.Duration) *IndeterminateUpdator {
+// NewIndeterminateUpdator creates and initializes a new IndeterminateUpdator.
+// It marks the associated TaskProgress as indeterminate.
+func NewIndeterminateUpdator(progress *progress.TaskProgress, interval time.Duration) *IndeterminateUpdator {
 	progress.Indeterminate = true
 	return &IndeterminateUpdator{
 		Progress: progress,

…a/progress/indeterminate_updator_test.go …ogressutil/indeterminate_updator_test.gopkg/inspection/metadata/progress/indeterminate_updator_test.go renamed to pkg/inspection/progressutil/indeterminate_updator_test.go pkg/inspection/metadata/progress/indeterminate_updator_test.go renamed to pkg/inspection/progressutil/indeterminate_updator_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package progress
+package progressutil
 
 import (
 	"testing"
@@ -22,32 +22,33 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 
 	_ "github.com/GoogleCloudPlatform/khi/internal/testflags"
+	"github.com/GoogleCloudPlatform/khi/pkg/inspection/metadata/progress"
 )
 
 func TestIndeterminateUpdator(t *testing.T) {
-	progress := NewTaskProgress("foo")
-	updator := NewIndeterminateUpdator(progress, 1000*time.Millisecond)
+	tp := progress.NewTaskProgress("foo")
+	updator := NewIndeterminateUpdator(tp, 1000*time.Millisecond)
 	err := updator.Start("working")
 	if err != nil {
 		t.Errorf("unexpected error %s", err)
 	}
 	time.Sleep(1500 * time.Millisecond)
-	if diff := cmp.Diff(&TaskProgress{
+	if diff := cmp.Diff(&progress.TaskProgress{
 		Id:            "foo",
 		Label:         "foo",
 		Message:       "working.",
 		Percentage:    0,
 		Indeterminate: true,
-	}, progress, cmpopts.IgnoreUnexported(TaskProgress{})); diff != "" {
+	}, tp, cmpopts.IgnoreUnexported(progress.TaskProgress{})); diff != "" {
 		t.Errorf("The result status is not in the expected status\n%s", diff)
 	}
 	err = updator.Done()
 	if err != nil {
 		t.Errorf("unexpected error %s", err)
 	}
-	msg := progress.Message
+	msg := tp.Message
 	time.Sleep(1000 * time.Millisecond)
-	if msg != progress.Message {
+	if msg != tp.Message {
 		t.Errorf("The progress is keeping updated")
 	}
 }