Skip to content

Commit

Permalink
Simplify CCV3 internal router
Browse files Browse the repository at this point in the history
- Now, we don't need to hit `/v3` before every command to populate the
router. Instead, we have just written the resource names directly into
the routes file.
- Refactor the routes file to be a map keyed by route name, instead of
an array of routes that gets later converted into a map on the fly.
- The `GetInfo()` method now does not return `ccv3.ResourceLinks`,
because we don't hit `/v3`.

Slack discussion where we asked if anyone knew why it was so complicated
(and nobody really knew):
https://cloudfoundry.slack.com/archives/C032824SM/p1594316115396400

Co-authored-by: Merric De Launey <mdelauney@pivotal.io>
Co-authored-by: Reid Mitchell <rmitchell@pivotal.io>
  • Loading branch information
MerricdeLauney and reidmit committed Jul 15, 2020
1 parent 9654e26 commit ad36825
Show file tree
Hide file tree
Showing 23 changed files with 243 additions and 536 deletions.
2 changes: 1 addition & 1 deletion actor/v3action/cloud_controller_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type CloudControllerClient interface {
GetDeployments(query ...ccv3.Query) ([]ccv3.Deployment, ccv3.Warnings, error)
GetDroplet(guid string) (resources.Droplet, ccv3.Warnings, error)
GetDroplets(query ...ccv3.Query) ([]resources.Droplet, ccv3.Warnings, error)
GetInfo() (ccv3.Info, ccv3.ResourceLinks, ccv3.Warnings, error)
GetInfo() (ccv3.Info, ccv3.Warnings, error)
GetIsolationSegment(guid string) (ccv3.IsolationSegment, ccv3.Warnings, error)
GetIsolationSegmentOrganizations(isolationSegmentGUID string) ([]resources.Organization, ccv3.Warnings, error)
GetIsolationSegments(query ...ccv3.Query) ([]ccv3.IsolationSegment, ccv3.Warnings, error)
Expand Down
2 changes: 1 addition & 1 deletion actor/v3action/logcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
// This works around the bug logged in story https://www.pivotaltracker.com/story/show/170138644

func (actor Actor) LogCacheURL() string {
info, _, _, err := actor.CloudControllerClient.GetInfo()
info, _, err := actor.CloudControllerClient.GetInfo()
if err == nil {
logCacheURL := info.LogCache()
if logCacheURL != "" {
Expand Down
6 changes: 3 additions & 3 deletions actor/v3action/logcache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var _ = Describe("LogCacheURL", func() {
Links: ccv3.InfoLinks{
LogCache: ccv3.APILink{
HREF: configuredLogcacheURL,
}}}, ccv3.ResourceLinks{}, ccv3.Warnings{}, nil)
}}}, ccv3.Warnings{}, nil)
})
It("uses it", func() {
Expect(actualLogCacheURL).To(Equal(configuredLogcacheURL))
Expand All @@ -48,7 +48,7 @@ var _ = Describe("LogCacheURL", func() {
BeforeEach(func() {
fakeCloudControllerClient.GetInfoReturns(ccv3.Info{
Links: ccv3.InfoLinks{},
}, ccv3.ResourceLinks{}, ccv3.Warnings{}, nil)
}, ccv3.Warnings{}, nil)
})
It("uses the target", func() {
if ccversion.MinSupportedV2ClientVersion != "2.128.0" {
Expand All @@ -63,7 +63,7 @@ var _ = Describe("LogCacheURL", func() {
BeforeEach(func() {
fakeCloudControllerClient.GetInfoReturns(ccv3.Info{
Links: ccv3.InfoLinks{},
}, ccv3.ResourceLinks{}, ccv3.Warnings{}, errors.New("awf splatz!"))
}, ccv3.Warnings{}, errors.New("awf splatz!"))
})
It("uses the target", func() {
Expect(actualLogCacheURL).To(Equal("https://log-cache.the-best-domain.com"))
Expand Down
43 changes: 19 additions & 24 deletions actor/v3action/v3actionfakes/fake_cloud_controller_client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions actor/v7action/cloud_controller_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ type CloudControllerClient interface {
GetDroplets(query ...ccv3.Query) ([]resources.Droplet, ccv3.Warnings, error)
GetEnvironmentVariableGroup(group constant.EnvironmentVariableGroupName) (ccv3.EnvironmentVariables, ccv3.Warnings, error)
GetEvents(query ...ccv3.Query) ([]ccv3.Event, ccv3.Warnings, error)
GetFeatureFlag(featureFlagName string) (resources.FeatureFlag, ccv3.Warnings, error)
GetFeatureFlags() ([]resources.FeatureFlag, ccv3.Warnings, error)
GetInfo() (ccv3.Info, ccv3.ResourceLinks, ccv3.Warnings, error)
GetFeatureFlag(featureFlagName string) (ccv3.FeatureFlag, ccv3.Warnings, error)
GetFeatureFlags() ([]ccv3.FeatureFlag, ccv3.Warnings, error)
GetInfo() (ccv3.Info, ccv3.Warnings, error)
GetIsolationSegment(guid string) (ccv3.IsolationSegment, ccv3.Warnings, error)
GetIsolationSegmentOrganizations(isolationSegmentGUID string) ([]resources.Organization, ccv3.Warnings, error)
GetIsolationSegments(query ...ccv3.Query) ([]ccv3.IsolationSegment, ccv3.Warnings, error)
Expand Down
2 changes: 1 addition & 1 deletion actor/v7action/info.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package v7action

func (actor Actor) GetLogCacheEndpoint() (string, Warnings, error) {
info, _, warnings, err := actor.CloudControllerClient.GetInfo()
info, warnings, err := actor.CloudControllerClient.GetInfo()
if err != nil {
return "", Warnings(warnings), err
}
Expand Down
2 changes: 0 additions & 2 deletions actor/v7action/info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ var _ = Describe("Info Actions", func() {
LogCache: ccv3.APILink{HREF: "some-log-cache-url"},
},
},
nil,
ccv3.Warnings{"warning-1", "warning-2"},
nil,
)
Expand All @@ -54,7 +53,6 @@ var _ = Describe("Info Actions", func() {
expectedErr = errors.New("I am a CloudControllerClient Error")
fakeCloudControllerClient.GetInfoReturns(
ccv3.Info{},
nil,
ccv3.Warnings{"warning-1", "warning-2"},
expectedErr,
)
Expand Down
43 changes: 19 additions & 24 deletions actor/v7action/v7actionfakes/fake_cloud_controller_client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

96 changes: 0 additions & 96 deletions api/cloudcontroller/ccv3/ccv3_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,100 +102,4 @@ func SetupV3Response() {
RespondWith(http.StatusOK, rootResponse),
),
)

v3Response := strings.Replace(`{
"links": {
"self": {
"href": "SERVER_URL/v3"
},
"apps": {
"href": "SERVER_URL/v3/apps"
},
"tasks": {
"href": "SERVER_URL/v3/tasks"
},
"isolation_segments": {
"href": "SERVER_URL/v3/isolation_segments"
},
"builds": {
"href": "SERVER_URL/v3/builds"
},
"organizations": {
"href": "SERVER_URL/v3/organizations"
},
"organization_quotas": {
"href": "SERVER_URL/v3/organization_quotas"
},
"security_groups": {
"href": "SERVER_URL/v3/security_groups"
},
"service_brokers": {
"href": "SERVER_URL/v3/service_brokers"
},
"service_instances": {
"href": "SERVER_URL/v3/service_instances"
},
"service_offerings": {
"href": "SERVER_URL/v3/service_offerings"
},
"service_plans": {
"href": "SERVER_URL/v3/service_plans"
},
"spaces": {
"href": "SERVER_URL/v3/spaces"
},
"space_quotas": {
"href": "SERVER_URL/v3/space_quotas"
},
"packages": {
"href": "SERVER_URL/v3/packages"
},
"processes": {
"href": "SERVER_URL/v3/processes"
},
"droplets": {
"href": "SERVER_URL/v3/droplets"
},
"audit_events": {
"href": "SERVER_URL/v3/audit_events"
},
"domains": {
"href": "SERVER_URL/v3/domains"
},
"deployments": {
"href": "SERVER_URL/v3/deployments"
},
"stacks": {
"href": "SERVER_URL/v3/stacks"
},
"buildpacks": {
"href": "SERVER_URL/v3/buildpacks"
},
"feature_flags": {
"href": "SERVER_URL/v3/feature_flags"
},
"resource_matches": {
"href": "SERVER_URL/v3/resource_matches"
},
"roles": {
"href": "SERVER_URL/v3/roles"
},
"routes": {
"href": "SERVER_URL/v3/routes"
},
"users": {
"href": "SERVER_URL/v3/users"
},
"environment_variable_groups": {
"href": "SERVER_URL/v3/environment_variable_groups"
}
}
}`, "SERVER_URL", serverURL, -1)

server.AppendHandlers(
CombineHandlers(
VerifyRequest(http.MethodGet, "/v3"),
RespondWith(http.StatusOK, v3Response),
),
)
}
Loading

0 comments on commit ad36825

Please sign in to comment.