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

fix(gen): reject repeated object query params #2383

Merged
merged 5 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions google-api-go-generator/gen.go
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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