Skip to content

Commit

Permalink
Extract CC Page and Chunk sizes as configurations (#83)
Browse files Browse the repository at this point in the history
  • Loading branch information
DimitarPetrov committed Dec 6, 2019
1 parent e4934f7 commit 354952e
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 61 deletions.
2 changes: 1 addition & 1 deletion Gopkg.lock

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

2 changes: 1 addition & 1 deletion Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

[[constraint]]
name = "github.com/Peripli/service-broker-proxy"
branch = "master"
version = "=0.7.1"

[[constraint]]
name = "github.com/cloudfoundry-community/go-cfenv"
Expand Down
20 changes: 9 additions & 11 deletions cf/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package cf
import (
"fmt"

"github.com/Peripli/service-broker-proxy/pkg/sbproxy"

"github.com/Peripli/service-broker-proxy/pkg/platform"
"github.com/cloudfoundry-community/go-cfclient"

Expand All @@ -18,37 +16,37 @@ type CloudFoundryErr cfclient.CloudFoundryError
// It is used to call into the cf that the proxy deployed at.
type PlatformClient struct {
client cfclient.CloudFoundryClient
settings *sbproxy.Settings
settings *Settings
}

// Broker returns platform client which can perform platform broker operations
func (c *PlatformClient) Broker() platform.BrokerClient {
return c
func (pc *PlatformClient) Broker() platform.BrokerClient {
return pc
}

// Visibility returns platform client which can perform visibility operations
func (c *PlatformClient) Visibility() platform.VisibilityClient {
return c
func (pc *PlatformClient) Visibility() platform.VisibilityClient {
return pc
}

// CatalogFetcher returns platform client which can perform refetching of service broker catalogs
func (c *PlatformClient) CatalogFetcher() platform.CatalogFetcher {
return c
func (pc *PlatformClient) CatalogFetcher() platform.CatalogFetcher {
return pc
}

// NewClient creates a new CF cf client from the specified configuration.
func NewClient(config *Settings) (*PlatformClient, error) {
if err := config.Validate(); err != nil {
return nil, err
}
cfClient, err := config.CF.CFClientProvider(config.CF.Config)
cfClient, err := config.CF.CFClientProvider(&config.CF.Config)
if err != nil {
return nil, err
}

return &PlatformClient{
client: cfClient,
settings: &config.Settings,
settings: config,
}, nil
}

Expand Down
18 changes: 13 additions & 5 deletions cf/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,15 @@ func ccClient(URL string) (*cf.Settings, *cf.PlatformClient) {
}

func ccClientWithThrottling(URL string, maxAllowedParallelRequests int) (*cf.Settings, *cf.PlatformClient) {
cfConfig := &cfclient.Config{
cfConfig := cfclient.Config{
ApiAddress: URL,
}
config := &cf.ClientConfiguration{
Config: cfConfig,
config := &cf.Config{
ClientConfiguration: &cf.ClientConfiguration{
Config: cfConfig,
PageSize: 500,
ChunkSize: 10,
},
CFClientProvider: cfclient.NewClient,
}
settings := &cf.Settings{
Expand Down Expand Up @@ -192,8 +196,12 @@ var _ = Describe("Client", func() {
)

BeforeEach(func() {
config := &cf.ClientConfiguration{
Config: cfclient.DefaultConfig(),
config := &cf.Config{
ClientConfiguration: &cf.ClientConfiguration{
Config: *cfclient.DefaultConfig(),
PageSize: 500,
ChunkSize: 10,
},
CFClientProvider: cfclient.NewClient,
}
settings = &cf.Settings{
Expand Down
45 changes: 30 additions & 15 deletions cf/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,35 @@ import (
"github.com/spf13/pflag"
)

// ClientConfiguration type holds config info for building the cf client
type ClientConfiguration struct {
*cfclient.Config `mapstructure:"client"`
// Config type holds config info for building the cf client
type Config struct {
*ClientConfiguration `mapstructure:"client"`

// CFClientProvider delays the creation of the creation of the CF client as it does remote calls during its creation which should be delayed
// until the application is ran.
CFClientProvider func(*cfclient.Config) (*cfclient.Client, error) `mapstructure:"-"`
}

// ClientConfiguration holds cf client configurations
type ClientConfiguration struct {
cfclient.Config `mapstructure:",squash"`

PageSize int `mapstructure:"page_size"`
ChunkSize int `mapstructure:"chunk_size"`
}

// Settings type wraps the CF client configuration
type Settings struct {
sbproxy.Settings `mapstructure:",squash"`

CF *ClientConfiguration `mapstructure:"cf"`
CF *Config `mapstructure:"cf"`
}

// DefaultSettings returns the default application settings
func DefaultSettings() *Settings {
return &Settings{
Settings: *sbproxy.DefaultSettings(),
CF: DefaultClientConfiguration(),
CF: DefaultCFConfiguration(),
}
}

Expand All @@ -47,14 +55,18 @@ func (s *Settings) Validate() error {
return s.Settings.Validate()
}

// DefaultClientConfiguration creates a default config for the CF client
func DefaultClientConfiguration() *ClientConfiguration {
// DefaultCFConfiguration creates a default config for the CF client
func DefaultCFConfiguration() *Config {
cfClientConfig := cfclient.DefaultConfig()
cfClientConfig.HttpClient.Timeout = 10 * time.Second
cfClientConfig.ApiAddress = ""

return &ClientConfiguration{
Config: cfClientConfig,
return &Config{
ClientConfiguration: &ClientConfiguration{
Config: *cfClientConfig,
PageSize: 500,
ChunkSize: 10,
},
CFClientProvider: cfclient.NewClient,
}
}
Expand All @@ -65,16 +77,19 @@ func CreatePFlagsForCFClient(set *pflag.FlagSet) {
}

// Validate validates the configuration and returns appropriate errors in case it is invalid
func (c *ClientConfiguration) Validate() error {
func (c *Config) Validate() error {
if c == nil {
return fmt.Errorf("CF Client configuration missing")
}
if c.ChunkSize <= 0 {
return errors.New("CF ChunkSize must be positive")
}
if c.PageSize <= 0 || c.PageSize > 500 {
return errors.New("CF PageSize must be between 1 and 500 inclusive")
}
if c.CFClientProvider == nil {
return errors.New("CF ClientCreateFunc missing")
}
if c.Config == nil {
return errors.New("CF client configuration missing")
}
if len(c.ApiAddress) == 0 {
return errors.New("CF client configuration ApiAddress missing")
}
Expand All @@ -84,11 +99,11 @@ func (c *ClientConfiguration) Validate() error {
return nil
}

// NewConfig creates ClientConfiguration from the provided environment
// NewConfig creates Config from the provided environment
func NewConfig(env env.Environment) (*Settings, error) {
cfSettings := &Settings{
Settings: *sbproxy.DefaultSettings(),
CF: DefaultClientConfiguration(),
CF: DefaultCFConfiguration(),
}

if err := env.Unmarshal(cfSettings); err != nil {
Expand Down
24 changes: 14 additions & 10 deletions cf/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ var _ = Describe("Config", func() {

Context("when address is missing", func() {
It("returns an error", func() {
settings.CF.Config = nil
settings.CF.ApiAddress = ""
assertErrorDuringValidate()
})
})

Context("when request timeout is missing", func() {
It("returns an error", func() {
settings.CF.ApiAddress = ""
settings.CF.HttpClient.Timeout = 0
assertErrorDuringValidate()
})
})
Expand Down Expand Up @@ -96,21 +96,25 @@ var _ = Describe("Config", func() {
settings cf.Settings

envSettings = cf.Settings{
CF: &cf.ClientConfiguration{
Config: &cfclient.Config{
ApiAddress: "https://example.com",
Username: "user",
Password: "password",
ClientID: "clientid",
ClientSecret: "clientsecret",
CF: &cf.Config{
ClientConfiguration: &cf.ClientConfiguration{
Config: cfclient.Config{
ApiAddress: "https://example.com",
Username: "user",
Password: "password",
ClientID: "clientid",
ClientSecret: "clientsecret",
},
PageSize: 500,
ChunkSize: 10,
},
CFClientProvider: cfclient.NewClient,
},
Settings: *sbproxy.DefaultSettings(),
}

emptySettings = cf.Settings{
CF: &cf.ClientConfiguration{},
CF: &cf.Config{},
Settings: *sbproxy.DefaultSettings(),
}
)
Expand Down
2 changes: 1 addition & 1 deletion cf/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ var _ = Describe("CF Env", func() {
It("CF overrides are not applied", func() {
Expect(environment.Get("app.legacy_url")).Should(Equal(server.DefaultSettings().Host))
Expect(environment.Get("server.port")).Should(Equal(server.DefaultSettings().Port))
Expect(environment.Get("cf.client.apiAddress")).Should(Equal(cf.DefaultClientConfiguration().ApiAddress))
Expect(environment.Get("cf.client.apiAddress")).Should(Equal(cf.DefaultCFConfiguration().ApiAddress))
})
})

Expand Down
2 changes: 1 addition & 1 deletion cf/service_access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ var _ = Describe("Client Service Plan Access", func() {
requestChecks: expectedRequest{
Method: http.MethodGet,
Path: "/v2/service_brokers",
RawQuery: encodeQuery(query),
RawQuery: encodeQuery(query) + "&results-per-page=500",
},
reaction: reactionResponse{
Code: http.StatusOK,
Expand Down
Loading

0 comments on commit 354952e

Please sign in to comment.