Skip to content

Commit

Permalink
[Bugfix] Idempotency when trying to share a service twice (#2875)
Browse files Browse the repository at this point in the history
  • Loading branch information
vchrisb authored May 8, 2024
1 parent b74ae37 commit 8494540
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package ccerror

// ServiceInstanceAlreadySharedError is returned when a
// service instance is already shared.
type ServiceInstanceAlreadySharedError struct {
Message string
}

func (e ServiceInstanceAlreadySharedError) Error() string {
return e.Message
}
3 changes: 3 additions & 0 deletions api/cloudcontroller/ccv3/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ func handleUnprocessableEntity(errorResponse ccerror.V3Error) error {
roleExistsRegexp := regexp.MustCompile(`User '.*' already has '.*' role.*`)
quotaExistsRegexp := regexp.MustCompile(`.* Quota '.*' already exists\.`)
securityGroupExistsRegexp := regexp.MustCompile(`Security group with name '.*' already exists\.`)
serviceInstanceSharedRegexp := regexp.MustCompile(`A service instance called .* has already been shared with .*\.`)

// boolean switch case with partial/regex string matchers
switch {
Expand All @@ -174,6 +175,8 @@ func handleUnprocessableEntity(errorResponse ccerror.V3Error) error {
case strings.Contains(errorString,
"The service instance name is taken"):
return ccerror.ServiceInstanceNameTakenError{Message: err.Message}
case serviceInstanceSharedRegexp.MatchString(errorString):
return ccerror.ServiceInstanceAlreadySharedError{Message: err.Message}
case orgNameTakenRegexp.MatchString(errorString):
return ccerror.OrganizationNameTakenError{UnprocessableEntityError: err}
case roleExistsRegexp.MatchString(errorString):
Expand Down
21 changes: 21 additions & 0 deletions api/cloudcontroller/ccv3/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,27 @@ var _ = Describe("Error Wrapper", func() {
})
})

When("the service instance has already been shared", func() {
BeforeEach(func() {
serverResponse = `
{
"errors": [
{
"code": 10008,
"detail": "A service instance called foo has already been shared with foo-space.",
"title": "CF-UnprocessableEntity"
}
]
}`
})

It("returns an ServiceInstanceAlreadySharedError", func() {
Expect(makeError).To(MatchError(ccerror.ServiceInstanceAlreadySharedError{
Message: "A service instance called foo has already been shared with foo-space.",
}))
})
})

When("the buildpack is invalid", func() {
BeforeEach(func() {
serverResponse = `
Expand Down
10 changes: 9 additions & 1 deletion command/v7/share_service_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package v7

import (
"code.cloudfoundry.org/cli/actor/v7action"
"code.cloudfoundry.org/cli/api/cloudcontroller/ccerror"
"code.cloudfoundry.org/cli/command/flag"
"code.cloudfoundry.org/cli/types"
)
Expand Down Expand Up @@ -38,7 +39,14 @@ func (cmd ShareServiceCommand) Execute(args []string) error {
})

cmd.UI.DisplayWarnings(warnings)
if err != nil {

switch err.(type) {
case nil:
case ccerror.ServiceInstanceAlreadySharedError:
cmd.UI.DisplayOK()
cmd.UI.DisplayTextWithFlavor("A service instance called {{.ServiceInstanceName}} has already been shared", map[string]interface{}{"ServiceInstanceName": cmd.RequiredArgs.ServiceInstance})
return nil
default:
return err
}

Expand Down
18 changes: 18 additions & 0 deletions command/v7/share_service_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package v7_test
import (
"errors"

"code.cloudfoundry.org/cli/api/cloudcontroller/ccerror"
"code.cloudfoundry.org/cli/util/configv3"

"code.cloudfoundry.org/cli/command/flag"
Expand Down Expand Up @@ -141,6 +142,23 @@ var _ = Describe("share-service command", func() {
})
})

When("the service instance is already shared", func() {
BeforeEach(func() {
fakeActor.ShareServiceInstanceToSpaceAndOrgReturns(v7action.Warnings{"warning one", "warning two"}, ccerror.ServiceInstanceAlreadySharedError{})
})

It("succeeds, displaying warnings, 'OK' and an informative message", func() {
Expect(executeErr).NotTo(HaveOccurred())

Expect(testUI.Out).To(Say(`OK`))
Expect(testUI.Err).To(SatisfyAll(
Say("warning one"),
Say("warning two"),
))
Expect(testUI.Out).To(Say("A service instance called %s has already been shared", expectedServiceInstanceName))
})
})

When("the actor errors", func() {
BeforeEach(func() {
fakeSharedActor.CheckTargetReturns(nil)
Expand Down

0 comments on commit 8494540

Please sign in to comment.