Unversioned handlers shouldn't not use versioned domain/apiresponses

[jangaraj]

Unversioned handlers e.g.:

brokerapi/handlers/deprovision.go

Line 9 in 4355274

"github.com/pivotal-cf/brokerapi/v7/domain/apiresponses"

shouldn't not import versioned apiresponses:

"github.com/pivotal-cf/brokerapi/v7/domain/apiresponses" 

but only unversioned apiresponses:

"github.com/pivotal-cf/brokerapi/domain/apiresponses" 

Otherwise handler responses with custom response code e.g. 400 are returned with 500 (http.StatusInternalServerError) response code due to used logic. Error type in this handler's switch is actually *v7/apiresponses.FailureResponse, so it is not matching right branch and default branch is used instead:

brokerapi/handlers/deprovision.go

Lines 49 to 61 in 4355274

	if err != nil {
	switch err := err.(type) {
	case *apiresponses.FailureResponse:
	logger.Error(err.LoggerAction(), err)
	h.respond(w, err.ValidatedStatusCode(logger), err.ErrorResponse())
	default:
	logger.Error(unknownErrorKey, err)
	h.respond(w, http.StatusInternalServerError, apiresponses.ErrorResponse{
	Description: err.Error(),
	})
	}
	return
	}

Users with correct usage of versioning/Go modules won't notice this issue. Generally, codes from v7 folder should be used only in v7 folder codes. Similar problems can be also with utils/middlewares.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/176678144

The labels on this github issue will be updated when the story is started.

[FelisiaM]

Hi @jangaraj

Thanks for raising this issue.

Since this is a go module with a major version higher than 1, the version needs to be in the import statement.
As of the next v8 release, we have removed the versioned folder and there is only one folder in the module so there should be straightforward to match to the right imported package.

Thanks
Felisia