diff --git a/internal/app/pipelines.go b/internal/app/pipelines.go index 43bc282..37de95f 100644 --- a/internal/app/pipelines.go +++ b/internal/app/pipelines.go @@ -191,26 +191,21 @@ func (s *Service) pipelineTarget(ctx context.Context, target Target) (string, st return spindleHost, repoDID, nil } -// ViewPipeline finds a pipeline by its spindle-local ID. +// ViewPipeline fetches a pipeline by its spindle-local ID. func (s *Service) ViewPipeline(ctx context.Context, target Target, pipelineID string) (*Pipeline, error) { - pipelines, err := s.ListPipelines(ctx, target) + client, repoDID, err := s.pipelineClient(ctx, target) if err != nil { return nil, err } - pipeline, err := findPipeline(pipelines, pipelineID) + pipeline, err := client.GetPipeline(ctx, pipelineID) if err != nil { - return nil, fmt.Errorf("pipeline %q not found for repository %q", pipelineID, target.String()) + return nil, err } - return pipeline, nil -} - -func findPipeline(pipelines []Pipeline, pipelineID string) (*Pipeline, error) { - for index := range pipelines { - if pipelines[index].ID == pipelineID { - return &pipelines[index], nil - } + if pipeline.Repo != repoDID { + return nil, fmt.Errorf("pipeline %q does not belong to repository %q", pipelineID, target.String()) } - return nil, fmt.Errorf("pipeline %q not found", pipelineID) + item := pipelineItem(*pipeline) + return &item, nil } func pipelineHasFailures(pipeline Pipeline) bool { @@ -242,17 +237,21 @@ func listPipelinePages(ctx context.Context, client pipelineClient, repoDID strin func pipelineItems(pipelines []spindle.Pipeline) []Pipeline { items := make([]Pipeline, 0, len(pipelines)) for _, pipeline := range pipelines { - workflows := make([]PipelineWorkflow, 0, len(pipeline.Workflows)) - for _, workflow := range pipeline.Workflows { - workflows = append(workflows, PipelineWorkflow{ - ID: workflow.ID, Name: workflow.Name, Status: workflow.Status, Error: workflow.Error, - StartedAt: workflow.StartedAt, FinishedAt: workflow.FinishedAt, - }) - } - items = append(items, Pipeline{ - ID: pipeline.ID, Commit: pipeline.Commit, CreatedAt: pipeline.CreatedAt, - Repo: pipeline.Repo, SourceRepo: pipeline.SourceRepo, Trigger: pipeline.Trigger, Workflows: workflows, - }) + items = append(items, pipelineItem(pipeline)) } return items } + +func pipelineItem(pipeline spindle.Pipeline) Pipeline { + workflows := make([]PipelineWorkflow, 0, len(pipeline.Workflows)) + for _, workflow := range pipeline.Workflows { + workflows = append(workflows, PipelineWorkflow{ + ID: workflow.ID, Name: workflow.Name, Status: workflow.Status, Error: workflow.Error, + StartedAt: workflow.StartedAt, FinishedAt: workflow.FinishedAt, + }) + } + return Pipeline{ + ID: pipeline.ID, Commit: pipeline.Commit, CreatedAt: pipeline.CreatedAt, + Repo: pipeline.Repo, SourceRepo: pipeline.SourceRepo, Trigger: pipeline.Trigger, Workflows: workflows, + } +} diff --git a/internal/app/pipelines_test.go b/internal/app/pipelines_test.go index 6f1a20b..4ec0067 100644 --- a/internal/app/pipelines_test.go +++ b/internal/app/pipelines_test.go @@ -37,14 +37,37 @@ func TestListPipelinePagesReturnsClientError(t *testing.T) { } } -func TestFindPipeline(t *testing.T) { - pipelines := []Pipeline{{ID: "first"}, {ID: "second"}} - found, err := findPipeline(pipelines, "second") +func TestViewPipelineFetchesPipelineDirectly(t *testing.T) { + client := &testPipelineClient{pipeline: &spindle.Pipeline{ID: "second", Repo: "did:plc:repo"}} + service := testService(&testPDS{}, &testGit{}, &testKnot{}) + service.appview = testAppview{repo: &tangled.Repo{Value: tangledlex.Repo{ + Spindle: optionalString("spindle.example"), RepoDid: optionalString("did:plc:repo"), + }}} + service.spindle = testSpindleFactory{client: client} + + found, err := service.ViewPipeline(context.Background(), Target{Handle: "owner.test", Repo: "example"}, "second") if err != nil { - t.Fatalf("findPipeline() error = %v", err) + t.Fatalf("ViewPipeline() error = %v", err) } if found.ID != "second" { - t.Fatalf("findPipeline() = %+v, want second", found) + t.Fatalf("ViewPipeline() = %+v, want second", found) + } + if client.pipelineID != "second" { + t.Fatalf("GetPipeline() ID = %q, want second", client.pipelineID) + } +} + +func TestViewPipelineRejectsPipelineFromAnotherRepository(t *testing.T) { + client := &testPipelineClient{pipeline: &spindle.Pipeline{ID: "second", Repo: "did:plc:other"}} + service := testService(&testPDS{}, &testGit{}, &testKnot{}) + service.appview = testAppview{repo: &tangled.Repo{Value: tangledlex.Repo{ + Spindle: optionalString("spindle.example"), RepoDid: optionalString("did:plc:repo"), + }}} + service.spindle = testSpindleFactory{client: client} + + _, err := service.ViewPipeline(context.Background(), Target{Handle: "owner.test", Repo: "example"}, "second") + if err == nil || err.Error() != "pipeline \"second\" does not belong to repository \"owner.test/example\"" { + t.Fatalf("ViewPipeline() error = %v", err) } } @@ -161,6 +184,7 @@ type testPipelineClient struct { err error cancelInput spindle.CancelPipelineInput pipeline *spindle.Pipeline + pipelineID string triggerInput spindle.TriggerPipelineInput triggerOutput *spindle.TriggerPipelineOutput } @@ -169,7 +193,8 @@ func (c *testPipelineClient) QueryLatestPipeline(_ context.Context, _ string) (* return c.responses[0], nil } -func (c *testPipelineClient) GetPipeline(context.Context, string) (*spindle.Pipeline, error) { +func (c *testPipelineClient) GetPipeline(_ context.Context, pipelineID string) (*spindle.Pipeline, error) { + c.pipelineID = pipelineID return c.pipeline, c.err }