Repository: googleapis/gapic-generator-go
Pull request #179 (2019-08-21 19:40:00)

SampleGen: CLI for sample configs
- Add a `--samples` flag which points at a sample config file or a directory containing sample config files
- Disambiguate sample IDs when there are multiple samples with the same ID.
- Fix a bunch of wrong yaml tags

Generated speech samples: https://github.com/hzyi-google/sample-staging/tree/master/20190821/go-output

Also, the output handling part is not handling repeated fields correctly. Bug filed: https://github.com/googleapis/gapic-generator-go/issues/180

hzyi-google: PTAL
cc/ @googleapis/samplegen 
hzyi-google: PTAL
hzyi-google: PTAL
hzyi-google: PTAL

Patch:
diff --git a/cmd/gen-go-sample/gapic.go b/cmd/gen-go-sample/gapic.go
index 803146f..5d824f4 100644
--- a/cmd/gen-go-sample/gapic.go
+++ b/cmd/gen-go-sample/gapic.go
@@ -30,7 +30,7 @@ type GAPICMethod struct {
 	// map[fieldName]ResourceName.EntityName
 	FieldNamePatterns map[string]string `yaml:"field_name_patterns"`
 
-	LongRunning LongRunningConfig `yaml: "long_running"`
+	LongRunning LongRunningConfig `yaml:"long_running"`
 }
 
 type ResourceName struct {
@@ -40,6 +40,6 @@ type ResourceName struct {
 
 // All other fields are left out because samples do not need to know polling config, and we are moving to annotations anyway
 type LongRunningConfig struct {
-	ReturnType   string `yaml: "return_type"`
-	MetadataType string `yaml: "metadata_type"`
+	ReturnType   string `yaml:"return_type"`
+	MetadataType string `yaml:"metadata_type"`
 }
diff --git a/cmd/gen-go-sample/main.go b/cmd/gen-go-sample/main.go
index a5a5864..d67ae6b 100644
--- a/cmd/gen-go-sample/main.go
+++ b/cmd/gen-go-sample/main.go
@@ -16,6 +16,9 @@ package main
 
 import (
 	"bytes"
+	"crypto/sha256"
+	"encoding/base32"
+	"encoding/json"
 	"flag"
 	"fmt"
 	"go/format"
@@ -25,6 +28,7 @@ import (
 	"path/filepath"
 	"sort"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/golang/protobuf/proto"
@@ -37,12 +41,30 @@ import (
 	yaml "gopkg.in/yaml.v2"
 )
 
+const expectedSampleConfigType = "com.google.api.codegen.samplegen.v1p2.SampleConfigProto"
+const expectedSampleConfigVersion = "1.2.0"
+
+type SampleValue []string
+
+func (v *SampleValue) String() string {
+	return strings.Join(*v, ",")
+}
+
+func (v *SampleValue) Set(value string) error {
+	*v = append(*v, value)
+	return nil
+}
+
 func main() {
 	descFname := flag.String("desc", "", "proto descriptor")
 	gapicFname := flag.String("gapic", "", "gapic config")
 	clientPkg := flag.String("clientpkg", "", "the package of the client, in format 'url/to/client/pkg;name'")
 	nofmt := flag.Bool("nofmt", false, "skip gofmt, useful for debugging code with syntax error")
 	outDir := flag.String("o", ".", "directory to write samples to")
+
+	var samplePaths SampleValue
+	flag.Var(&samplePaths, "sample", "path to a sample config file. There can be more than one --sample flag.")
+
 	flag.Parse()
 
 	gen := generator{
@@ -55,8 +77,11 @@ func main() {
 		log.Fatalf("need -clientPkg in 'url/to/client/pkg;name' format, got %q", *clientPkg)
 	}
 
-	donec := make(chan struct{})
+	var wg sync.WaitGroup
+
+	wg.Add(1)
 	go func() {
+		defer wg.Done()
 		f, err := os.Open(*gapicFname)
 		if err != nil {
 			log.Fatal(errors.E(err, "cannot read GAPIC config file"))
@@ -66,9 +91,11 @@ func main() {
 		if err := yaml.NewDecoder(f).Decode(&gen.gapic); err != nil {
 			log.Fatal(errors.E(err, "error reading GAPIC config file"))
 		}
-		donec <- struct{}{}
 	}()
+
+	wg.Add(1)
 	go func() {
+		defer wg.Done()
 		descBytes, err := ioutil.ReadFile(*descFname)
 		if err != nil {
 			log.Fatal(errors.E(err, "cannot read proto descriptor file"))
@@ -79,32 +106,81 @@ func main() {
 		}
 
 		gen.descInfo = pbinfo.Of(gen.desc.GetFile())
-		donec <- struct{}{}
 	}()
 
-	// TODO(hzyi): Read sample config
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		if err := readSampleConfigFiles(&gen, samplePaths); err != nil {
+			log.Fatal(err)
+		}
+	}()
 
 	if err := os.MkdirAll(*outDir, 0755); err != nil {
 		log.Fatal(err)
 	}
 
-	<-donec
-	<-donec
+	wg.Wait()
 
-	if err := genMethodSamples(&gen, gen.sampleConfig, *nofmt, *outDir); err != nil {
+	if err := genMethodSamples(&gen, *nofmt, *outDir); err != nil {
 		log.Fatal(err)
 	}
 
 }
 
-func genMethodSamples(gen *generator, sampConf schema_v1p2.SampleConfig, nofmt bool, outDir string) error {
-	for _, samp := range sampConf.Samples {
+func readSampleConfigFiles(gen *generator, paths []string) error {
+	for _, path := range paths {
+		f, err := os.Open(path)
+		if err != nil {
+			return errors.E(err, "cannot open sample config file: %s", path)
+		}
+		decoder := yaml.NewDecoder(f)
+		config, err := readSampleConfig(decoder, path)
+		if err != nil {
+			return err
+		}
+		gen.sampleConfig.Samples = append(gen.sampleConfig.Samples, config.Samples...)
+	}
+	return nil
+}
+
+func readSampleConfig(decoder *yaml.Decoder, fname string) (schema_v1p2.SampleConfig, error) {
+	var config schema_v1p2.SampleConfig
+	for true {
+		var sc schema_v1p2.SampleConfig
+		err := decoder.Decode(&sc)
+		if err != nil && err.Error() == "EOF" {
+			// last YAML document, all done
+			break
+		}
+		if err != nil {
+			return config, err
+		}
+		if sc.Type != expectedSampleConfigType {
+			// ignore non sample config YAMLs
+			continue
+		}
+		if sc.Version != expectedSampleConfigVersion {
+			// ignore unsupported versions
+			continue
+		}
+		config.Samples = append(config.Samples, sc.Samples...)
+	}
+	if len(config.Samples) == 0 {
+		return config, errors.E(nil, "Found no valid sample config in %q", fname)
+	}
+	return config, nil
+}
+
+func genMethodSamples(gen *generator, nofmt bool, outDir string) error {
+	gen.disambiguateSampleIDs()
+	for _, samp := range gen.sampleConfig.Samples {
 		var iface GAPICInterface
 		var method GAPICMethod
 		for _, iface = range gen.gapic.Interfaces {
 			if iface.Name == samp.Service {
 				for _, method = range iface.Methods {
-					if method.Name != samp.Rpc {
+					if method.Name == samp.Rpc {
 						break
 					}
 				}
@@ -113,14 +189,14 @@ func genMethodSamples(gen *generator, sampConf schema_v1p2.SampleConfig, nofmt b
 		}
 
 		if method.Name != samp.Rpc {
-			return errors.E(nil, "generating sample %q: rpc %q not found", samp.ID, method.Name)
+			return errors.E(nil, "generating sample %q: rpc %q not found", samp.ID, samp.Rpc)
 		}
 		if iface.Name != samp.Service {
-			return errors.E(nil, "generating sample %q: service %q not found", samp.ID, iface.Name)
+			return errors.E(nil, "generating sample %q: service %q not found", samp.ID, samp.Service)
 		}
 
 		gen.reset()
-		if err := gen.genSample(samp, method); err != nil {
+		if err := gen.genSample(*samp, method); err != nil {
 			err = errors.E(err, "generating: %s.%s:%s", iface.Name, method.Name, samp.ID)
 			log.Fatal(err)
 		}
@@ -129,7 +205,7 @@ func genMethodSamples(gen *generator, sampConf schema_v1p2.SampleConfig, nofmt b
 		if err != nil {
 			return err
 		}
-		// TODO(hzyi): Handle duplicate sample IDs
+
 		fname := samp.ID + ".go"
 		if err := ioutil.WriteFile(filepath.Join(outDir, fname), content, 0644); err != nil {
 			return err
@@ -203,6 +279,40 @@ func (g *generator) commit(gofmt bool, year int) ([]byte, error) {
 	return b, nil
 }
 
+func (g *generator) disambiguateSampleIDs() error {
+	idCount := make(map[string]int)
+	hashes := make(map[string]bool)
+	samples := g.sampleConfig.Samples
+	for i := range samples {
+		// default ID to region tag
+		if samples[i].ID == "" {
+			samples[i].ID = samples[i].RegionTag
+		}
+		if samples[i].ID != "" {
+			idCount[samples[i].ID]++
+		}
+	}
+
+	for i := range samples {
+		// Generate a sample ID when the user-provided ID is empty not unique
+		if samples[i].ID == "" || idCount[samples[i].ID] > 1 {
+			jsonStr, err := json.Marshal(samples[i])
+			if err != nil {
+				return err
+			}
+			checkSum := sha256.Sum256([]byte(jsonStr))
+			encodedStr := base32.StdEncoding.EncodeToString(checkSum[:])
+			suffix := string([]rune(encodedStr)[0:8])
+			if _, found := hashes[suffix]; found {
+				return errors.E(nil, "unable to get a unique hash, multiple samples with identical contents?")
+			}
+			hashes[suffix] = true
+			samples[i].ID += suffix
+		}
+	}
+	return nil
+}
+
 func (g *generator) genSample(sampConf schema_v1p2.Sample, methConf GAPICMethod) error {
 	ifaceName := sampConf.Service
 
diff --git a/cmd/gen-go-sample/schema_v1p2/schema.go b/cmd/gen-go-sample/schema_v1p2/schema.go
index 42dedce..6c9e0de 100644
--- a/cmd/gen-go-sample/schema_v1p2/schema.go
+++ b/cmd/gen-go-sample/schema_v1p2/schema.go
@@ -16,18 +16,18 @@ package schema_v1p2
 
 type SampleConfig struct {
 	Type    string
-	Version string `yaml: config_schema_version`
-	Samples []Sample
+	Version string `yaml:"schema_version"`
+	Samples []*Sample
 }
 
 type Sample struct {
-	ID              string `yaml: id`
+	ID              string `yaml:"id"`
 	Title           string
-	RegionTag       string `yaml: region_tag`
+	RegionTag       string `yaml:"region_tag"`
 	Description     string
 	Service         string
 	Rpc             string
-	CallingPatterns []string `yaml: calling_patterns`
+	CallingPatterns []string `yaml:"calling_patterns"`
 	Request         []RequestConfig
 	Response        []ResponseConfig
 }
@@ -36,8 +36,8 @@ type RequestConfig struct {
 	Field          string
 	Comment        string
 	Value          string
-	ValueIsFile    bool   `yaml: value_is_file`
-	InputParameter string `yaml: input_parameter`
+	ValueIsFile    bool   `yaml:"value_is_file"`
+	InputParameter string `yaml:"input_parameter"`
 }
 
 type ResponseConfig struct {


Test patch:
diff --git a/cmd/gen-go-sample/main_sampleconfig_test.go b/cmd/gen-go-sample/main_sampleconfig_test.go
new file mode 100644
index 0000000..d08ef9f
--- /dev/null
+++ b/cmd/gen-go-sample/main_sampleconfig_test.go
@@ -0,0 +1,225 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	yaml "gopkg.in/yaml.v2"
+	"strings"
+	"testing"
+
+	"github.com/googleapis/gapic-generator-go/cmd/gen-go-sample/schema_v1p2"
+	"github.com/googleapis/gapic-generator-go/internal/errors"
+)
+
+func TestDisambiguateUniqueSampleIDs(t *testing.T) {
+	gen := &generator{
+		sampleConfig: schema_v1p2.SampleConfig{
+			Samples: []*schema_v1p2.Sample{
+				&schema_v1p2.Sample{ID: "sample0", Service: "foo.FooService", Rpc: "CreateFoo"},
+				&schema_v1p2.Sample{ID: "sample1", Service: "foo.FooService", Rpc: "UpdateFoo"},
+				&schema_v1p2.Sample{ID: "sample2", Service: "foo.FooService", Rpc: "DeleteFoo"},
+			},
+		},
+	}
+
+	gen.disambiguateSampleIDs()
+	if gen.sampleConfig.Samples[0].ID != "sample0" {
+		t.Fatal(errors.E(nil, `expected "sample0", got %q`, gen.sampleConfig.Samples[0].ID))
+	}
+	if gen.sampleConfig.Samples[1].ID != "sample1" {
+		t.Fatal(errors.E(nil, `expected "sample1", got %q`, gen.sampleConfig.Samples[1].ID))
+	}
+	if gen.sampleConfig.Samples[2].ID != "sample2" {
+		t.Fatal(errors.E(nil, `expected "sample2", got %q`, gen.sampleConfig.Samples[2].ID))
+	}
+}
+
+func TestDisambiguateRepeatedSampleIDs(t *testing.T) {
+	gen := &generator{
+		sampleConfig: schema_v1p2.SampleConfig{
+			Samples: []*schema_v1p2.Sample{
+				&schema_v1p2.Sample{ID: "sample0", Service: "foo.FooService", Rpc: "CreateFoo"},
+				&schema_v1p2.Sample{ID: "sample0", Service: "foo.FooService", Rpc: "UpdateFoo"},
+				&schema_v1p2.Sample{ID: "sample1", Service: "foo.FooService", Rpc: "DeleteFoo"},
+			},
+		},
+	}
+
+	gen.disambiguateSampleIDs()
+	if gen.sampleConfig.Samples[0].ID != "sample0WRAP2GEJ" {
+		t.Fatal(errors.E(nil, `expected "sample0WRAP2GEJ", got %q`, gen.sampleConfig.Samples[0].ID))
+	}
+	if gen.sampleConfig.Samples[1].ID != "sample05ITXN42C" {
+		t.Fatal(errors.E(nil, `expected "sample05ITXN42C", got %q`, gen.sampleConfig.Samples[1].ID))
+	}
+	if gen.sampleConfig.Samples[2].ID != "sample1" {
+		t.Fatal(errors.E(nil, `expected "sample1", got %q`, gen.sampleConfig.Samples[2].ID))
+	}
+}
+
+func TestDisambiguateRepeatedSampleIDsFromRegionTags(t *testing.T) {
+	gen := &generator{
+		sampleConfig: schema_v1p2.SampleConfig{
+			Samples: []*schema_v1p2.Sample{
+				&schema_v1p2.Sample{ID: "sample0", Service: "foo.FooService", Rpc: "CreateFoo"},
+				&schema_v1p2.Sample{RegionTag: "sample0", Service: "foo.FooService", Rpc: "UpdateFoo"},
+				&schema_v1p2.Sample{ID: "sample1", Service: "foo.FooService", Rpc: "DeleteFoo"},
+			},
+		},
+	}
+
+	gen.disambiguateSampleIDs()
+	if gen.sampleConfig.Samples[0].ID != "sample0WRAP2GEJ" {
+		t.Fatal(errors.E(nil, `expected "sample0WRAP2GEJ", got %q`, gen.sampleConfig.Samples[0].ID))
+	}
+	if gen.sampleConfig.Samples[1].ID != "sample0QAE76E4S" {
+		t.Fatal(errors.E(nil, `expected "sample0QAE76E4S", got %q`, gen.sampleConfig.Samples[1].ID))
+	}
+	if gen.sampleConfig.Samples[2].ID != "sample1" {
+		t.Fatal(errors.E(nil, `expected "sample2", got %q`, gen.sampleConfig.Samples[2].ID))
+	}
+}
+
+func TestReadSampleConfigSingleDoc(t *testing.T) {
+	yamlStr := `
+type: com.google.api.codegen.samplegen.v1p2.SampleConfigProto
+schema_version: 1.2.0
+samples:
+- service: FooService
+  rpc: CreateFoo
+  region_tag: awesome_region
+`
+	decoder := yaml.NewDecoder(strings.NewReader(yamlStr))
+	config, err := readSampleConfig(decoder, "some.yaml")
+	if err != nil {
+		t.Fatal(errors.E(nil, "unexpected error: %q", err.Error()))
+	}
+	if len(config.Samples) != 1 {
+		t.Fatal(errors.E(nil, "expected one valid sample, got %d", len(config.Samples)))
+	}
+}
+
+func TestReadSampleConfigSingleDoc_BadTypeError(t *testing.T) {
+	yamlStr := `
+type: com.google.api.codegen.samplegen.v1p2.NotSampleConfigProto
+schema_version: 1.2.0
+samples:
+- service: FooService
+  rpc: CreateFoo
+  region_tag: awesome_region
+`
+	decoder := yaml.NewDecoder(strings.NewReader(yamlStr))
+	_, err := readSampleConfig(decoder, "some.yaml")
+	expectedErrMsg := `Found no valid sample config in "some.yaml"`
+	if err == nil || err.Error() != expectedErrMsg {
+		t.Fatal(errors.E(nil, "expected error with message %q, got %q", expectedErrMsg, err.Error()))
+	}
+}
+
+func TestReadSampleConfigSingleDoc_BadSchemaVersionError(t *testing.T) {
+	yamlStr := `
+type: com.google.api.codegen.samplegen.v1p2.SampleConfigProto
+schema_version: 1.3.0
+samples:
+- service: FooService
+  rpc: CreateFoo
+  region_tag: awesome_region
+`
+	decoder := yaml.NewDecoder(strings.NewReader(yamlStr))
+	_, err := readSampleConfig(decoder, "some.yaml")
+	expectedErrMsg := `Found no valid sample config in "some.yaml"`
+	if err == nil || err.Error() != expectedErrMsg {
+		t.Fatal(errors.E(nil, "expected error with message %q, got %q", expectedErrMsg, err.Error()))
+	}
+}
+
+func TestReadSampleConfig_EmptyYamlError(t *testing.T) {
+	yamlStr := ""
+	decoder := yaml.NewDecoder(strings.NewReader(yamlStr))
+	_, err := readSampleConfig(decoder, "some.yaml")
+	expectedErrMsg := `Found no valid sample config in "some.yaml"`
+	if err == nil || err.Error() != expectedErrMsg {
+		t.Fatal(errors.E(nil, "expected error with message %q, got %q", expectedErrMsg, err.Error()))
+	}
+}
+
+func TestReadSampleConfigMultiDoc(t *testing.T) {
+	yamlStr := `
+type: com.google.api.codegen.samplegen.v1p2.SampleConfigProto
+schema_version: 1.2.0
+samples:
+- service: FooService
+  rpc: CreateFoo
+  region_tag: awesome_region
+---
+type: com.google.api.codegen.samplegen.v1p2.SampleConfigProto
+schema_version: 1.2.0
+samples:
+- service: FooService
+  rpc: GetFoo
+  region_tag: lame_region
+`
+	decoder := yaml.NewDecoder(strings.NewReader(yamlStr))
+	config, err := readSampleConfig(decoder, "some.yaml")
+	if err != nil {
+		t.Fatal(errors.E(nil, "unexpected error: %q", err.Error()))
+	}
+	if config.Samples[0].Rpc != "CreateFoo" {
+		t.Fatal(errors.E(nil, `expected "CreateFoo", got %q`, config.Samples[0].Rpc))
+	}
+	if config.Samples[1].Rpc != "GetFoo" {
+		t.Fatal(errors.E(nil, `expected "GetFoo", got %q`, config.Samples[1].Rpc))
+	}
+}
+
+func TestReadSampleConfigMultiDocSkipNonSampleConfig(t *testing.T) {
+	yamlStr := `
+type: com.google.api.codegen.samplegen.v1p2.SampleConfigProto
+schema_version: 1.2.0
+samples:
+- service: FooService
+  rpc: CreateFoo
+  region_tag: awesome_region
+---
+type: another_type
+`
+	decoder := yaml.NewDecoder(strings.NewReader(yamlStr))
+	config, err := readSampleConfig(decoder, "some.yaml")
+	if err != nil {
+		t.Fatal(errors.E(nil, "unexpected error: %q", err.Error()))
+	}
+	if len(config.Samples) != 1 {
+		t.Fatal(errors.E(nil, "expected one valid sample, got %d", len(config.Samples)))
+	}
+}
+
+func TestReadSampleConfigMultiDoc_BadFormatError(t *testing.T) {
+	yamlStr := `
+type: com.google.api.codegen.samplegen.v1p2.SampleConfigProto
+schema_version: 1.2.0
+samples:
+- service: FooService
+  rpc: CreateFoo
+  region_tag: awesome_region
+---
+type: another_type
+  bad_field: derp
+`
+	decoder := yaml.NewDecoder(strings.NewReader(yamlStr))
+	_, err := readSampleConfig(decoder, "some.yaml")
+	if err == nil {
+		t.Fatal(errors.E(nil, "expected error"))
+	}
+}