Skip to content

Commit

Permalink
Refactor ServiceBroker interface
Browse files Browse the repository at this point in the history
* asyncAllowed parameter to Provision used instead of 2 methods
* Provision must return whether or not it chose to provision async
* This reflects the service broker API more closely, as a broker does
  not have to return 202 if async is allowed, only if it chooses to
  provision asynchronously.
  • Loading branch information
Craig Furman committed Nov 30, 2015
1 parent a44d3f9 commit 724bdb1
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 51 deletions.
8 changes: 1 addition & 7 deletions api.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,7 @@ func provision(serviceBroker ServiceBroker, router httpRouter, logger lager.Logg
instanceDetailsLogKey: details,
})

async := IsAsync(false)
var err error
if acceptsIncompleteFlag == true {
async, err = serviceBroker.ProvisionAsync(instanceID, details)
} else {
err = serviceBroker.ProvisionSync(instanceID, details)
}
async, err := serviceBroker.Provision(instanceID, details, acceptsIncompleteFlag)

if err != nil {
switch err {
Expand Down
44 changes: 30 additions & 14 deletions api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,17 +357,17 @@ var _ = Describe("Service Broker API", func() {
makeInstanceProvisioningRequestWithAcceptsIncomplete(instanceID, details, acceptsIncomplete)
Expect(fakeServiceBroker.ProvisionDetails).To(Equal(details))

Expect(fakeServiceBroker.AysncProvisionInstanceIds).To(ContainElement(instanceID))
Expect(fakeServiceBroker.ProvisionedInstanceIDs).To(ContainElement(instanceID))
})

Context("when the broker chooses to provision asyncronously", func() {
BeforeEach(func() {
fakeServiceBroker = &fakes.FakeServiceBroker{
SupportsAsync: true,
InstanceLimit: 3,
}
fakeAsyncServiceBroker := &fakes.FakeAsyncServiceBroker{
*fakeServiceBroker,
FakeServiceBroker: *fakeServiceBroker,
ShouldProvisionAsync: true,
}
brokerAPI = brokerapi.New(fakeAsyncServiceBroker, brokerLogger, credentials)
})
Expand All @@ -381,11 +381,11 @@ var _ = Describe("Service Broker API", func() {
Context("when the broker chooses to provision syncronously", func() {
BeforeEach(func() {
fakeServiceBroker = &fakes.FakeServiceBroker{
SupportsAsync: false,
InstanceLimit: 3,
}
fakeAsyncServiceBroker := &fakes.FakeAsyncServiceBroker{
*fakeServiceBroker,
FakeServiceBroker: *fakeServiceBroker,
ShouldProvisionAsync: false,
}
brokerAPI = brokerapi.New(fakeAsyncServiceBroker, brokerLogger, credentials)
})
Expand All @@ -398,11 +398,9 @@ var _ = Describe("Service Broker API", func() {
})

Context("when the accepts_incomplete flag is false", func() {
It("calls Provision on the service broker with acceptsIncomplete", func() {
acceptsIncomplete := false
makeInstanceProvisioningRequestWithAcceptsIncomplete(instanceID, details, acceptsIncomplete)
Expect(fakeServiceBroker.ProvisionDetails).To(Equal(details))
Expect(fakeServiceBroker.AcceptsIncomplete).To(Equal(acceptsIncomplete))
It("returns a 201", func() {
response := makeInstanceProvisioningRequestWithAcceptsIncomplete(instanceID, details, false)
Expect(response.StatusCode).To(Equal(http.StatusCreated))
})

Context("when broker can only respond asynchronously", func() {
Expand All @@ -426,10 +424,28 @@ var _ = Describe("Service Broker API", func() {
})

Context("when the accepts_incomplete flag is missing", func() {
It("calls Provision on the service broker with acceptsIncomplete", func() {
makeInstanceProvisioningRequest(instanceID, details, "")
Expect(fakeServiceBroker.ProvisionDetails).To(Equal(details))
Expect(fakeServiceBroker.AcceptsIncomplete).To(Equal(false))
It("returns a 201", func() {
response := makeInstanceProvisioningRequest(instanceID, details, "")
Expect(response.StatusCode).To(Equal(http.StatusCreated))
})

Context("when broker can only respond asynchronously", func() {
BeforeEach(func() {
fakeServiceBroker = &fakes.FakeServiceBroker{
InstanceLimit: 3,
}
fakeAsyncServiceBroker := &fakes.FakeAsyncOnlyServiceBroker{
*fakeServiceBroker,
}
brokerAPI = brokerapi.New(fakeAsyncServiceBroker, brokerLogger, credentials)
})

It("returns a 422", func() {
acceptsIncomplete := false
response := makeInstanceProvisioningRequestWithAcceptsIncomplete(instanceID, details, acceptsIncomplete)
Expect(response.StatusCode).To(Equal(422))
Expect(response.Body).To(MatchJSON(fixture("async_required.json")))
})
})
})
})
Expand Down
48 changes: 20 additions & 28 deletions fakes/fake_service_broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,16 @@ package fakes
import "github.com/pivotal-cf/brokerapi"

type FakeServiceBroker struct {
ProvisionDetails brokerapi.ProvisionDetails
AcceptsIncomplete bool
ProvisionDetails brokerapi.ProvisionDetails

ProvisionedInstanceIDs []string
DeprovisionedInstanceIDs []string
AysncProvisionInstanceIds []string
ProvisionedInstanceIDs []string
DeprovisionedInstanceIDs []string

BoundInstanceIDs []string
BoundBindingIDs []string
BoundBindingDetails brokerapi.BindDetails

InstanceLimit int
SupportsAsync brokerapi.IsAsync

ProvisionError error
BindError error
Expand All @@ -29,6 +26,7 @@ type FakeServiceBroker struct {

type FakeAsyncServiceBroker struct {
FakeServiceBroker
ShouldProvisionAsync bool
}

type FakeAsyncOnlyServiceBroker struct {
Expand Down Expand Up @@ -76,57 +74,47 @@ func (fakeBroker *FakeServiceBroker) Services() []brokerapi.Service {
}
}

func (fakeBroker *FakeServiceBroker) ProvisionAsync(instanceID string, details brokerapi.ProvisionDetails) (brokerapi.IsAsync, error) {
fakeBroker.ProvisionDetails = details
fakeBroker.AysncProvisionInstanceIds = append(fakeBroker.AysncProvisionInstanceIds, instanceID)
return fakeBroker.SupportsAsync, nil
}

func (fakeBroker *FakeServiceBroker) ProvisionSync(instanceID string, details brokerapi.ProvisionDetails) error {
func (fakeBroker *FakeServiceBroker) Provision(instanceID string, details brokerapi.ProvisionDetails, asyncAllowed bool) (brokerapi.IsAsync, error) {
fakeBroker.BrokerCalled = true

if fakeBroker.ProvisionError != nil {
return fakeBroker.ProvisionError
return false, fakeBroker.ProvisionError
}

if len(fakeBroker.ProvisionedInstanceIDs) >= fakeBroker.InstanceLimit {
return brokerapi.ErrInstanceLimitMet
return false, brokerapi.ErrInstanceLimitMet
}

if sliceContains(instanceID, fakeBroker.ProvisionedInstanceIDs) {
return brokerapi.ErrInstanceAlreadyExists
return false, brokerapi.ErrInstanceAlreadyExists
}

fakeBroker.ProvisionDetails = details
fakeBroker.ProvisionedInstanceIDs = append(fakeBroker.ProvisionedInstanceIDs, instanceID)
return nil
return false, nil
}

func (fakeBroker *FakeAsyncServiceBroker) ProvisionSync(instanceID string, details brokerapi.ProvisionDetails) error {
func (fakeBroker *FakeAsyncServiceBroker) Provision(instanceID string, details brokerapi.ProvisionDetails, asyncAllowed bool) (brokerapi.IsAsync, error) {
fakeBroker.BrokerCalled = true

if fakeBroker.ProvisionError != nil {
return fakeBroker.ProvisionError
return false, fakeBroker.ProvisionError
}

if len(fakeBroker.ProvisionedInstanceIDs) >= fakeBroker.InstanceLimit {
return brokerapi.ErrInstanceLimitMet
return false, brokerapi.ErrInstanceLimitMet
}

if sliceContains(instanceID, fakeBroker.ProvisionedInstanceIDs) {
return brokerapi.ErrInstanceAlreadyExists
return false, brokerapi.ErrInstanceAlreadyExists
}

fakeBroker.ProvisionDetails = details
fakeBroker.ProvisionedInstanceIDs = append(fakeBroker.ProvisionedInstanceIDs, instanceID)
return nil
}

func (fakeBroker *FakeAsyncOnlyServiceBroker) ProvisionSync(instanceID string, details brokerapi.ProvisionDetails) error {
return brokerapi.ErrAsyncRequired
return brokerapi.IsAsync(fakeBroker.ShouldProvisionAsync), nil
}

func (fakeBroker *FakeAsyncOnlyServiceBroker) ProvisionAsync(instanceID string, details brokerapi.ProvisionDetails) (brokerapi.IsAsync, error) {
func (fakeBroker *FakeAsyncOnlyServiceBroker) Provision(instanceID string, details brokerapi.ProvisionDetails, asyncAllowed bool) (brokerapi.IsAsync, error) {
fakeBroker.BrokerCalled = true

if fakeBroker.ProvisionError != nil {
Expand All @@ -141,9 +129,13 @@ func (fakeBroker *FakeAsyncOnlyServiceBroker) ProvisionAsync(instanceID string,
return false, brokerapi.ErrInstanceAlreadyExists
}

if !asyncAllowed {
return true, brokerapi.ErrAsyncRequired
}

fakeBroker.ProvisionDetails = details
fakeBroker.ProvisionedInstanceIDs = append(fakeBroker.ProvisionedInstanceIDs, instanceID)
return fakeBroker.SupportsAsync, nil
return true, nil
}

func (fakeBroker *FakeServiceBroker) Deprovision(instanceID string) error {
Expand Down
3 changes: 1 addition & 2 deletions service_broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import "errors"
type ServiceBroker interface {
Services() []Service

ProvisionSync(instanceID string, details ProvisionDetails) error
ProvisionAsync(instanceID string, details ProvisionDetails) (IsAsync, error)
Provision(instanceID string, details ProvisionDetails, asyncAllowed bool) (IsAsync, error)

Deprovision(instanceID string) error

Expand Down

2 comments on commit 724bdb1

@avade
Copy link
Contributor

@avade avade commented on 724bdb1 Dec 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the docs to reflect this please?

@craigfurman
Copy link
Contributor

@craigfurman craigfurman commented on 724bdb1 Dec 1, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.