From 9a674c82354905c3d29356f7244f37a4d48cdf6a Mon Sep 17 00:00:00 2001 From: johha Date: Thu, 3 Sep 2026 11:25:57 +0200 Subject: [PATCH] Stream async job warnings as they arrive for delete commands Migrate delete, delete-org, and delete-space from aggregated PollJob to a streaming job poll (PollJobToEventStream -> WaitForResult), so warnings print live instead of dumped at the end. Dedupe distinct warnings to at most once per operation. --- actor/v7action/application.go | 67 +++--- actor/v7action/application_test.go | 195 ++++++++++-------- actor/v7action/organization.go | 11 +- actor/v7action/organization_test.go | 38 +++- actor/v7action/space.go | 13 +- actor/v7action/space_test.go | 37 +++- command/v7/actor.go | 6 +- command/v7/bind_route_service_command_test.go | 2 +- command/v7/bind_service_command_test.go | 2 +- ..._outdated_service_bindings_command_test.go | 2 +- command/v7/create_service_command_test.go | 2 +- command/v7/create_service_key_command_test.go | 2 +- command/v7/delete_command.go | 9 +- command/v7/delete_command_test.go | 21 +- command/v7/delete_org_command.go | 5 +- command/v7/delete_org_command_test.go | 14 +- command/v7/delete_service_command_test.go | 2 +- command/v7/delete_service_key_command_test.go | 2 +- command/v7/delete_space_command.go | 5 +- command/v7/delete_space_command_test.go | 17 +- command/v7/shared/result_waiter.go | 20 +- command/v7/shared/result_waiter_test.go | 136 ++++++++++++ .../v7/unbind_route_service_command_test.go | 2 +- command/v7/unbind_service_command_test.go | 2 +- command/v7/update_service_command_test.go | 2 +- command/v7/upgrade_service_command_test.go | 2 +- command/v7/v7fakes/fake_actor.go | 129 +++++++----- 27 files changed, 511 insertions(+), 234 deletions(-) create mode 100644 command/v7/shared/result_waiter_test.go diff --git a/actor/v7action/application.go b/actor/v7action/application.go index 7c45b6ff09b..b7f86c15472 100644 --- a/actor/v7action/application.go +++ b/actor/v7action/application.go @@ -14,14 +14,13 @@ import ( "code.cloudfoundry.org/cli/v8/util/unique" ) -func (actor Actor) DeleteApplicationByNameAndSpace(name, spaceGUID string, deleteRoutes bool) (Warnings, error) { +func (actor Actor) DeleteApplicationByNameAndSpace(name, spaceGUID string, deleteRoutes bool) (chan PollJobEvent, Warnings, error) { var allWarnings Warnings - var jobQueue []ccv3.JobURL app, getAppWarnings, err := actor.GetApplicationByNameAndSpace(name, spaceGUID) allWarnings = append(allWarnings, getAppWarnings...) if err != nil { - return allWarnings, err + return nil, allWarnings, err } var routes []resources.Route @@ -30,15 +29,14 @@ func (actor Actor) DeleteApplicationByNameAndSpace(name, spaceGUID string, delet routes, getRoutesWarnings, err = actor.GetApplicationRoutes(app.GUID) allWarnings = append(allWarnings, getRoutesWarnings...) if err != nil { - return allWarnings, err + return nil, allWarnings, err } for _, route := range routes { if len(route.Destinations) > 1 { for _, destination := range route.Destinations { - guid := destination.App.GUID - if guid != app.GUID { - return allWarnings, actionerror.RouteBoundToMultipleAppsError{AppName: app.Name, RouteURL: route.URL} + if destination.App.GUID != app.GUID { + return nil, allWarnings, actionerror.RouteBoundToMultipleAppsError{AppName: app.Name, RouteURL: route.URL} } } } @@ -48,39 +46,44 @@ func (actor Actor) DeleteApplicationByNameAndSpace(name, spaceGUID string, delet appDeleteJobURL, deleteAppWarnings, err := actor.CloudControllerClient.DeleteApplication(app.GUID) allWarnings = append(allWarnings, deleteAppWarnings...) if err != nil { - return allWarnings, err + return nil, allWarnings, err } - pollWarnings, err := actor.CloudControllerClient.PollJob(appDeleteJobURL) - allWarnings = append(allWarnings, pollWarnings...) - if err != nil { - return allWarnings, err - } + stream := make(chan PollJobEvent) + go func() { + defer close(stream) - if deleteRoutes { - for _, route := range routes { - jobURL, deleteRouteWarnings, err := actor.CloudControllerClient.DeleteRoute(route.GUID) - allWarnings = append(allWarnings, deleteRouteWarnings...) - if err != nil { - if _, ok := err.(ccerror.ResourceNotFoundError); ok { - continue - } - return allWarnings, err + for event := range actor.PollJobToEventStream(appDeleteJobURL) { + stream <- event + if event.Err != nil { + return } - - jobQueue = append(jobQueue, jobURL) } - } - for _, job := range jobQueue { - pollWarnings, err := actor.CloudControllerClient.PollJob(job) - allWarnings = append(allWarnings, pollWarnings...) - if err != nil { - return allWarnings, err + if deleteRoutes { + for _, route := range routes { + jobURL, deleteWarnings, err := actor.CloudControllerClient.DeleteRoute(route.GUID) + if err != nil { + if _, ok := err.(ccerror.ResourceNotFoundError); ok { + stream <- PollJobEvent{Warnings: Warnings(deleteWarnings)} + continue + } + stream <- PollJobEvent{Warnings: Warnings(deleteWarnings), Err: err} + return + } + stream <- PollJobEvent{Warnings: Warnings(deleteWarnings)} + + for event := range actor.PollJobToEventStream(jobURL) { + stream <- event + if event.Err != nil { + return + } + } + } } - } + }() - return allWarnings, err + return stream, allWarnings, nil } func (actor Actor) GetApplicationsByGUIDs(appGUIDs []string) ([]resources.Application, Warnings, error) { diff --git a/actor/v7action/application_test.go b/actor/v7action/application_test.go index 3eeb2dd03a2..d0859234369 100644 --- a/actor/v7action/application_test.go +++ b/actor/v7action/application_test.go @@ -34,8 +34,18 @@ var _ = Describe("Application Actions", func() { actor = NewActor(fakeCloudControllerClient, fakeConfig, nil, nil, nil, fakeClock) }) + ccStream := func(events ...ccv3.PollJobEvent) chan ccv3.PollJobEvent { + s := make(chan ccv3.PollJobEvent, len(events)) + for _, e := range events { + s <- e + } + close(s) + return s + } + Describe("DeleteApplicationByNameAndSpace", func() { var ( + stream chan PollJobEvent warnings Warnings executeErr error deleteMappedRoutes bool @@ -44,7 +54,18 @@ var _ = Describe("Application Actions", func() { JustBeforeEach(func() { appName = "some-app" - warnings, executeErr = actor.DeleteApplicationByNameAndSpace(appName, "some-space-guid", deleteMappedRoutes) + var upfrontWarnings Warnings + stream, upfrontWarnings, executeErr = actor.DeleteApplicationByNameAndSpace(appName, "some-space-guid", deleteMappedRoutes) + + warnings = upfrontWarnings + if stream != nil { + for event := range stream { + warnings = append(warnings, event.Warnings...) + if event.Err != nil { + executeErr = event.Err + } + } + } }) When("looking up the app guid fails", func() { @@ -82,7 +103,11 @@ var _ = Describe("Application Actions", func() { When("polling fails", func() { BeforeEach(func() { - fakeCloudControllerClient.PollJobReturns(ccv3.Warnings{"some-poll-warning"}, errors.New("some-poll-error")) + fakeCloudControllerClient.PollJobToEventStreamReturns(ccStream(ccv3.PollJobEvent{ + State: constant.JobFailed, + Warnings: ccv3.Warnings{"some-poll-warning"}, + Err: errors.New("some-poll-error"), + })) }) It("returns the warnings and poll error", func() { @@ -93,12 +118,17 @@ var _ = Describe("Application Actions", func() { When("polling succeeds", func() { BeforeEach(func() { - fakeCloudControllerClient.PollJobReturns(ccv3.Warnings{"some-poll-warning"}, nil) + fakeCloudControllerClient.PollJobToEventStreamReturns(ccStream(ccv3.PollJobEvent{ + State: constant.JobComplete, + Warnings: ccv3.Warnings{"some-poll-warning"}, + })) }) It("returns all the warnings and no error", func() { Expect(warnings).To(ConsistOf("some-get-app-warning", "some-delete-app-warning", "some-poll-warning")) Expect(executeErr).ToNot(HaveOccurred()) + Expect(fakeCloudControllerClient.GetApplicationRoutesCallCount()).To(Equal(0)) + Expect(fakeCloudControllerClient.DeleteRouteCallCount()).To(Equal(0)) }) }) }) @@ -108,6 +138,7 @@ var _ = Describe("Application Actions", func() { BeforeEach(func() { deleteMappedRoutes = true fakeCloudControllerClient.GetApplicationsReturns([]resources.Application{{Name: "some-app", GUID: "abc123"}}, nil, nil) + fakeCloudControllerClient.DeleteApplicationReturns("/some-job-url", nil, nil) }) When("getting the routes fails", func() { @@ -118,113 +149,99 @@ var _ = Describe("Application Actions", func() { It("returns the warnings and an error", func() { Expect(warnings).To(ConsistOf("get-routes-warning")) Expect(executeErr).To(MatchError("get-routes-error")) + Expect(fakeCloudControllerClient.DeleteApplicationCallCount()).To(Equal(0)) }) }) - When("getting the routes succeeds", func() { - When("there are no routes", func() { - BeforeEach(func() { - fakeCloudControllerClient.GetApplicationRoutesReturns([]resources.Route{}, nil, nil) - }) + When("app to delete has a route bound to another app", func() { + BeforeEach(func() { + fakeCloudControllerClient.GetApplicationRoutesReturns( + []resources.Route{ + {GUID: "route-1-guid"}, + {GUID: "route-2-guid", + URL: "route-2.example.com", + Destinations: []resources.RouteDestination{ + {App: resources.RouteDestinationApp{GUID: "abc123"}}, + {App: resources.RouteDestinationApp{GUID: "different-app-guid"}}, + }, + }, + }, + nil, + nil, + ) + }) - It("does not delete any routes", func() { - Expect(fakeCloudControllerClient.DeleteRouteCallCount()).To(Equal(0)) - }) + It("refuses the entire operation", func() { + Expect(executeErr).To(MatchError(actionerror.RouteBoundToMultipleAppsError{AppName: "some-app", RouteURL: "route-2.example.com"})) + Expect(warnings).To(BeEmpty()) + Expect(fakeCloudControllerClient.DeleteApplicationCallCount()).To(Equal(0)) + Expect(fakeCloudControllerClient.DeleteRouteCallCount()).To(Equal(0)) + }) + }) + + When("getting the routes succeeds", func() { + BeforeEach(func() { + fakeCloudControllerClient.GetApplicationRoutesReturns([]resources.Route{{GUID: "route-1-guid"}, {GUID: "route-2-guid"}}, nil, nil) + fakeCloudControllerClient.PollJobToEventStreamReturnsOnCall(0, ccStream(ccv3.PollJobEvent{State: constant.JobComplete, Warnings: ccv3.Warnings{"app-poll-warning"}})) }) When("there are routes", func() { BeforeEach(func() { - fakeCloudControllerClient.GetApplicationRoutesReturns([]resources.Route{{GUID: "route-1-guid"}, {GUID: "route-2-guid", URL: "route-2.example.com"}}, nil, nil) + fakeCloudControllerClient.DeleteRouteReturnsOnCall(0, "/route-1-job", ccv3.Warnings{"delete-route-1-warning"}, nil) + fakeCloudControllerClient.DeleteRouteReturnsOnCall(1, "/route-2-job", ccv3.Warnings{"delete-route-2-warning"}, nil) + fakeCloudControllerClient.PollJobToEventStreamReturnsOnCall(1, ccStream(ccv3.PollJobEvent{State: constant.JobComplete, Warnings: ccv3.Warnings{"route-1-poll-warning"}})) + fakeCloudControllerClient.PollJobToEventStreamReturnsOnCall(2, ccStream(ccv3.PollJobEvent{State: constant.JobComplete, Warnings: ccv3.Warnings{"route-2-poll-warning"}})) }) It("deletes the routes", func() { - Expect(fakeCloudControllerClient.GetApplicationRoutesCallCount()).To(Equal(1)) - Expect(fakeCloudControllerClient.GetApplicationRoutesArgsForCall(0)).To(Equal("abc123")) + Expect(executeErr).ToNot(HaveOccurred()) Expect(fakeCloudControllerClient.DeleteRouteCallCount()).To(Equal(2)) - guids := []string{fakeCloudControllerClient.DeleteRouteArgsForCall(0), fakeCloudControllerClient.DeleteRouteArgsForCall(1)} - Expect(guids).To(ConsistOf("route-1-guid", "route-2-guid")) + Expect(fakeCloudControllerClient.DeleteRouteArgsForCall(0)).To(Equal("route-1-guid")) + Expect(fakeCloudControllerClient.DeleteRouteArgsForCall(1)).To(Equal("route-2-guid")) + Expect(warnings).To(ConsistOf( + "app-poll-warning", + "delete-route-1-warning", "route-1-poll-warning", + "delete-route-2-warning", "route-2-poll-warning", + )) }) + }) - When("the route has already been deleted", func() { - BeforeEach(func() { - fakeCloudControllerClient.DeleteRouteReturnsOnCall(0, - "", - ccv3.Warnings{"delete-route-1-warning"}, - ccerror.ResourceNotFoundError{}, - ) - fakeCloudControllerClient.DeleteRouteReturnsOnCall(1, - "poll-job-url", - ccv3.Warnings{"delete-route-2-warning"}, - nil, - ) - fakeCloudControllerClient.PollJobReturnsOnCall(1, ccv3.Warnings{"poll-job-warning"}, nil) - }) - - It("does **not** fail", func() { - Expect(executeErr).ToNot(HaveOccurred()) - Expect(warnings).To(ConsistOf("delete-route-1-warning", "delete-route-2-warning", "poll-job-warning")) - Expect(fakeCloudControllerClient.DeleteRouteCallCount()).To(Equal(2)) - Expect(fakeCloudControllerClient.PollJobCallCount()).To(Equal(2)) - Expect(fakeCloudControllerClient.PollJobArgsForCall(1)).To(BeEquivalentTo("poll-job-url")) - }) + When("the route has already been deleted", func() { + BeforeEach(func() { + fakeCloudControllerClient.DeleteRouteReturnsOnCall(0, + "", + ccv3.Warnings{"delete-route-1-warning"}, + ccerror.ResourceNotFoundError{}, + ) + fakeCloudControllerClient.DeleteRouteReturnsOnCall(1, + "/route-2-job", + ccv3.Warnings{"delete-route-2-warning"}, + nil, + ) + fakeCloudControllerClient.PollJobToEventStreamReturnsOnCall(1, ccStream(ccv3.PollJobEvent{State: constant.JobComplete})) }) - When("app to delete has a route bound to another app", func() { - BeforeEach(func() { - fakeCloudControllerClient.GetApplicationRoutesReturns( - []resources.Route{ - {GUID: "route-1-guid"}, - {GUID: "route-2-guid", - URL: "route-2.example.com", - Destinations: []resources.RouteDestination{ - {App: resources.RouteDestinationApp{GUID: "abc123"}}, - {App: resources.RouteDestinationApp{GUID: "different-app-guid"}}, - }, - }, - }, - nil, - nil, - ) - }) - - It("refuses the entire operation", func() { - Expect(executeErr).To(MatchError(actionerror.RouteBoundToMultipleAppsError{AppName: "some-app", RouteURL: "route-2.example.com"})) - Expect(warnings).To(BeEmpty()) - Expect(fakeCloudControllerClient.DeleteApplicationCallCount()).To(Equal(0)) - Expect(fakeCloudControllerClient.DeleteRouteCallCount()).To(Equal(0)) - }) + It("does **not** fail", func() { + Expect(executeErr).ToNot(HaveOccurred()) + Expect(fakeCloudControllerClient.DeleteRouteCallCount()).To(Equal(2)) + Expect(warnings).To(ConsistOf("app-poll-warning", "delete-route-1-warning", "delete-route-2-warning")) }) + }) - When("deleting the route fails", func() { - BeforeEach(func() { - fakeCloudControllerClient.DeleteRouteReturnsOnCall(0, - "poll-job-url", - ccv3.Warnings{"delete-route-1-warning"}, - nil, - ) - fakeCloudControllerClient.DeleteRouteReturnsOnCall(1, - "", - ccv3.Warnings{"delete-route-2-warning"}, - errors.New("delete-route-2-error"), - ) - }) - - It("returns the error", func() { - Expect(executeErr).To(MatchError("delete-route-2-error")) - Expect(warnings).To(ConsistOf("delete-route-1-warning", "delete-route-2-warning")) - }) + When("deleting the route fails", func() { + BeforeEach(func() { + fakeCloudControllerClient.DeleteRouteReturnsOnCall(0, + "", + ccv3.Warnings{"delete-route-1-warning"}, + errors.New("delete-route-error"), + ) }) - When("the polling job fails", func() { - BeforeEach(func() { - fakeCloudControllerClient.PollJobReturns(ccv3.Warnings{"poll-job-warning"}, errors.New("poll-job-error")) - }) - - It("returns the error", func() { - Expect(executeErr).To(MatchError("poll-job-error")) - }) + It("returns the error", func() { + Expect(executeErr).To(MatchError("delete-route-error")) + Expect(warnings).To(ConsistOf("app-poll-warning", "delete-route-1-warning")) + Expect(fakeCloudControllerClient.DeleteRouteCallCount()).To(Equal(1)) }) - }) }) }) diff --git a/actor/v7action/organization.go b/actor/v7action/organization.go index 1ebed1b9d09..e513f157542 100644 --- a/actor/v7action/organization.go +++ b/actor/v7action/organization.go @@ -87,25 +87,22 @@ func (actor Actor) RenameOrganization(oldOrgName, newOrgName string) (resources. return org, allWarnings, nil } -func (actor Actor) DeleteOrganization(name string) (Warnings, error) { +func (actor Actor) DeleteOrganization(name string) (chan PollJobEvent, Warnings, error) { var allWarnings Warnings org, warnings, err := actor.GetOrganizationByName(name) allWarnings = append(allWarnings, warnings...) if err != nil { - return allWarnings, err + return nil, allWarnings, err } jobURL, deleteWarnings, err := actor.CloudControllerClient.DeleteOrganization(org.GUID) allWarnings = append(allWarnings, Warnings(deleteWarnings)...) if err != nil { - return allWarnings, err + return nil, allWarnings, err } - ccWarnings, err := actor.CloudControllerClient.PollJob(jobURL) - allWarnings = append(allWarnings, Warnings(ccWarnings)...) - - return allWarnings, err + return actor.PollJobToEventStream(jobURL), allWarnings, nil } func (actor Actor) GetDefaultDomain(orgGUID string) (resources.Domain, Warnings, error) { diff --git a/actor/v7action/organization_test.go b/actor/v7action/organization_test.go index efeb65de65a..4fb634aa274 100644 --- a/actor/v7action/organization_test.go +++ b/actor/v7action/organization_test.go @@ -7,6 +7,7 @@ import ( . "code.cloudfoundry.org/cli/v8/actor/v7action" "code.cloudfoundry.org/cli/v8/actor/v7action/v7actionfakes" "code.cloudfoundry.org/cli/v8/api/cloudcontroller/ccv3" + "code.cloudfoundry.org/cli/v8/api/cloudcontroller/ccv3/constant" "code.cloudfoundry.org/cli/v8/resources" . "github.com/onsi/ginkgo/v2" @@ -354,8 +355,28 @@ var _ = Describe("Organization Actions", func() { err error ) + ccStream := func(events ...ccv3.PollJobEvent) chan ccv3.PollJobEvent { + stream := make(chan ccv3.PollJobEvent, len(events)) + for _, e := range events { + stream <- e + } + close(stream) + return stream + } + JustBeforeEach(func() { - warnings, err = actor.DeleteOrganization("some-org") + var stream chan PollJobEvent + var upfront Warnings + stream, upfront, err = actor.DeleteOrganization("some-org") + warnings = upfront + if stream != nil { + for event := range stream { + warnings = append(warnings, event.Warnings...) + if event.Err != nil { + err = event.Err + } + } + } }) When("the org is not found", func() { @@ -417,7 +438,11 @@ var _ = Describe("Organization Actions", func() { BeforeEach(func() { expectedErr = errors.New("Never expected, by anyone") - fakeCloudControllerClient.PollJobReturns(ccv3.Warnings{"warning-7", "warning-8"}, expectedErr) + fakeCloudControllerClient.PollJobToEventStreamReturns(ccStream(ccv3.PollJobEvent{ + State: constant.JobFailed, + Warnings: ccv3.Warnings{"warning-7", "warning-8"}, + Err: expectedErr, + })) }) It("returns the error", func() { @@ -428,7 +453,10 @@ var _ = Describe("Organization Actions", func() { When("the job is successful", func() { BeforeEach(func() { - fakeCloudControllerClient.PollJobReturns(ccv3.Warnings{"warning-7", "warning-8"}, nil) + fakeCloudControllerClient.PollJobToEventStreamReturns(ccStream(ccv3.PollJobEvent{ + State: constant.JobComplete, + Warnings: ccv3.Warnings{"warning-7", "warning-8"}, + })) }) It("returns warnings and no error", func() { @@ -445,8 +473,8 @@ var _ = Describe("Organization Actions", func() { Expect(fakeCloudControllerClient.DeleteOrganizationCallCount()).To(Equal(1)) Expect(fakeCloudControllerClient.DeleteOrganizationArgsForCall(0)).To(Equal("some-org-guid")) - Expect(fakeCloudControllerClient.PollJobCallCount()).To(Equal(1)) - Expect(fakeCloudControllerClient.PollJobArgsForCall(0)).To(Equal(ccv3.JobURL("some-url"))) + Expect(fakeCloudControllerClient.PollJobToEventStreamCallCount()).To(Equal(1)) + Expect(fakeCloudControllerClient.PollJobToEventStreamArgsForCall(0)).To(Equal(ccv3.JobURL("some-url"))) }) }) }) diff --git a/actor/v7action/space.go b/actor/v7action/space.go index e788fc6abca..8ccbbdad661 100644 --- a/actor/v7action/space.go +++ b/actor/v7action/space.go @@ -242,31 +242,28 @@ func (actor Actor) GetOrganizationSpaces(orgGUID string) ([]resources.Space, War return actor.GetOrganizationSpacesWithLabelSelector(orgGUID, "") } -func (actor Actor) DeleteSpaceByNameAndOrganizationName(spaceName string, orgName string) (Warnings, error) { +func (actor Actor) DeleteSpaceByNameAndOrganizationName(spaceName string, orgName string) (chan PollJobEvent, Warnings, error) { var allWarnings Warnings org, actorWarnings, err := actor.GetOrganizationByName(orgName) allWarnings = append(allWarnings, actorWarnings...) if err != nil { - return allWarnings, err + return nil, allWarnings, err } space, warnings, err := actor.GetSpaceByNameAndOrganization(spaceName, org.GUID) allWarnings = append(allWarnings, warnings...) if err != nil { - return allWarnings, err + return nil, allWarnings, err } jobURL, deleteWarnings, err := actor.CloudControllerClient.DeleteSpace(space.GUID) allWarnings = append(allWarnings, Warnings(deleteWarnings)...) if err != nil { - return allWarnings, err + return nil, allWarnings, err } - ccWarnings, err := actor.CloudControllerClient.PollJob(jobURL) - allWarnings = append(allWarnings, Warnings(ccWarnings)...) - - return allWarnings, err + return actor.PollJobToEventStream(jobURL), allWarnings, nil } func (actor Actor) RenameSpaceByNameAndOrganizationGUID(oldSpaceName, newSpaceName, orgGUID string) (resources.Space, Warnings, error) { diff --git a/actor/v7action/space_test.go b/actor/v7action/space_test.go index 358ecb9308b..eea62ecbe17 100644 --- a/actor/v7action/space_test.go +++ b/actor/v7action/space_test.go @@ -420,8 +420,28 @@ var _ = Describe("Space", func() { err error ) + ccStream := func(events ...ccv3.PollJobEvent) chan ccv3.PollJobEvent { + stream := make(chan ccv3.PollJobEvent, len(events)) + for _, e := range events { + stream <- e + } + close(stream) + return stream + } + JustBeforeEach(func() { - warnings, err = actor.DeleteSpaceByNameAndOrganizationName("some-space", "some-org") + var stream chan PollJobEvent + var upfront Warnings + stream, upfront, err = actor.DeleteSpaceByNameAndOrganizationName("some-space", "some-org") + warnings = upfront + if stream != nil { + for event := range stream { + warnings = append(warnings, event.Warnings...) + if event.Err != nil { + err = event.Err + } + } + } }) When("the org is not found", func() { @@ -509,7 +529,11 @@ var _ = Describe("Space", func() { BeforeEach(func() { expectedErr = errors.New("Never expected, by anyone") - fakeCloudControllerClient.PollJobReturns(ccv3.Warnings{"warning-7", "warning-8"}, expectedErr) + fakeCloudControllerClient.PollJobToEventStreamReturns(ccStream(ccv3.PollJobEvent{ + State: constant.JobFailed, + Warnings: ccv3.Warnings{"warning-7", "warning-8"}, + Err: expectedErr, + })) }) It("returns the error", func() { @@ -520,7 +544,10 @@ var _ = Describe("Space", func() { When("the job is successful", func() { BeforeEach(func() { - fakeCloudControllerClient.PollJobReturns(ccv3.Warnings{"warning-7", "warning-8"}, nil) + fakeCloudControllerClient.PollJobToEventStreamReturns(ccStream(ccv3.PollJobEvent{ + State: constant.JobComplete, + Warnings: ccv3.Warnings{"warning-7", "warning-8"}, + })) }) It("returns warnings and no error", func() { @@ -545,8 +572,8 @@ var _ = Describe("Space", func() { Expect(fakeCloudControllerClient.DeleteSpaceCallCount()).To(Equal(1)) Expect(fakeCloudControllerClient.DeleteSpaceArgsForCall(0)).To(Equal("some-space-guid")) - Expect(fakeCloudControllerClient.PollJobCallCount()).To(Equal(1)) - Expect(fakeCloudControllerClient.PollJobArgsForCall(0)).To(Equal(ccv3.JobURL("some-url"))) + Expect(fakeCloudControllerClient.PollJobToEventStreamCallCount()).To(Equal(1)) + Expect(fakeCloudControllerClient.PollJobToEventStreamArgsForCall(0)).To(Equal(ccv3.JobURL("some-url"))) }) }) }) diff --git a/command/v7/actor.go b/command/v7/actor.go index 8874df4015a..5ebc63bd47c 100644 --- a/command/v7/actor.go +++ b/command/v7/actor.go @@ -58,13 +58,13 @@ type Actor interface { CreateSpaceRole(roleType constant.RoleType, orgGUID string, spaceGUID string, userNameOrGUID string, userOrigin string, isClient bool) (v7action.Warnings, error) CreateUser(username string, password string, origin string) (resources.User, v7action.Warnings, error) CreateUserProvidedServiceInstance(instance resources.ServiceInstance) (v7action.Warnings, error) - DeleteApplicationByNameAndSpace(name, spaceGUID string, deleteRoutes bool) (v7action.Warnings, error) + DeleteApplicationByNameAndSpace(name, spaceGUID string, deleteRoutes bool) (chan v7action.PollJobEvent, v7action.Warnings, error) DeleteRoutePolicyBySource(domainName, source, hostname, path string) (v7action.Warnings, error) DeleteBuildpackByNameAndStackAndLifecycle(buildpackName string, buildpackStack string, buildpackLifecycle string) (v7action.Warnings, error) DeleteDomain(domain resources.Domain) (v7action.Warnings, error) DeleteInstanceByApplicationNameSpaceProcessTypeAndIndex(appName string, spaceGUID string, processType string, instanceIndex int) (v7action.Warnings, error) DeleteOrgRole(roleType constant.RoleType, orgGUID string, userNameOrGUID string, userOrigin string, isClient bool) (v7action.Warnings, error) - DeleteOrganization(orgName string) (v7action.Warnings, error) + DeleteOrganization(orgName string) (chan v7action.PollJobEvent, v7action.Warnings, error) DeleteOrganizationQuota(quotaName string) (v7action.Warnings, error) DeleteOrphanedRoutes(spaceGUID string) (v7action.Warnings, error) DeleteRoute(domainName, hostname, path string, port int) (v7action.Warnings, error) @@ -74,7 +74,7 @@ type Actor interface { DeleteServiceBroker(serviceBrokerGUID string) (v7action.Warnings, error) DeleteServiceInstance(serviceInstanceName, spaceGUID string) (chan v7action.PollJobEvent, v7action.Warnings, error) DeleteServiceKeyByServiceInstanceAndName(serviceInstanceName, serviceKeyName, spaceGUID string) (chan v7action.PollJobEvent, v7action.Warnings, error) - DeleteSpaceByNameAndOrganizationName(spaceName string, orgName string) (v7action.Warnings, error) + DeleteSpaceByNameAndOrganizationName(spaceName string, orgName string) (chan v7action.PollJobEvent, v7action.Warnings, error) DeleteSpaceQuotaByName(quotaName string, orgGUID string) (v7action.Warnings, error) DeleteSpaceRole(roleType constant.RoleType, spaceGUID string, userNameOrGUID string, userOrigin string, isClient bool) (v7action.Warnings, error) DeleteUser(userGuid string) (v7action.Warnings, error) diff --git a/command/v7/bind_route_service_command_test.go b/command/v7/bind_route_service_command_test.go index 4dce41620fd..9863428b382 100644 --- a/command/v7/bind_route_service_command_test.go +++ b/command/v7/bind_route_service_command_test.go @@ -304,7 +304,7 @@ var _ = Describe("bind-route-service Command", func() { It("waits for the event stream to complete", func() { Expect(testUI.Out).To(SatisfyAll( - Say(`Waiting for the operation to complete\.\.\.\n`), + Say(`Waiting for the operation to complete\n\.\.\.`), Say(`\n`), Say(`OK\n`), )) diff --git a/command/v7/bind_service_command_test.go b/command/v7/bind_service_command_test.go index 43faf4e1140..22dd0c47057 100644 --- a/command/v7/bind_service_command_test.go +++ b/command/v7/bind_service_command_test.go @@ -306,7 +306,7 @@ var _ = Describe("bind-service Command", func() { It("waits for the event stream to complete", func() { Expect(testUI.Out).To(SatisfyAll( - Say(`Waiting for the operation to complete\.\.\.\n`), + Say(`Waiting for the operation to complete\n\.\.\.`), Say(`\n`), Say(`OK\n`), Say(`\n`), diff --git a/command/v7/cleanup_outdated_service_bindings_command_test.go b/command/v7/cleanup_outdated_service_bindings_command_test.go index a9e24163e4e..7bc17182305 100644 --- a/command/v7/cleanup_outdated_service_bindings_command_test.go +++ b/command/v7/cleanup_outdated_service_bindings_command_test.go @@ -484,7 +484,7 @@ var _ = Describe("cleanup-outdated-service-bindings Command", func() { It("waits for the event stream to complete", func() { Expect(testUI.Out).To(SatisfyAll( Say(`Deleting service binding %s\.\.\.\n`, fakeBindingGUID1), - Say(`Waiting for the operation to complete\.+\n`), + Say(`Waiting for the operation to complete\n\.+`), Say(`\n`), Say(`OK\n`), )) diff --git a/command/v7/create_service_command_test.go b/command/v7/create_service_command_test.go index 631955faca3..0da4aa1e00a 100644 --- a/command/v7/create_service_command_test.go +++ b/command/v7/create_service_command_test.go @@ -277,7 +277,7 @@ var _ = Describe("create-service Command", func() { Expect(testUI.Out).To(SatisfyAll( Say(`Creating service instance %s in org %s / space %s as %s\.\.\.\n`, requestedServiceInstanceName, fakeOrgName, fakeSpaceName, fakeUserName), Say(`\n`), - Say(`Waiting for the operation to complete\.\.\n`), + Say(`Waiting for the operation to complete\n\.\.`), Say(`\n`), Say(`Service instance %s created\.\n`, requestedServiceInstanceName), Say(`OK\n`), diff --git a/command/v7/create_service_key_command_test.go b/command/v7/create_service_key_command_test.go index 4982956d8cb..24d07569509 100644 --- a/command/v7/create_service_key_command_test.go +++ b/command/v7/create_service_key_command_test.go @@ -250,7 +250,7 @@ var _ = Describe("create-service-key Command", func() { It("waits for the event stream to complete", func() { Expect(testUI.Out).To(SatisfyAll( - Say(`Waiting for the operation to complete\.\.\.\n`), + Say(`Waiting for the operation to complete\n\.\.\.`), Say(`\n`), Say(`OK\n`), )) diff --git a/command/v7/delete_command.go b/command/v7/delete_command.go index 1767fe0ea6c..8fe9508595b 100644 --- a/command/v7/delete_command.go +++ b/command/v7/delete_command.go @@ -3,6 +3,7 @@ package v7 import ( "code.cloudfoundry.org/cli/v8/actor/actionerror" "code.cloudfoundry.org/cli/v8/command/flag" + "code.cloudfoundry.org/cli/v8/command/v7/shared" ) type DeleteCommand struct { @@ -56,7 +57,7 @@ func (cmd DeleteCommand) Execute(args []string) error { "Username": currentUser.Name, }) - warnings, err := cmd.Actor.DeleteApplicationByNameAndSpace( + stream, warnings, err := cmd.Actor.DeleteApplicationByNameAndSpace( cmd.RequiredArgs.AppName, cmd.Config.TargetedSpace().GUID, cmd.DeleteMappedRoutes, @@ -68,6 +69,8 @@ func (cmd DeleteCommand) Execute(args []string) error { cmd.UI.DisplayWarning("App '{{.AppName}}' does not exist.", map[string]interface{}{ "AppName": cmd.RequiredArgs.AppName, }) + cmd.UI.DisplayOK() + return nil case actionerror.RouteBoundToMultipleAppsError: cmd.UI.DeferText( "\nTIP: Run 'cf delete {{.AppName}}' to delete the app and 'cf delete-route' to delete the route.", @@ -81,6 +84,10 @@ func (cmd DeleteCommand) Execute(args []string) error { } } + if _, err := shared.WaitForResult(stream, cmd.UI, true); err != nil { + return err + } + cmd.UI.DisplayOK() return nil diff --git a/command/v7/delete_command_test.go b/command/v7/delete_command_test.go index baa83816c32..c3cc771307f 100644 --- a/command/v7/delete_command_test.go +++ b/command/v7/delete_command_test.go @@ -64,6 +64,15 @@ var _ = Describe("delete Command", func() { fakeActor.GetCurrentUserReturns(configv3.User{Name: "steve"}, nil) }) + closedStream := func() chan v7action.PollJobEvent { + stream := make(chan v7action.PollJobEvent) + go func() { + stream <- v7action.PollJobEvent{State: v7action.JobComplete} + close(stream) + }() + return stream + } + JustBeforeEach(func() { executeErr = cmd.Execute(nil) }) @@ -75,7 +84,7 @@ var _ = Describe("delete Command", func() { _, err := input.Write([]byte("y\n")) Expect(err).ToNot(HaveOccurred()) - fakeActor.DeleteApplicationByNameAndSpaceReturns(v7action.Warnings{"some-warning"}, nil) + fakeActor.DeleteApplicationByNameAndSpaceReturns(closedStream(), v7action.Warnings{"some-warning"}, nil) }) It("asks for a special prompt about deleting associated routes", func() { @@ -97,7 +106,7 @@ var _ = Describe("delete Command", func() { When("the route is mapped to a different app", func() { BeforeEach(func() { - fakeActor.DeleteApplicationByNameAndSpaceReturns(v7action.Warnings{"some-warning"}, actionerror.RouteBoundToMultipleAppsError{}) + fakeActor.DeleteApplicationByNameAndSpaceReturns(nil, v7action.Warnings{"some-warning"}, actionerror.RouteBoundToMultipleAppsError{}) }) It("returns the error", func() { @@ -150,7 +159,7 @@ var _ = Describe("delete Command", func() { _, err := input.Write([]byte("y\n")) Expect(err).ToNot(HaveOccurred()) - fakeActor.DeleteApplicationByNameAndSpaceReturns(v7action.Warnings{"some-warning"}, nil) + fakeActor.DeleteApplicationByNameAndSpaceReturns(closedStream(), v7action.Warnings{"some-warning"}, nil) }) It("delegates to the Actor", func() { @@ -224,7 +233,7 @@ var _ = Describe("delete Command", func() { When("deleting the app errors", func() { Context("generic error", func() { BeforeEach(func() { - fakeActor.DeleteApplicationByNameAndSpaceReturns(v7action.Warnings{"some-warning"}, errors.New("some-error")) + fakeActor.DeleteApplicationByNameAndSpaceReturns(nil, v7action.Warnings{"some-warning"}, errors.New("some-error")) }) It("displays all warnings, and returns the error", func() { @@ -238,7 +247,7 @@ var _ = Describe("delete Command", func() { When("the app doesn't exist", func() { BeforeEach(func() { - fakeActor.DeleteApplicationByNameAndSpaceReturns(v7action.Warnings{"some-warning"}, actionerror.ApplicationNotFoundError{Name: "some-app"}) + fakeActor.DeleteApplicationByNameAndSpaceReturns(nil, v7action.Warnings{"some-warning"}, actionerror.ApplicationNotFoundError{Name: "some-app"}) }) It("displays all warnings, that the app wasn't found, and does not error", func() { @@ -253,7 +262,7 @@ var _ = Describe("delete Command", func() { When("the app exists", func() { BeforeEach(func() { - fakeActor.DeleteApplicationByNameAndSpaceReturns(v7action.Warnings{"some-warning"}, nil) + fakeActor.DeleteApplicationByNameAndSpaceReturns(closedStream(), v7action.Warnings{"some-warning"}, nil) }) It("displays all warnings, and does not error", func() { diff --git a/command/v7/delete_org_command.go b/command/v7/delete_org_command.go index 79a8d9ad4b7..30b16e45362 100644 --- a/command/v7/delete_org_command.go +++ b/command/v7/delete_org_command.go @@ -3,6 +3,7 @@ package v7 import ( "code.cloudfoundry.org/cli/v8/actor/actionerror" "code.cloudfoundry.org/cli/v8/command/flag" + "code.cloudfoundry.org/cli/v8/command/v7/shared" ) type DeleteOrgCommand struct { @@ -46,7 +47,7 @@ func (cmd *DeleteOrgCommand) Execute(args []string) error { "Username": user.Name, }) - warnings, err := cmd.Actor.DeleteOrganization(cmd.RequiredArgs.Organization) + stream, warnings, err := cmd.Actor.DeleteOrganization(cmd.RequiredArgs.Organization) cmd.UI.DisplayWarnings(warnings) if err != nil { switch err.(type) { @@ -57,6 +58,8 @@ func (cmd *DeleteOrgCommand) Execute(args []string) error { default: return err } + } else if _, err := shared.WaitForResult(stream, cmd.UI, true); err != nil { + return err } cmd.UI.DisplayOK() diff --git a/command/v7/delete_org_command_test.go b/command/v7/delete_org_command_test.go index 45a29468a38..26aad12d973 100644 --- a/command/v7/delete_org_command_test.go +++ b/command/v7/delete_org_command_test.go @@ -48,6 +48,15 @@ var _ = Describe("delete-org Command", func() { fakeConfig.BinaryNameReturns(binaryName) }) + closedStream := func() chan v7action.PollJobEvent { + stream := make(chan v7action.PollJobEvent) + go func() { + stream <- v7action.PollJobEvent{State: v7action.JobComplete} + close(stream) + }() + return stream + } + JustBeforeEach(func() { executeErr = cmd.Execute(nil) }) @@ -100,7 +109,7 @@ var _ = Describe("delete-org Command", func() { When("no errors are encountered", func() { BeforeEach(func() { - fakeActor.DeleteOrganizationReturns(v7action.Warnings{"warning-1", "warning-2"}, nil) + fakeActor.DeleteOrganizationReturns(closedStream(), v7action.Warnings{"warning-1", "warning-2"}, nil) }) It("does not prompt for user confirmation, displays warnings, and deletes the org", func() { @@ -123,6 +132,7 @@ var _ = Describe("delete-org Command", func() { When("the organization does not exist", func() { BeforeEach(func() { fakeActor.DeleteOrganizationReturns( + nil, v7action.Warnings{"warning-1", "warning-2"}, actionerror.OrganizationNotFoundError{ Name: "some-org", @@ -152,7 +162,7 @@ var _ = Describe("delete-org Command", func() { BeforeEach(func() { returnedErr = errors.New("some error") - fakeActor.DeleteOrganizationReturns(v7action.Warnings{"warning-1", "warning-2"}, returnedErr) + fakeActor.DeleteOrganizationReturns(nil, v7action.Warnings{"warning-1", "warning-2"}, returnedErr) }) It("returns the error, displays all warnings, and does not delete the org", func() { diff --git a/command/v7/delete_service_command_test.go b/command/v7/delete_service_command_test.go index 5bbefe52bdb..efdacea0084 100644 --- a/command/v7/delete_service_command_test.go +++ b/command/v7/delete_service_command_test.go @@ -188,7 +188,7 @@ var _ = Describe("delete-service command", func() { Expect(executeErr).NotTo(HaveOccurred()) Expect(testUI.Err).To(Say("delete warning")) Expect(testUI.Out).To(SatisfyAll( - Say(`Waiting for the operation to complete\.\.\n`), + Say(`Waiting for the operation to complete\n\.\.`), Say("\n"), Say(`Service instance %s deleted\.\n`, serviceInstanceName), Say("OK\n"), diff --git a/command/v7/delete_service_key_command_test.go b/command/v7/delete_service_key_command_test.go index 4292f3365db..aaddeb96c2e 100644 --- a/command/v7/delete_service_key_command_test.go +++ b/command/v7/delete_service_key_command_test.go @@ -324,7 +324,7 @@ var _ = Describe("delete-service-key Command", func() { It("waits for the event stream to complete", func() { Expect(testUI.Out).To(SatisfyAll( - Say(`Waiting for the operation to complete\.\.\.\n`), + Say(`Waiting for the operation to complete\n\.\.\.`), Say(`\n`), Say(`OK\n`), )) diff --git a/command/v7/delete_space_command.go b/command/v7/delete_space_command.go index 67f60388f56..61cd59e90c4 100644 --- a/command/v7/delete_space_command.go +++ b/command/v7/delete_space_command.go @@ -3,6 +3,7 @@ package v7 import ( "code.cloudfoundry.org/cli/v8/actor/actionerror" "code.cloudfoundry.org/cli/v8/command/flag" + "code.cloudfoundry.org/cli/v8/command/v7/shared" ) type DeleteSpaceCommand struct { @@ -62,7 +63,7 @@ func (cmd DeleteSpaceCommand) Execute(args []string) error { "CurrentUser": user.Name, }) - warnings, err := cmd.Actor.DeleteSpaceByNameAndOrganizationName(cmd.RequiredArgs.Space, orgName) + stream, warnings, err := cmd.Actor.DeleteSpaceByNameAndOrganizationName(cmd.RequiredArgs.Space, orgName) cmd.UI.DisplayWarnings(warnings) if err != nil { switch err.(type) { @@ -73,6 +74,8 @@ func (cmd DeleteSpaceCommand) Execute(args []string) error { default: return err } + } else if _, err := shared.WaitForResult(stream, cmd.UI, true); err != nil { + return err } cmd.UI.DisplayOK() diff --git a/command/v7/delete_space_command_test.go b/command/v7/delete_space_command_test.go index d86abce9c0c..b022f80c5c8 100644 --- a/command/v7/delete_space_command_test.go +++ b/command/v7/delete_space_command_test.go @@ -50,6 +50,15 @@ var _ = Describe("delete-space Command", func() { fakeActor.GetCurrentUserReturns(configv3.User{Name: "some-user"}, nil) }) + closedStream := func() chan v7action.PollJobEvent { + stream := make(chan v7action.PollJobEvent) + go func() { + stream <- v7action.PollJobEvent{State: v7action.JobComplete} + close(stream) + }() + return stream + } + JustBeforeEach(func() { executeErr = cmd.Execute(nil) }) @@ -116,7 +125,7 @@ var _ = Describe("delete-space Command", func() { When("the deleting the space errors", func() { BeforeEach(func() { - fakeActor.DeleteSpaceByNameAndOrganizationNameReturns(v7action.Warnings{"warning-1", "warning-2"}, actionerror.SpaceNotFoundError{Name: "some-space"}) + fakeActor.DeleteSpaceByNameAndOrganizationNameReturns(nil, v7action.Warnings{"warning-1", "warning-2"}, actionerror.SpaceNotFoundError{Name: "some-space"}) }) It("displays all warnings and does not error", func() { @@ -131,7 +140,7 @@ var _ = Describe("delete-space Command", func() { When("the deleting the space succeeds", func() { BeforeEach(func() { - fakeActor.DeleteSpaceByNameAndOrganizationNameReturns(v7action.Warnings{"warning-1", "warning-2"}, nil) + fakeActor.DeleteSpaceByNameAndOrganizationNameReturns(closedStream(), v7action.Warnings{"warning-1", "warning-2"}, nil) }) When("the user was targeted to the space", func() { @@ -190,7 +199,7 @@ var _ = Describe("delete-space Command", func() { _, err := input.Write([]byte("y\n")) Expect(err).ToNot(HaveOccurred()) - fakeActor.DeleteSpaceByNameAndOrganizationNameReturns(v7action.Warnings{"warning-1", "warning-2"}, nil) + fakeActor.DeleteSpaceByNameAndOrganizationNameReturns(closedStream(), v7action.Warnings{"warning-1", "warning-2"}, nil) }) It("deletes the space", func() { @@ -258,7 +267,7 @@ var _ = Describe("delete-space Command", func() { cmd.Org = "" cmd.Force = true fakeConfig.TargetedOrganizationReturns(configv3.Organization{Name: "some-targeted-org"}) - fakeActor.DeleteSpaceByNameAndOrganizationNameReturns(v7action.Warnings{"warning-1", "warning-2"}, nil) + fakeActor.DeleteSpaceByNameAndOrganizationNameReturns(closedStream(), v7action.Warnings{"warning-1", "warning-2"}, nil) }) It("deletes the space in the targeted org", func() { diff --git a/command/v7/shared/result_waiter.go b/command/v7/shared/result_waiter.go index 45922a9a1b2..5a454658880 100644 --- a/command/v7/shared/result_waiter.go +++ b/command/v7/shared/result_waiter.go @@ -13,7 +13,7 @@ func WaitForResult(stream chan v7action.PollJobEvent, ui command.UI, waitForComp } if waitForCompletion { - fmt.Fprint(ui.Writer(), "Waiting for the operation to complete") + fmt.Fprintln(ui.Writer(), "Waiting for the operation to complete") defer func() { ui.DisplayNewline() @@ -21,8 +21,9 @@ func WaitForResult(stream chan v7action.PollJobEvent, ui command.UI, waitForComp }() } + seen := map[string]bool{} for event := range stream { - ui.DisplayWarnings(event.Warnings) + ui.DisplayWarnings(dedupeSeenWarnings(event.Warnings, seen)) if waitForCompletion { fmt.Fprint(ui.Writer(), ".") } @@ -36,3 +37,18 @@ func WaitForResult(stream chan v7action.PollJobEvent, ui command.UI, waitForComp return true, nil } + +// dedupeSeenWarnings prints each distinct warning at most once per operation: +// CC re-sends warnings like "still in progress" on every poll tick. +func dedupeSeenWarnings(warnings v7action.Warnings, seen map[string]bool) v7action.Warnings { + var deduped v7action.Warnings + for _, warning := range warnings { + if seen[warning] { + continue + } + seen[warning] = true + deduped = append(deduped, warning) + } + + return deduped +} diff --git a/command/v7/shared/result_waiter_test.go b/command/v7/shared/result_waiter_test.go new file mode 100644 index 00000000000..2a1f84703c7 --- /dev/null +++ b/command/v7/shared/result_waiter_test.go @@ -0,0 +1,136 @@ +package shared_test + +import ( + "errors" + + "code.cloudfoundry.org/cli/v8/actor/v7action" + . "code.cloudfoundry.org/cli/v8/command/v7/shared" + "code.cloudfoundry.org/cli/v8/util/ui" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gbytes" +) + +var _ = Describe("WaitForResult", func() { + var ( + testUI *ui.UI + stream chan v7action.PollJobEvent + completed bool + err error + ) + + BeforeEach(func() { + testUI = ui.NewTestUI(nil, NewBuffer(), NewBuffer()) + }) + + When("the stream is nil (synchronous operation)", func() { + It("reports completion with no error", func() { + completed, err = WaitForResult(nil, testUI, false) + Expect(err).NotTo(HaveOccurred()) + Expect(completed).To(BeTrue()) + }) + }) + + When("not waiting for completion and the job starts polling", func() { + BeforeEach(func() { + s := make(chan v7action.PollJobEvent, 1) + stream = s + s <- v7action.PollJobEvent{State: v7action.JobPolling, Warnings: v7action.Warnings{"a warning"}} + // channel intentionally left open + }) + + It("returns not-completed and displays the warning", func() { + completed, err = WaitForResult(stream, testUI, false) + Expect(err).NotTo(HaveOccurred()) + Expect(completed).To(BeFalse()) + Expect(testUI.Err).To(Say("a warning")) + }) + }) + + When("an event carries an error", func() { + BeforeEach(func() { + s := make(chan v7action.PollJobEvent, 1) + stream = s + s <- v7action.PollJobEvent{State: v7action.JobFailed, Err: errors.New("boom")} + close(s) + }) + + It("returns the error and not-completed", func() { + completed, err = WaitForResult(stream, testUI, false) + Expect(err).To(MatchError("boom")) + Expect(completed).To(BeFalse()) + }) + }) + + When("the job completes", func() { + BeforeEach(func() { + s := make(chan v7action.PollJobEvent, 1) + stream = s + s <- v7action.PollJobEvent{State: v7action.JobComplete} + close(s) + }) + + It("returns completed with no error", func() { + completed, err = WaitForResult(stream, testUI, true) + Expect(err).NotTo(HaveOccurred()) + Expect(completed).To(BeTrue()) + }) + }) + + Describe("duplicate warning suppression", func() { + When("the same warning is re-sent on later polls", func() { + BeforeEach(func() { + s := make(chan v7action.PollJobEvent, 3) + stream = s + s <- v7action.PollJobEvent{State: v7action.JobPolling, Warnings: v7action.Warnings{"still in progress"}} + s <- v7action.PollJobEvent{State: v7action.JobPolling, Warnings: v7action.Warnings{"still in progress"}} + s <- v7action.PollJobEvent{State: v7action.JobComplete, Warnings: v7action.Warnings{"still in progress"}} + close(s) + }) + + It("prints the warning only once", func() { + completed, err = WaitForResult(stream, testUI, true) + Expect(err).NotTo(HaveOccurred()) + Expect(completed).To(BeTrue()) + Expect(testUI.Err).To(Say("still in progress")) + Expect(testUI.Err).NotTo(Say("still in progress")) + }) + }) + + When("distinct warnings arrive", func() { + BeforeEach(func() { + s := make(chan v7action.PollJobEvent, 2) + stream = s + s <- v7action.PollJobEvent{State: v7action.JobPolling, Warnings: v7action.Warnings{"first"}} + s <- v7action.PollJobEvent{State: v7action.JobComplete, Warnings: v7action.Warnings{"second"}} + close(s) + }) + + It("prints each distinct warning, preserving order", func() { + completed, err = WaitForResult(stream, testUI, true) + Expect(err).NotTo(HaveOccurred()) + Expect(testUI.Err).To(Say("first")) + Expect(testUI.Err).To(Say("second")) + }) + }) + + When("a warning recurs after a distinct warning intervened", func() { + BeforeEach(func() { + s := make(chan v7action.PollJobEvent, 3) + stream = s + s <- v7action.PollJobEvent{State: v7action.JobPolling, Warnings: v7action.Warnings{"persisted"}} + s <- v7action.PollJobEvent{State: v7action.JobPolling, Warnings: v7action.Warnings{"persisted", "changed"}} + s <- v7action.PollJobEvent{State: v7action.JobComplete, Warnings: v7action.Warnings{"persisted"}} + close(s) + }) + + It("prints each distinct warning only once for the whole operation", func() { + completed, err = WaitForResult(stream, testUI, true) + Expect(err).NotTo(HaveOccurred()) + Expect(testUI.Err).To(Say("persisted")) + Expect(testUI.Err).To(Say("changed")) + Expect(testUI.Err).NotTo(Say("persisted")) + }) + }) + }) +}) diff --git a/command/v7/unbind_route_service_command_test.go b/command/v7/unbind_route_service_command_test.go index 05910c0157e..8603e65dcd2 100644 --- a/command/v7/unbind_route_service_command_test.go +++ b/command/v7/unbind_route_service_command_test.go @@ -409,7 +409,7 @@ var _ = Describe("unbind-route-service Command", func() { It("waits for the event stream to complete", func() { Expect(testUI.Out).To(SatisfyAll( - Say(`Waiting for the operation to complete\.\.\.\n`), + Say(`Waiting for the operation to complete\n\.\.\.`), Say(`\n`), Say(`OK\n`), )) diff --git a/command/v7/unbind_service_command_test.go b/command/v7/unbind_service_command_test.go index 405bef7d728..20000beb47e 100644 --- a/command/v7/unbind_service_command_test.go +++ b/command/v7/unbind_service_command_test.go @@ -296,7 +296,7 @@ var _ = Describe("unbind-service Command", func() { It("waits for the event stream to complete", func() { Expect(testUI.Out).To(SatisfyAll( Say(`Deleting service binding %s\.\.\.\n`, fakeBindingGUID), - Say(`Waiting for the operation to complete\.+\n`), + Say(`Waiting for the operation to complete\n\.+`), Say(`\n`), Say(`OK\n`), )) diff --git a/command/v7/update_service_command_test.go b/command/v7/update_service_command_test.go index 4d81b73ddb9..a1b9045d568 100644 --- a/command/v7/update_service_command_test.go +++ b/command/v7/update_service_command_test.go @@ -231,7 +231,7 @@ var _ = Describe("update-service command", func() { Expect(testUI.Out).To(SatisfyAll( Say(`Updating service instance %s in org %s / space %s as %s...\n`, serviceInstanceName, orgName, spaceName, username), Say(`\n`), - Say(`Waiting for the operation to complete\.\.\n`), + Say(`Waiting for the operation to complete\n\.\.`), Say(`\n`), Say(`Update of service instance %s complete\.\n`, serviceInstanceName), Say(`OK\n`), diff --git a/command/v7/upgrade_service_command_test.go b/command/v7/upgrade_service_command_test.go index 3c8acb319c3..cc3fff8c6c6 100644 --- a/command/v7/upgrade_service_command_test.go +++ b/command/v7/upgrade_service_command_test.go @@ -199,7 +199,7 @@ var _ = Describe("upgrade-service command", func() { Expect(testUI.Out).To(SatisfyAll( Say(`Upgrading service instance %s in org %s / space %s as %s...\n`, serviceInstanceName, orgName, spaceName, username), Say(`\n`), - Say(`Waiting for the operation to complete\.\.\n`), + Say(`Waiting for the operation to complete\n\.\.`), Say(`\n`), Say(`Upgrade of service instance %s complete\.\n`, serviceInstanceName), Say(`OK\n`), diff --git a/command/v7/v7fakes/fake_actor.go b/command/v7/v7fakes/fake_actor.go index 3dcef96142f..4037b461001 100644 --- a/command/v7/v7fakes/fake_actor.go +++ b/command/v7/v7fakes/fake_actor.go @@ -577,7 +577,7 @@ type FakeActor struct { result1 v7action.Warnings result2 error } - DeleteApplicationByNameAndSpaceStub func(string, string, bool) (v7action.Warnings, error) + DeleteApplicationByNameAndSpaceStub func(string, string, bool) (chan v7action.PollJobEvent, v7action.Warnings, error) deleteApplicationByNameAndSpaceMutex sync.RWMutex deleteApplicationByNameAndSpaceArgsForCall []struct { arg1 string @@ -585,12 +585,14 @@ type FakeActor struct { arg3 bool } deleteApplicationByNameAndSpaceReturns struct { - result1 v7action.Warnings - result2 error + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error } deleteApplicationByNameAndSpaceReturnsOnCall map[int]struct { - result1 v7action.Warnings - result2 error + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error } DeleteBuildpackByNameAndStackAndLifecycleStub func(string, string, string) (v7action.Warnings, error) deleteBuildpackByNameAndStackAndLifecycleMutex sync.RWMutex @@ -680,18 +682,20 @@ type FakeActor struct { result1 v7action.Warnings result2 error } - DeleteOrganizationStub func(string) (v7action.Warnings, error) + DeleteOrganizationStub func(string) (chan v7action.PollJobEvent, v7action.Warnings, error) deleteOrganizationMutex sync.RWMutex deleteOrganizationArgsForCall []struct { arg1 string } deleteOrganizationReturns struct { - result1 v7action.Warnings - result2 error + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error } deleteOrganizationReturnsOnCall map[int]struct { - result1 v7action.Warnings - result2 error + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error } DeleteOrganizationQuotaStub func(string) (v7action.Warnings, error) deleteOrganizationQuotaMutex sync.RWMutex @@ -840,19 +844,21 @@ type FakeActor struct { result2 v7action.Warnings result3 error } - DeleteSpaceByNameAndOrganizationNameStub func(string, string) (v7action.Warnings, error) + DeleteSpaceByNameAndOrganizationNameStub func(string, string) (chan v7action.PollJobEvent, v7action.Warnings, error) deleteSpaceByNameAndOrganizationNameMutex sync.RWMutex deleteSpaceByNameAndOrganizationNameArgsForCall []struct { arg1 string arg2 string } deleteSpaceByNameAndOrganizationNameReturns struct { - result1 v7action.Warnings - result2 error + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error } deleteSpaceByNameAndOrganizationNameReturnsOnCall map[int]struct { - result1 v7action.Warnings - result2 error + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error } DeleteSpaceQuotaByNameStub func(string, string) (v7action.Warnings, error) deleteSpaceQuotaByNameMutex sync.RWMutex @@ -6318,7 +6324,7 @@ func (fake *FakeActor) CreateUserProvidedServiceInstanceReturnsOnCall(i int, res }{result1, result2} } -func (fake *FakeActor) DeleteApplicationByNameAndSpace(arg1 string, arg2 string, arg3 bool) (v7action.Warnings, error) { +func (fake *FakeActor) DeleteApplicationByNameAndSpace(arg1 string, arg2 string, arg3 bool) (chan v7action.PollJobEvent, v7action.Warnings, error) { fake.deleteApplicationByNameAndSpaceMutex.Lock() ret, specificReturn := fake.deleteApplicationByNameAndSpaceReturnsOnCall[len(fake.deleteApplicationByNameAndSpaceArgsForCall)] fake.deleteApplicationByNameAndSpaceArgsForCall = append(fake.deleteApplicationByNameAndSpaceArgsForCall, struct { @@ -6334,9 +6340,9 @@ func (fake *FakeActor) DeleteApplicationByNameAndSpace(arg1 string, arg2 string, return stub(arg1, arg2, arg3) } if specificReturn { - return ret.result1, ret.result2 + return ret.result1, ret.result2, ret.result3 } - return fakeReturns.result1, fakeReturns.result2 + return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3 } func (fake *FakeActor) DeleteApplicationByNameAndSpaceCallCount() int { @@ -6345,7 +6351,7 @@ func (fake *FakeActor) DeleteApplicationByNameAndSpaceCallCount() int { return len(fake.deleteApplicationByNameAndSpaceArgsForCall) } -func (fake *FakeActor) DeleteApplicationByNameAndSpaceCalls(stub func(string, string, bool) (v7action.Warnings, error)) { +func (fake *FakeActor) DeleteApplicationByNameAndSpaceCalls(stub func(string, string, bool) (chan v7action.PollJobEvent, v7action.Warnings, error)) { fake.deleteApplicationByNameAndSpaceMutex.Lock() defer fake.deleteApplicationByNameAndSpaceMutex.Unlock() fake.DeleteApplicationByNameAndSpaceStub = stub @@ -6358,30 +6364,33 @@ func (fake *FakeActor) DeleteApplicationByNameAndSpaceArgsForCall(i int) (string return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } -func (fake *FakeActor) DeleteApplicationByNameAndSpaceReturns(result1 v7action.Warnings, result2 error) { +func (fake *FakeActor) DeleteApplicationByNameAndSpaceReturns(result1 chan v7action.PollJobEvent, result2 v7action.Warnings, result3 error) { fake.deleteApplicationByNameAndSpaceMutex.Lock() defer fake.deleteApplicationByNameAndSpaceMutex.Unlock() fake.DeleteApplicationByNameAndSpaceStub = nil fake.deleteApplicationByNameAndSpaceReturns = struct { - result1 v7action.Warnings - result2 error - }{result1, result2} + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error + }{result1, result2, result3} } -func (fake *FakeActor) DeleteApplicationByNameAndSpaceReturnsOnCall(i int, result1 v7action.Warnings, result2 error) { +func (fake *FakeActor) DeleteApplicationByNameAndSpaceReturnsOnCall(i int, result1 chan v7action.PollJobEvent, result2 v7action.Warnings, result3 error) { fake.deleteApplicationByNameAndSpaceMutex.Lock() defer fake.deleteApplicationByNameAndSpaceMutex.Unlock() fake.DeleteApplicationByNameAndSpaceStub = nil if fake.deleteApplicationByNameAndSpaceReturnsOnCall == nil { fake.deleteApplicationByNameAndSpaceReturnsOnCall = make(map[int]struct { - result1 v7action.Warnings - result2 error + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error }) } fake.deleteApplicationByNameAndSpaceReturnsOnCall[i] = struct { - result1 v7action.Warnings - result2 error - }{result1, result2} + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error + }{result1, result2, result3} } func (fake *FakeActor) DeleteBuildpackByNameAndStackAndLifecycle(arg1 string, arg2 string, arg3 string) (v7action.Warnings, error) { @@ -6778,7 +6787,7 @@ func (fake *FakeActor) DeleteOrgRoleReturnsOnCall(i int, result1 v7action.Warnin }{result1, result2} } -func (fake *FakeActor) DeleteOrganization(arg1 string) (v7action.Warnings, error) { +func (fake *FakeActor) DeleteOrganization(arg1 string) (chan v7action.PollJobEvent, v7action.Warnings, error) { fake.deleteOrganizationMutex.Lock() ret, specificReturn := fake.deleteOrganizationReturnsOnCall[len(fake.deleteOrganizationArgsForCall)] fake.deleteOrganizationArgsForCall = append(fake.deleteOrganizationArgsForCall, struct { @@ -6792,9 +6801,9 @@ func (fake *FakeActor) DeleteOrganization(arg1 string) (v7action.Warnings, error return stub(arg1) } if specificReturn { - return ret.result1, ret.result2 + return ret.result1, ret.result2, ret.result3 } - return fakeReturns.result1, fakeReturns.result2 + return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3 } func (fake *FakeActor) DeleteOrganizationCallCount() int { @@ -6803,7 +6812,7 @@ func (fake *FakeActor) DeleteOrganizationCallCount() int { return len(fake.deleteOrganizationArgsForCall) } -func (fake *FakeActor) DeleteOrganizationCalls(stub func(string) (v7action.Warnings, error)) { +func (fake *FakeActor) DeleteOrganizationCalls(stub func(string) (chan v7action.PollJobEvent, v7action.Warnings, error)) { fake.deleteOrganizationMutex.Lock() defer fake.deleteOrganizationMutex.Unlock() fake.DeleteOrganizationStub = stub @@ -6816,30 +6825,33 @@ func (fake *FakeActor) DeleteOrganizationArgsForCall(i int) string { return argsForCall.arg1 } -func (fake *FakeActor) DeleteOrganizationReturns(result1 v7action.Warnings, result2 error) { +func (fake *FakeActor) DeleteOrganizationReturns(result1 chan v7action.PollJobEvent, result2 v7action.Warnings, result3 error) { fake.deleteOrganizationMutex.Lock() defer fake.deleteOrganizationMutex.Unlock() fake.DeleteOrganizationStub = nil fake.deleteOrganizationReturns = struct { - result1 v7action.Warnings - result2 error - }{result1, result2} + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error + }{result1, result2, result3} } -func (fake *FakeActor) DeleteOrganizationReturnsOnCall(i int, result1 v7action.Warnings, result2 error) { +func (fake *FakeActor) DeleteOrganizationReturnsOnCall(i int, result1 chan v7action.PollJobEvent, result2 v7action.Warnings, result3 error) { fake.deleteOrganizationMutex.Lock() defer fake.deleteOrganizationMutex.Unlock() fake.DeleteOrganizationStub = nil if fake.deleteOrganizationReturnsOnCall == nil { fake.deleteOrganizationReturnsOnCall = make(map[int]struct { - result1 v7action.Warnings - result2 error + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error }) } fake.deleteOrganizationReturnsOnCall[i] = struct { - result1 v7action.Warnings - result2 error - }{result1, result2} + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error + }{result1, result2, result3} } func (fake *FakeActor) DeleteOrganizationQuota(arg1 string) (v7action.Warnings, error) { @@ -7503,7 +7515,7 @@ func (fake *FakeActor) DeleteServiceKeyByServiceInstanceAndNameReturnsOnCall(i i }{result1, result2, result3} } -func (fake *FakeActor) DeleteSpaceByNameAndOrganizationName(arg1 string, arg2 string) (v7action.Warnings, error) { +func (fake *FakeActor) DeleteSpaceByNameAndOrganizationName(arg1 string, arg2 string) (chan v7action.PollJobEvent, v7action.Warnings, error) { fake.deleteSpaceByNameAndOrganizationNameMutex.Lock() ret, specificReturn := fake.deleteSpaceByNameAndOrganizationNameReturnsOnCall[len(fake.deleteSpaceByNameAndOrganizationNameArgsForCall)] fake.deleteSpaceByNameAndOrganizationNameArgsForCall = append(fake.deleteSpaceByNameAndOrganizationNameArgsForCall, struct { @@ -7518,9 +7530,9 @@ func (fake *FakeActor) DeleteSpaceByNameAndOrganizationName(arg1 string, arg2 st return stub(arg1, arg2) } if specificReturn { - return ret.result1, ret.result2 + return ret.result1, ret.result2, ret.result3 } - return fakeReturns.result1, fakeReturns.result2 + return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3 } func (fake *FakeActor) DeleteSpaceByNameAndOrganizationNameCallCount() int { @@ -7529,7 +7541,7 @@ func (fake *FakeActor) DeleteSpaceByNameAndOrganizationNameCallCount() int { return len(fake.deleteSpaceByNameAndOrganizationNameArgsForCall) } -func (fake *FakeActor) DeleteSpaceByNameAndOrganizationNameCalls(stub func(string, string) (v7action.Warnings, error)) { +func (fake *FakeActor) DeleteSpaceByNameAndOrganizationNameCalls(stub func(string, string) (chan v7action.PollJobEvent, v7action.Warnings, error)) { fake.deleteSpaceByNameAndOrganizationNameMutex.Lock() defer fake.deleteSpaceByNameAndOrganizationNameMutex.Unlock() fake.DeleteSpaceByNameAndOrganizationNameStub = stub @@ -7542,30 +7554,33 @@ func (fake *FakeActor) DeleteSpaceByNameAndOrganizationNameArgsForCall(i int) (s return argsForCall.arg1, argsForCall.arg2 } -func (fake *FakeActor) DeleteSpaceByNameAndOrganizationNameReturns(result1 v7action.Warnings, result2 error) { +func (fake *FakeActor) DeleteSpaceByNameAndOrganizationNameReturns(result1 chan v7action.PollJobEvent, result2 v7action.Warnings, result3 error) { fake.deleteSpaceByNameAndOrganizationNameMutex.Lock() defer fake.deleteSpaceByNameAndOrganizationNameMutex.Unlock() fake.DeleteSpaceByNameAndOrganizationNameStub = nil fake.deleteSpaceByNameAndOrganizationNameReturns = struct { - result1 v7action.Warnings - result2 error - }{result1, result2} + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error + }{result1, result2, result3} } -func (fake *FakeActor) DeleteSpaceByNameAndOrganizationNameReturnsOnCall(i int, result1 v7action.Warnings, result2 error) { +func (fake *FakeActor) DeleteSpaceByNameAndOrganizationNameReturnsOnCall(i int, result1 chan v7action.PollJobEvent, result2 v7action.Warnings, result3 error) { fake.deleteSpaceByNameAndOrganizationNameMutex.Lock() defer fake.deleteSpaceByNameAndOrganizationNameMutex.Unlock() fake.DeleteSpaceByNameAndOrganizationNameStub = nil if fake.deleteSpaceByNameAndOrganizationNameReturnsOnCall == nil { fake.deleteSpaceByNameAndOrganizationNameReturnsOnCall = make(map[int]struct { - result1 v7action.Warnings - result2 error + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error }) } fake.deleteSpaceByNameAndOrganizationNameReturnsOnCall[i] = struct { - result1 v7action.Warnings - result2 error - }{result1, result2} + result1 chan v7action.PollJobEvent + result2 v7action.Warnings + result3 error + }{result1, result2, result3} } func (fake *FakeActor) DeleteSpaceQuotaByName(arg1 string, arg2 string) (v7action.Warnings, error) {