idl paths per module

README.md

@@ -237,7 +237,7 @@ import (
 
 	"github.com/uber/zanzibar/examples/example-gateway/build/endpoints/contacts/module"
 	"github.com/uber/zanzibar/examples/example-gateway/build/endpoints/contacts/workflow"
-	contacts "github.com/uber/zanzibar/examples/example-gateway/build/gen-code/endpoints/contacts/contacts"
+	contacts "github.com/uber/zanzibar/examples/example-gateway/build/gen-code/endpoints-idl/endpoints/contacts/contacts"
 
 	zanzibar "github.com/uber/zanzibar/runtime"
 	"go.uber.org/zap"
@@ -425,7 +425,7 @@ package fixture
 
 import (
 	mc "github.com/uber/zanzibar/examples/example-gateway/build/clients/contacts/mock-client"
-	gen "github.com/uber/zanzibar/examples/example-gateway/build/gen-code/clients/contacts/contacts"
+	gen "github.com/uber/zanzibar/examples/example-gateway/build/gen-code/clients-idl/clients/contacts/contacts"
 )
 
 var saveContactsFixtures = &mc.SaveContactsScenarios{

benchmarks/benchserver/main.go

@@ -29,7 +29,7 @@ import (
 	"go.uber.org/zap/zapcore"
 
 	baz "github.com/uber/zanzibar/examples/example-gateway/build/clients/baz"
-	clientsBazBaz "github.com/uber/zanzibar/examples/example-gateway/build/gen-code/clients/baz/baz"
+	clientsBazBaz "github.com/uber/zanzibar/examples/example-gateway/build/gen-code/clients-idl/clients/baz/baz"
 	testBackend "github.com/uber/zanzibar/test/lib/test_backend"
 )
 

codegen/client.go

@@ -112,7 +112,7 @@ func newClientSpec(
 	h *PackageHelper,
 	annotate bool,
 ) (*ClientSpec, error) {
-	thriftFile := filepath.Join(h.ThriftIDLPath(), config.IDLFile)
+	thriftFile := filepath.Join(h.ThriftIDLPath(), h.GetModuleIdlSubDir(false), config.IDLFile)
 	mspec, err := NewModuleSpec(thriftFile, annotate, false, h)
 
 	if err != nil {
@@ -252,7 +252,7 @@ func newGRPCClientSpec(
 	instance *ModuleInstance,
 	h *PackageHelper,
 ) (*ClientSpec, error) {
-	protoFile := filepath.Join(h.ThriftIDLPath(), config.IDLFile)
+	protoFile := filepath.Join(h.ThriftIDLPath(), h.GetModuleIdlSubDir(false), config.IDLFile)
 	protoSpec, err := NewProtoModuleSpec(protoFile, false, h)
 	if err != nil {
 		return nil, errors.Wrapf(

codegen/client_test.go

@@ -434,6 +434,7 @@ func newTestPackageHelper(t *testing.T) *PackageHelper {
 		CopyrightHeader: "copyright",
 		GenCodePackage:  packageRoot + "/build/gen-code",
 		TraceKey:        "trace-key",
+		ModuleIdlSubDir: map[string]string{"endpoints": "endpoints-idl", "default": "clients-idl"},
 	}
 
 	h, err := NewPackageHelper(
@@ -488,7 +489,7 @@ func doNewClientSpecTest(t *testing.T, rawConfig []byte, clientType string) {
 	}
 	h := newTestPackageHelper(t)
 
-	idlFile := filepath.Join(h.ThriftIDLPath(), "clients/bar/bar.thrift")
+	idlFile := filepath.Join(h.ThriftIDLPath(), h.GetModuleIdlSubDir(false), "clients/bar/bar.thrift")
 	expectedSpec := &ClientSpec{
 		ModuleSpec:         nil,
 		YAMLFile:           instance.YAMLFileName,
@@ -537,7 +538,7 @@ func TestGRPCClientNewClientSpec(t *testing.T) {
 	}
 	h := newTestPackageHelper(t)
 
-	idlFile := filepath.Join(h.ThriftIDLPath(), "clients/echo/echo.proto")
+	idlFile := filepath.Join(h.ThriftIDLPath(), h.GetModuleIdlSubDir(false), "clients/echo/echo.proto")
 	expectedSpec := &ClientSpec{
 		ModuleSpec:         nil,
 		YAMLFile:           instance.YAMLFileName,

codegen/gateway.go

@@ -325,7 +325,7 @@ func NewEndpointSpec(
 	}
 
 	thriftFile := filepath.Join(
-		h.ThriftIDLPath(), endpointConfigObj["thriftFile"].(string),
+		h.ThriftIDLPath(), h.GetModuleIdlSubDir(true), endpointConfigObj["thriftFile"].(string),
 	)
 
 	mspec, err := NewModuleSpec(thriftFile, endpointType == "http", true, h)

codegen/module.go

@@ -1089,7 +1089,11 @@ func (system *ModuleSystem) populateSpec(instance *ModuleInstance) error {
 		fmt.Println("error when running computespec", err.Error())
 		return err
 	}
-	instance.genSpec = spec
+	if spec != nil {
+		instance.mu.Lock()
+		instance.genSpec = spec
+		instance.mu.Unlock()
+	}
 	// HACK: to get get of bad modules, which should not be there at first place
 	filterNilClientDeps(instance)
 	return nil
@@ -1446,7 +1450,9 @@ func (system *ModuleSystem) Build(packageRoot string, baseDirectory string, phys
 	if buildResult == nil {
 		return nil
 	}
+	instance.mu.Lock()
 	instance.genSpec = buildResult.Spec
+	instance.mu.Unlock()
 	if !commitChange {
 		return nil
 	}
@@ -1686,7 +1692,7 @@ type ModuleInstance struct {
 	// genSpec is used to share generated specs across dependencies. Generators
 	// should not mutate this directly, and should return the spec as a result.
 	// Only the module system code should mutate a module instance.
-	genSpec interface{}
+	genSpec interface{} // protected by mu
 	// PackageInfo is the name for the generated module instance
 	PackageInfo *PackageInfo
 	// ClassName is the name of the class as defined in the module system
@@ -1727,6 +1733,7 @@ type ModuleInstance struct {
 	YAMLFileRaw []byte
 	// SelectiveBuilding allows the module to be built with subset of dependencies
 	SelectiveBuilding bool
+	mu                sync.RWMutex
 }
 
 func (instance *ModuleInstance) String() string {
@@ -1735,6 +1742,8 @@ func (instance *ModuleInstance) String() string {
 
 // GeneratedSpec returns the last spec result returned for the module instance
 func (instance *ModuleInstance) GeneratedSpec() interface{} {
+	instance.mu.RLock()
+	defer instance.mu.RUnlock()
 	return instance.genSpec
 }
 

codegen/package.go

@@ -21,6 +21,7 @@
 package codegen
 
 import (
+	"fmt"
 	"path"
 	"path/filepath"
 	"strings"
@@ -30,8 +31,10 @@ import (
 )
 
 const (
-	_thriftSuffix = ".thrift"
-	_protoSuffix  = ".proto"
+	_thriftSuffix          = ".thrift"
+	_protoSuffix           = ".proto"
+	_endpointModuleName    = "endpoints"
+	_defaultModuleFallback = "default"
 )
 
 // PackageHelper manages the mapping from thrift file to generated type code and service code.
@@ -42,6 +45,8 @@ type PackageHelper struct {
 	configRoot string
 	// The absolute root directory containing thrift files
 	thriftRootDir string
+	// moduleIdlSubDir defines subdir for idl per module
+	moduleIdlSubDir map[string]string
 	// The go package name of where all the generated structs are
 	genCodePackage string
 	// The absolute directory to put the generated service code
@@ -70,7 +75,7 @@ type PackageHelper struct {
 	defaultDependencies map[string][]string
 }
 
-//NewDefaultPackageHelperOptions returns a new default PackageHelperOptions, all optional fields are set as default.
+// NewDefaultPackageHelperOptions returns a new default PackageHelperOptions, all optional fields are set as default.
 func NewDefaultPackageHelperOptions() *PackageHelperOptions {
 	return &PackageHelperOptions{}
 }
@@ -80,6 +85,8 @@ func NewDefaultPackageHelperOptions() *PackageHelperOptions {
 type PackageHelperOptions struct {
 	// relative path to the idl dir, defaults to "./idl"
 	RelThriftRootDir string
+	// subdir for idl per module
+	ModuleIdlSubDir map[string]string
 	// relative path to the target dir that will contain generated code, defaults to "./build"
 	RelTargetGenDir string
 	// relative path to the middleware config dir, defaults to ""
@@ -123,6 +130,10 @@ func (p *PackageHelperOptions) relThriftRootDir() string {
 	return "./idl"
 }
 
+func (p *PackageHelperOptions) moduleIdlSubDir() map[string]string {
+	return p.ModuleIdlSubDir
+}
+
 func (p *PackageHelperOptions) relMiddlewareConfigDir() string {
 	return p.RelMiddlewareConfigDir
 }
@@ -199,6 +210,11 @@ func NewPackageHelper(
 		return nil, errors.Wrapf(err, "cannot load default middlewares")
 	}
 
+	moduleIdlSubDir := options.moduleIdlSubDir()
+	if len(moduleIdlSubDir) == 0 {
+		moduleIdlSubDir = map[string]string{_endpointModuleName: ".", _defaultModuleFallback: "."}
+	}
+
 	p := &PackageHelper{
 		packageRoot:            packageRoot,
 		configRoot:             absConfigRoot,
@@ -215,6 +231,7 @@ func NewPackageHelper(
 		moduleSearchPaths:      options.ModuleSearchPaths,
 		defaultDependencies:    options.DefaultDependencies,
 		defaultHeaders:         options.DefaultHeaders,
+		moduleIdlSubDir:        moduleIdlSubDir,
 	}
 	return p, nil
 }
@@ -328,6 +345,22 @@ func (p PackageHelper) getRelativeFileName(idlFile string) (string, error) {
 	return idlFile[idx+len(p.thriftRootDir):], nil
 }
 
+// GetModuleIdlSubDir returns subdir for idl per module
+func (p PackageHelper) GetModuleIdlSubDir(isEndpoint bool) string {
+	className := p.getModuleClass(isEndpoint)
+	if subDir, ok := p.moduleIdlSubDir[className]; ok {
+		return subDir
+	}
+	panic(fmt.Sprintf("unrecognized module %s", className))
+}
+
+func (p PackageHelper) getModuleClass(isEndpoint bool) string {
+	if isEndpoint {
+		return _endpointModuleName
+	}
+	return _defaultModuleFallback
+}
+
 // TargetClientsInitPath returns where the clients init should go
 func (p PackageHelper) TargetClientsInitPath() string {
 	return path.Join(p.targetGenDir, "clients", "clients.go")

codegen/package_test.go

@@ -36,7 +36,7 @@ var fooThrift = filepath.Join(
 	os.Getenv("GOPATH"),
 	"/src/github.com/uber/zanzibar/",
 	"examples/example-gateway/idl/",
-	"clients/foo/foo.thrift")
+	"clients-idl/clients/foo/foo.thrift")
 
 var testCopyrightHeader = `// Copyright (c) 2018 Uber Technologies, Inc.
 //
@@ -88,7 +88,8 @@ func TestImportPath(t *testing.T) {
 	h := newPackageHelper(t)
 	p, err := h.TypeImportPath(fooThrift)
 	assert.Nil(t, err, "should not return error")
-	assert.Equal(t, "github.com/uber/zanzibar/examples/example-gateway/build/gen-code/clients/foo/foo", p, "wrong type import path")
+	assert.Equal(t, "github.com/uber/zanzibar/examples/example-gateway/build/gen-code/clients-idl/clients/foo/foo",
+		p, "wrong type import path")
 	_, err = h.TypeImportPath("/Users/xxx/go/src/github.com/uber/zanzibar/examples/example-gateway/build/idl/github.com/uber/zanzibar/clients/foo/foo.go")
 	assert.Error(t, err, "should return error for not a thrift file")
 	_, err = h.TypeImportPath("/Users/xxx/go/src/github.com/uber/zanzibar/examples/example-gateway/build/zanzibar/clients/foo/foo.thrift")
@@ -99,7 +100,7 @@ func TestTypePackageName(t *testing.T) {
 	h := newPackageHelper(t)
 	packageName, err := h.TypePackageName(fooThrift)
 	assert.Nil(t, err, "should not return error")
-	assert.Equal(t, "clientsFooFoo", packageName, "wrong package name")
+	assert.Equal(t, "clientsIDlClientsFooFoo", packageName, "wrong package name")
 	_, err = h.TypeImportPath("/Users/xxx/go/src/github.com/uber/zanzibar/examples/example-gateway/build/idl/github.com/uber/zanzibar/clients/foo/foo.txt")
 	assert.Error(t, err, "should return error for not a thrift file")
 }

codegen/post_gen_hooks.go

@@ -345,10 +345,10 @@ func WorkflowMockGenHook(h *PackageHelper, t *Template) PostGenHook {
 		errChanSize := 0
 		shouldGenMap := map[*ModuleInstance][]*EndpointSpec{}
 		for _, instance := range instances["endpoint"] {
-			if instance.genSpec == nil {
+			if instance.GeneratedSpec() == nil {
 				continue
 			}
-			endpointSpecs := instance.genSpec.([]*EndpointSpec)
+			endpointSpecs := instance.GeneratedSpec().([]*EndpointSpec)
 			for _, endpointSpec := range endpointSpecs {
 				if endpointSpec.WorkflowType == "custom" {
 					shouldGenMap[instance] = endpointSpecs