Skip to content

Commit

Permalink
fix(gen): reject repeated object query params (#2383)
Browse files Browse the repository at this point in the history
Updates #2379
  • Loading branch information
noahdietz committed Jan 30, 2024
1 parent 54c764a commit f29f327
Show file tree
Hide file tree
Showing 4 changed files with 405 additions and 9 deletions.
3 changes: 3 additions & 0 deletions google-api-go-generator/gen.go
Expand Up @@ -1967,6 +1967,9 @@ func (meth *Method) generateCode() {
if opt.p.Location != "query" {
panicf("optional parameter has unsupported location %q", opt.p.Location)
}
if opt.p.Repeated && opt.p.Type == "object" {
panic(fmt.Sprintf("field %q: repeated fields of type message are prohibited as query parameters", opt.p.Name))
}
setter := initialCap(opt.p.Name)
des := opt.p.Description
des = strings.Replace(des, "Optional.", "", 1)
Expand Down
60 changes: 51 additions & 9 deletions google-api-go-generator/gen_test.go
Expand Up @@ -16,6 +16,7 @@ import (
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"google.golang.org/api/google-api-go-generator/internal/disco"
"google.golang.org/api/internal"
)
Expand Down Expand Up @@ -44,6 +45,7 @@ func TestAPIs(t *testing.T) {
"param-rename",
"quotednum",
"repeated",
"repeated_any_query_error",
"required-query",
"resource-named-service", // appengine/v1/appengine-api.json
"unfortunatedefaults",
Expand All @@ -52,6 +54,36 @@ func TestAPIs(t *testing.T) {
}
for _, name := range names {
t.Run(name, func(t *testing.T) {
defer func() {
r := recover()
wantPanic := strings.HasSuffix(name, "_error")
if r != nil && !wantPanic {
t.Fatal("unexpected panic", r)
}
if r == nil && !wantPanic {
return
}
if r == nil && wantPanic {
t.Fatal("wanted test to panic, but it didn't")
}

// compare panic message received vs. desired
got, ok := r.(string)
if !ok {
gotE, okE := r.(error)
if !okE {
t.Fatalf("panic with non-string/error input: %v", r)
}
got = gotE.Error()
}
want, err := readOrUpdate(name, got)
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff(got, string(want)); diff != "" {
t.Errorf("got(-),want(+):\n %s", diff)
}
}()
api, err := apiFromFile(filepath.Join("testdata", name+".json"))
if err != nil {
t.Fatalf("Error loading API testdata/%s.json: %v", name, err)
Expand All @@ -60,17 +92,12 @@ func TestAPIs(t *testing.T) {
if err != nil {
t.Fatalf("Error generating code for %s: %v", name, err)
}
goldenFile := filepath.Join("testdata", name+".want")
if *updateGolden {
clean := strings.Replace(string(clean), fmt.Sprintf("gdcl/%s", internal.Version), "gdcl/00000000", -1)
if err := os.WriteFile(goldenFile, []byte(clean), 0644); err != nil {
t.Fatal(err)
}
}
want, err := os.ReadFile(goldenFile)

want, err := readOrUpdate(name, string(clean))
if err != nil {
t.Fatal(err)
}

wantStr := strings.Replace(string(want), "gdcl/00000000", fmt.Sprintf("gdcl/%s", internal.Version), -1)
if !bytes.Equal([]byte(wantStr), clean) {
tf, _ := os.CreateTemp("", "api-"+name+"-got-json.")
Expand All @@ -81,12 +108,27 @@ func TestAPIs(t *testing.T) {
t.Fatal(err)
}
// NOTE: update golden files with `go test -update_golden`
t.Errorf("Output for API %s differs: diff -u %s %s", name, goldenFile, tf.Name())
t.Errorf("Output for API %s differs: diff -u %s %s", name, goldenFileName(name), tf.Name())
}
})
}
}

func readOrUpdate(name, clean string) ([]byte, error) {
goldenFile := goldenFileName(name)
if *updateGolden {
clean := strings.Replace(string(clean), fmt.Sprintf("gdcl/%s", internal.Version), "gdcl/00000000", -1)
if err := os.WriteFile(goldenFile, []byte(clean), 0644); err != nil {
return nil, err
}
}
return os.ReadFile(goldenFile)
}

func goldenFileName(name string) string {
return filepath.Join("testdata", name+".want")
}

func TestScope(t *testing.T) {
tests := [][]string{
{
Expand Down

0 comments on commit f29f327

Please sign in to comment.