Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add module generator option #1400

Merged
merged 2 commits into from
May 26, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions protoc-gen-grpc-gateway/internal/gengateway/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ type generator struct {
useRequestContext bool
registerFuncSuffix string
pathType pathType
modulePath string
allowPatchFeature bool
}

// New returns a new generator which generates grpc gateway files.
func New(reg *descriptor.Registry, useRequestContext bool, registerFuncSuffix, pathTypeString string, allowPatchFeature bool) gen.Generator {
func New(reg *descriptor.Registry, useRequestContext bool, registerFuncSuffix, pathTypeString, modulePathString string, allowPatchFeature bool) gen.Generator {
var imports []descriptor.GoPackage
for _, pkgpath := range []string{
"context",
Expand Down Expand Up @@ -84,6 +85,7 @@ func New(reg *descriptor.Registry, useRequestContext bool, registerFuncSuffix, p
useRequestContext: useRequestContext,
registerFuncSuffix: registerFuncSuffix,
pathType: pathType,
modulePath: modulePathString,
allowPatchFeature: allowPatchFeature,
}
}
Expand All @@ -105,9 +107,10 @@ func (g *generator) Generate(targets []*descriptor.File) ([]*plugin.CodeGenerato
glog.Errorf("%v: %s", err, code)
return nil, err
}
name := file.GetName()
if g.pathType == pathTypeImport && file.GoPkg.Path != "" {
name = fmt.Sprintf("%s/%s", file.GoPkg.Path, filepath.Base(name))
name, err := g.getFilePath(file)
if err != nil {
glog.Errorf("%v: %s", err, code)
return nil, err
}
ext := filepath.Ext(name)
base := strings.TrimSuffix(name, ext)
Expand All @@ -121,6 +124,24 @@ func (g *generator) Generate(targets []*descriptor.File) ([]*plugin.CodeGenerato
return files, nil
}

func (g *generator) getFilePath(file *descriptor.File) (string, error) {
name := file.GetName()
if g.modulePath != "" {
if g.pathType != pathTypeImport {
return "", errors.New("cannot use module= with paths=source_relative")
}
trimPath, pkgPath := g.modulePath+"/", file.GoPkg.Path+"/"
if !strings.HasPrefix(pkgPath, trimPath) {
return "", fmt.Errorf("%v: file go path does not match module prefix: %v", file.GoPkg.Path, trimPath)
}
name = filepath.Join(strings.TrimPrefix(pkgPath, trimPath), filepath.Base(name))
} else if g.pathType == pathTypeImport && file.GoPkg.Path != "" {
// Only respect the path value if module is not supplied
name = fmt.Sprintf("%s/%s", file.GoPkg.Path, filepath.Base(name))
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whenever I see something like else if I think it can usually be done neater with a switch statement. What do you think about:

Suggested change
if g.modulePath != "" {
if g.pathType != pathTypeImport {
return "", errors.New("cannot use module= with paths=source_relative")
}
trimPath, pkgPath := g.modulePath+"/", file.GoPkg.Path+"/"
if !strings.HasPrefix(pkgPath, trimPath) {
return "", fmt.Errorf("%v: file go path does not match module prefix: %v", file.GoPkg.Path, trimPath)
}
name = filepath.Join(strings.TrimPrefix(pkgPath, trimPath), filepath.Base(name))
} else if g.pathType == pathTypeImport && file.GoPkg.Path != "" {
// Only respect the path value if module is not supplied
name = fmt.Sprintf("%s/%s", file.GoPkg.Path, filepath.Base(name))
}
switch {
case g.modulePath != "" && g.pathType != pathTypeImport:
return "", errors.New("cannot use module= with paths=source_relative")
case g.modulePath != "":
trimPath, pkgPath := g.modulePath+"/", file.GoPkg.Path+"/"
if !strings.HasPrefix(pkgPath, trimPath) {
return "", fmt.Errorf("%v: file go path does not match module prefix: %v", file.GoPkg.Path, trimPath)
}
name = filepath.Join(strings.TrimPrefix(pkgPath, trimPath), filepath.Base(name))
case g.pathType == pathTypeImport && file.GoPkg.Path != "":
// Only respect the path value if module is not supplied
name = fmt.Sprintf("%s/%s", file.GoPkg.Path, filepath.Base(name))
}

return name, nil
}

func (g *generator) generate(file *descriptor.File) (string, error) {
pkgSeen := make(map[string]bool)
var imports []descriptor.GoPackage
Expand Down
77 changes: 73 additions & 4 deletions protoc-gen-grpc-gateway/internal/gengateway/generator_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gengateway

import (
"errors"
"path/filepath"
"strings"
"testing"
Expand Down Expand Up @@ -100,9 +101,11 @@ func TestGenerateServiceWithoutBindings(t *testing.T) {

func TestGenerateOutputPath(t *testing.T) {
cases := []struct {
file *descriptor.File
pathType pathType
expected string
file *descriptor.File
pathType pathType
modulePath string
expected string
expectedError error
}{
{
file: newExampleFileDescriptorWithGoPkg(
Expand Down Expand Up @@ -142,13 +145,79 @@ func TestGenerateOutputPath(t *testing.T) {
pathType: pathTypeSourceRelative,
expected: ".",
},
{
file: newExampleFileDescriptorWithGoPkg(
&descriptor.GoPackage{
Path: "example.com/path/root",
Name: "example_pb",
},
),
modulePath: "example.com/path/root",
expected: ".",
},
{
file: newExampleFileDescriptorWithGoPkg(
&descriptor.GoPackage{
Path: "example.com/path/to/example",
Name: "example_pb",
},
),
modulePath: "example.com/path/to",
expected: "example",
},
{
file: newExampleFileDescriptorWithGoPkg(
&descriptor.GoPackage{
Path: "example.com/path/to/example/with/many/nested/paths",
Name: "example_pb",
},
),
modulePath: "example.com/path/to",
expected: "example/with/many/nested/paths",
},

// Error cases
{
file: newExampleFileDescriptorWithGoPkg(
&descriptor.GoPackage{
Path: "example.com/path/root",
Name: "example_pb",
},
),
modulePath: "example.com/path/root",
pathType: pathTypeSourceRelative, // Not allowed
expectedError: errors.New("cannot use module= with paths=source_relative"),
},
{
file: newExampleFileDescriptorWithGoPkg(
&descriptor.GoPackage{
Path: "example.com/path/rootextra",
Name: "example_pb",
},
),
modulePath: "example.com/path/root",
expectedError: errors.New("example.com/path/rootextra: file go path does not match module prefix: example.com/path/root/"),
},
}

for _, c := range cases {
g := &generator{pathType: c.pathType}
g := &generator{
pathType: c.pathType,
modulePath: c.modulePath,
}

file := c.file
gots, err := g.Generate([]*descriptor.File{crossLinkFixture(file)})

// If we expect an error response, check it matches what we want
if c.expectedError != nil {
if err == nil || err.Error() != c.expectedError.Error() {
t.Errorf("Generate(%#v) failed with %v; wants error of: %v", file, err, c.expectedError)
}
return
}

// Handle case where we don't expect an error
if err != nil {
t.Errorf("Generate(%#v) failed with %v; wants success", file, err)
return
Expand Down
3 changes: 2 additions & 1 deletion protoc-gen-grpc-gateway/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var (
allowDeleteBody = flag.Bool("allow_delete_body", false, "unless set, HTTP DELETE methods may not have a body")
grpcAPIConfiguration = flag.String("grpc_api_configuration", "", "path to gRPC API Configuration in YAML format")
pathType = flag.String("paths", "", "specifies how the paths of generated files are structured")
modulePath = flag.String("module", "", "specifies a module prefix that will be stripped from the go package to determine the output directory")
allowRepeatedFieldsInBody = flag.Bool("allow_repeated_fields_in_body", false, "allows to use repeated field in `body` and `response_body` field of `google.api.http` annotation option")
repeatedPathParamSeparator = flag.String("repeated_path_param_separator", "csv", "configures how repeated fields should be split. Allowed values are `csv`, `pipes`, `ssv` and `tsv`.")
allowPatchFeature = flag.Bool("allow_patch_feature", true, "determines whether to use PATCH feature involving update masks (using google.protobuf.FieldMask).")
Expand Down Expand Up @@ -81,7 +82,7 @@ func main() {
}
}

g := gengateway.New(reg, *useRequestContext, *registerFuncSuffix, *pathType, *allowPatchFeature)
g := gengateway.New(reg, *useRequestContext, *registerFuncSuffix, *pathType, *modulePath, *allowPatchFeature)

if *grpcAPIConfiguration != "" {
if err := reg.LoadGrpcAPIServiceFromYAML(*grpcAPIConfiguration); err != nil {
Expand Down