Skip to content

Commit

Permalink
Merge pull request #518 from uber/client_config_2
Browse files Browse the repository at this point in the history 
Refactor client config
  • Loading branch information
@l-w-2017
l-w-2017 committed Nov 13, 2018
2 parents 3592025 + 1701b94 commit 207cc52
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 110 deletions.
84 changes: 38 additions & 46 deletions codegen/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,38 @@ type clientConfig interface {
NewClientSpec(
instance *ModuleInstance,
h *PackageHelper) (*ClientSpec, error)
customImportPath() string
fixture() *Fixture
}

// ClientSubConfig is the "config" field in the client-config.yaml for http
// ClientThriftConfig is the "config" field in the client-config.yaml for http
// client and tchannel client.
type ClientSubConfig struct {
ExposedMethods map[string]string `yaml:"exposedMethods" json:"exposedMethods" validate:"exposedMethods"`
CustomImportPath string `yaml:"customImportPath" json:"customImportPath"`
ThriftFile string `yaml:"thriftFile" json:"thriftFile" validate:"nonzero"`
ThriftFileSha string `yaml:"thriftFileSha" json:"thriftFileSha" validate:"nonzero"`
SidecarRouter string `yaml:"sidecarRouter" json:"sidecarRouter"`
Fixture *Fixture `yaml:"fixture" json:"fixture"`
type ClientThriftConfig struct {
ExposedMethods map[string]string `yaml:"exposedMethods" json:"exposedMethods" validate:"exposedMethods"`
ThriftFile string `yaml:"thriftFile" json:"thriftFile" validate:"nonzero"`
ThriftFileSha string `yaml:"thriftFileSha" json:"thriftFileSha" validate:"nonzero"`
SidecarRouter string `yaml:"sidecarRouter" json:"sidecarRouter"`
Fixture *Fixture `yaml:"fixture" json:"fixture"`
}

// Fixture specifies client fixture import path and all scenarios
type Fixture struct {
// ImportPath is the package where the user-defined Fixture global variable is contained.
// The Fixture object defines, for a given client, the standardized list of fixture scenarios for that client
ImportPath string `yaml:"importPath" json:"importPath" validate:"nonzero"`
// Scenarios is a map from zanzibar's exposed method name to a list of user-defined fixture scenarios for a client
Scenarios map[string][]string `yaml:"scenarios" json:"scenarios"`
}

// Validate the fixture configuration
func (f *Fixture) Validate(methods map[string]interface{}) error {
if f.ImportPath == "" {
return errors.New("fixture importPath is empty")
}
for m := range f.Scenarios {
if _, ok := methods[m]; !ok {
return errors.Errorf("%q is not a valid method", m)
}
}
return nil
}

func validateExposedMethods(v interface{}, param string) error {
Expand All @@ -71,10 +90,10 @@ func validateExposedMethods(v interface{}, param string) error {
return nil
}

// HTTPClientConfig represents the config for a HTTP client.
// HTTPClientConfig represents the "config" field for a HTTP client-config.yaml
type HTTPClientConfig struct {
ClientConfigBase `yaml:",inline" json:",inline"`
Config *ClientSubConfig `yaml:"config" json:"config" validate:"nonzero"`
Config *ClientThriftConfig `yaml:"config" json:"config" validate:"nonzero"`
}

func newHTTPClientConfig(raw []byte) (*HTTPClientConfig, error) {
Expand All @@ -95,7 +114,7 @@ func newHTTPClientConfig(raw []byte) (*HTTPClientConfig, error) {

func newClientSpec(
clientType string,
config *ClientSubConfig,
config *ClientThriftConfig,
instance *ModuleInstance,
h *PackageHelper,
annotate bool,
Expand Down Expand Up @@ -136,18 +155,10 @@ func (c *HTTPClientConfig) NewClientSpec(
return newClientSpec(c.Type, c.Config, instance, h, true)
}

func (c *HTTPClientConfig) customImportPath() string {
return c.Config.CustomImportPath
}

func (c *HTTPClientConfig) fixture() *Fixture {
return c.Config.Fixture
}

// TChannelClientConfig represents the config for a TChannel client.
// TChannelClientConfig represents the "config" field for a TChannel client-config.yaml
type TChannelClientConfig struct {
ClientConfigBase `yaml:",inline" json:",inline"`
Config *ClientSubConfig `yaml:"config" json:"config" validate:"nonzero"`
Config *ClientThriftConfig `yaml:"config" json:"config" validate:"nonzero"`
}

func newTChannelClientConfig(raw []byte) (*TChannelClientConfig, error) {
Expand All @@ -173,24 +184,13 @@ func (c *TChannelClientConfig) NewClientSpec(
return newClientSpec(c.Type, c.Config, instance, h, false)
}

func (c *TChannelClientConfig) customImportPath() string {
return c.Config.CustomImportPath
}

func (c *TChannelClientConfig) fixture() *Fixture {
return c.Config.Fixture
}

// CustomClientSubConfig is for custom client
type CustomClientSubConfig struct {
CustomImportPath string `yaml:"customImportPath" json:"customImportPath" validate:"nonzero"`
Fixture *Fixture `yaml:"fixture" json:"fixture"`
}

// CustomClientConfig represents the config for a custom client.
type CustomClientConfig struct {
ClientConfigBase `yaml:",inline" json:",inline"`
Config *CustomClientSubConfig `yaml:"config" json:"config" validate:"nonzero"`
Config *struct {
Fixture *Fixture `yaml:"fixture" json:"fixture"`
CustomImportPath string `yaml:"customImportPath" json:"customImportPath" validate:"nonzero"`
} `yaml:"config" json:"config" validate:"nonzero"`
}

func newCustomClientConfig(raw []byte) (*CustomClientConfig, error) {
Expand Down Expand Up @@ -229,14 +229,6 @@ func (c *CustomClientConfig) NewClientSpec(
return spec, nil
}

func (c *CustomClientConfig) customImportPath() string {
return c.Config.CustomImportPath
}

func (c *CustomClientConfig) fixture() *Fixture {
return c.Config.Fixture
}

func clientType(raw []byte) (string, error) {
clientConfig := ClientConfigBase{}
if err := yaml.Unmarshal(raw, &clientConfig); err != nil {
Expand Down
97 changes: 55 additions & 42 deletions codegen/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func TestThriftFileShaMissingValidation(t *testing.T) {
doThriftFileShaMissingTest(t, "tchannel")
}

func TestNewCustomClientConfigValidationFailure(t *testing.T) {
func TestCustomClientRequiresCustomImportPath(t *testing.T) {
configYAML := `
name: test
type: custom
Expand Down Expand Up @@ -277,14 +277,13 @@ func TestNewClientConfigGetHTTPClient(t *testing.T) {
Client: []string{"a", "b"},
},
},
Config: &ClientSubConfig{
Config: &ClientThriftConfig{
ExposedMethods: map[string]string{
"a": "method",
},
CustomImportPath: "path",
ThriftFileSha: "thriftFileSha",
ThriftFile: "clients/bar/bar.thrift",
SidecarRouter: "sidecar",
ThriftFileSha: "thriftFileSha",
ThriftFile: "clients/bar/bar.thrift",
SidecarRouter: "sidecar",
Fixture: &Fixture{
ImportPath: "import",
Scenarios: map[string][]string{
Expand All @@ -307,14 +306,13 @@ func TestNewClientConfigGetTChannelClient(t *testing.T) {
Client: []string{"a", "b"},
},
},
Config: &ClientSubConfig{
Config: &ClientThriftConfig{
ExposedMethods: map[string]string{
"a": "method",
},
CustomImportPath: "path",
ThriftFileSha: "thriftFileSha",
ThriftFile: "clients/bar/bar.thrift",
SidecarRouter: "sidecar",
ThriftFileSha: "thriftFileSha",
ThriftFile: "clients/bar/bar.thrift",
SidecarRouter: "sidecar",
Fixture: &Fixture{
ImportPath: "import",
Scenarios: map[string][]string{
Expand All @@ -337,7 +335,10 @@ func TestNewClientConfigGetCustomClient(t *testing.T) {
Client: []string{"a", "b"},
},
},
Config: &CustomClientSubConfig{
Config: &struct {
Fixture *Fixture `yaml:"fixture" json:"fixture"`
CustomImportPath string `yaml:"customImportPath" json:"customImportPath" validate:"nonzero"`
}{
CustomImportPath: "path",
Fixture: &Fixture{
ImportPath: "import",
Expand All @@ -351,36 +352,6 @@ func TestNewClientConfigGetCustomClient(t *testing.T) {
assert.Equal(t, &expectedClient, client)
}

func doGetCustomImportPathTest(t *testing.T, config string) {
client, err := newClientConfig([]byte(config))
assert.NoError(t, err)
assert.Equal(t, "path", client.customImportPath())
}

func TestGetCustomImportPath(t *testing.T) {
doGetCustomImportPathTest(t, httpClientYAML)
doGetCustomImportPathTest(t, tchannelClientYAML)
doGetCustomImportPathTest(t, customClientYAML)
}

func doGetFixtureTest(t *testing.T, config string) {
client, err := newClientConfig([]byte(config))
assert.NoError(t, err)
expectedFixture := &Fixture{
ImportPath: "import",
Scenarios: map[string][]string{
"scenario": {"s1", "s2"},
},
}
assert.Equal(t, expectedFixture, client.fixture())
}

func TestGetFixture(t *testing.T) {
doGetFixtureTest(t, httpClientYAML)
doGetFixtureTest(t, tchannelClientYAML)
doGetFixtureTest(t, customClientYAML)
}

func newTestPackageHelper(t *testing.T) *PackageHelper {
relativeGatewayPath := "../examples/example-gateway"
absGatewayPath, err := filepath.Abs(relativeGatewayPath)
Expand Down Expand Up @@ -502,3 +473,45 @@ func TestCustomClientNewClientSpec(t *testing.T) {
assert.NoError(t, errSpec)
assert.Equal(t, expectedSpec, spec)
}

func TestConfigNoFixture(t *testing.T) {
configYAML := `
name: test
type: testable
config:
customImportPath: path
`
client, err := newMockableClient([]byte(configYAML))
assert.NoError(t, err)
expectedClient := &mockableClient{
ClientConfigBase: ClientConfigBase{
Name: "test",
Type: "testable",
},
Config: &struct {
Fixture *Fixture `yaml:"fixture" json:"fixture"`
CustomImportPath string `yaml:"customImportPath" json:"customImportPath"`
}{
CustomImportPath: "path",
},
}
assert.Equal(t, expectedClient, client)
}

func TestClientFixtureImportPathMissing(t *testing.T) {
configYAML := `
name: test
type: testable
config:
customImportPath: path
fixture:
# ImportPath is missing
scenarios:
scenario:
- s1
`
_, err := newMockableClient([]byte(configYAML))
expectedErr := "testable client config validation failed: Config.Fixture.ImportPath: zero value"
assert.Error(t, err)
assert.Equal(t, expectedErr, err.Error())
}
47 changes: 25 additions & 22 deletions codegen/post_gen_hooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,33 +30,36 @@ import (
"sync/atomic"

"github.com/pkg/errors"
"gopkg.in/validator.v2"
yaml "gopkg.in/yaml.v2"
)

const (
clientInterface = "Client"
custom = "custom"
)

// Fixture specifies client fixture import path and all scenarios
type Fixture struct {
// ImportPath is the package where the user-defined Fixture global variable is contained.
// The Fixture object defines, for a given client, the standardized list of fixture scenarios for that client
ImportPath string `yaml:"importPath" json:"importPath"`
// Scenarios is a map from zanzibar's exposed method name to a list of user-defined fixture scenarios for a client
Scenarios map[string][]string `yaml:"scenarios" json:"scenarios"`
type mockableClient struct {
ClientConfigBase `yaml:",inline" json:",inline"`
Config *struct {
Fixture *Fixture `yaml:"fixture" json:"fixture"`
CustomImportPath string `yaml:"customImportPath" json:"customImportPath"`
} `yaml:"config" json:"config" validate:"nonzero"`
}

// Validate the fixture configuration
func (f *Fixture) Validate(methods map[string]interface{}) error {
if f.ImportPath == "" {
return errors.New("fixture importPath is empty")
func newMockableClient(raw []byte) (*mockableClient, error) {
config := &mockableClient{}
if errUnmarshal := yaml.Unmarshal(raw, config); errUnmarshal != nil {
return nil, errors.Wrap(
errUnmarshal, "Could not parse testable client config data")
}
for m := range f.Scenarios {
if _, ok := methods[m]; !ok {
return errors.Errorf("%q is not a valid method", m)
}

if errValidate := validator.Validate(config); errValidate != nil {
return nil, errors.Wrap(
errValidate, "testable client config validation failed")
}
return nil

return config, nil
}

// ClientMockGenHook returns a PostGenHook to generate client mocks
Expand All @@ -76,7 +79,7 @@ func ClientMockGenHook(h *PackageHelper, t *Template) (PostGenHook, error) {
pathSymbolMap := make(map[string]string)
for _, instance := range clientInstances {
key := instance.ClassType + instance.InstanceName
client, errClient := newClientConfig(instance.YAMLFileRaw)
client, errClient := newMockableClient(instance.YAMLFileRaw)
if errClient != nil {
return errors.Wrapf(
err,
Expand All @@ -87,13 +90,13 @@ func ClientMockGenHook(h *PackageHelper, t *Template) (PostGenHook, error) {

importPath := instance.PackageInfo.GeneratedPackagePath
if instance.ClassType == custom {
importPath = client.customImportPath()
importPath = client.Config.CustomImportPath
}

importPathMap[key] = importPath

// gather all modules that need to generate fixture types
f := client.fixture()
f := client.Config.Fixture
if f != nil && f.Scenarios != nil {
pathSymbolMap[importPath] = clientInterface
fixtureMap[key] = f
Expand Down Expand Up @@ -303,7 +306,7 @@ func generateMockInitializer(instance *ModuleInstance, h *PackageHelper, t *Temp
func FindClientsWithFixture(instance *ModuleInstance) (map[string]string, error) {
clientsWithFixture := map[string]string{}
for _, leaf := range instance.RecursiveDependencies["client"] {
client, err := newClientConfig(leaf.YAMLFileRaw)
client, err := newMockableClient(leaf.YAMLFileRaw)
if err != nil {
return nil, errors.Wrapf(
err,
Expand All @@ -312,8 +315,8 @@ func FindClientsWithFixture(instance *ModuleInstance) (map[string]string, error)
)
}

if client.fixture() != nil {
clientsWithFixture[leaf.InstanceName] = client.fixture().ImportPath
if client.Config.Fixture != nil {
clientsWithFixture[leaf.InstanceName] = client.Config.Fixture.ImportPath
}
}
return clientsWithFixture, nil
Expand Down

0 comments on commit 207cc52

Please sign in to comment.