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(internal/gapicgen): use correct region tags for gensnippets #4022

Merged
merged 3 commits into from Apr 29, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion internal/gapicgen/generator/gapics.go
Expand Up @@ -86,7 +86,7 @@ func (g *GapicGenerator) Regen(ctx context.Context) error {
return err
}
if err := gensnippets.Generate(g.googleCloudDir, snippetDir, apiShortnames); err != nil {
return fmt.Errorf("error generating snippets: %v", err)
log.Printf("warning: got the following non-fatal errors generating snippets: %v", err)
}
if err := replaceAllForSnippets(g.googleCloudDir, snippetDir); err != nil {
return err
Expand Down
126 changes: 77 additions & 49 deletions internal/gapicgen/gensnippets/gensnippets.go
Expand Up @@ -31,6 +31,8 @@ import (
"cloud.google.com/go/internal/godocfx/pkgload"
"cloud.google.com/go/third_party/go/doc"
"golang.org/x/sys/execabs"
"google.golang.org/genproto/googleapis/gapic/metadata"
"google.golang.org/protobuf/encoding/protojson"
)

// Generate reads all modules in rootDir and outputs their examples in outDir.
Expand Down Expand Up @@ -68,13 +70,13 @@ func Generate(rootDir, outDir string, apiShortnames map[string]string) error {
return fmt.Errorf("failed to load packages: %v", err)
}
for _, pi := range pis {
if err := processExamples(pi.Doc, pi.Fset, trimPrefix, outDir, apiShortnames); err != nil {
errs = append(errs, fmt.Errorf("failed to process examples: %v", err))
if eErrs := processExamples(pi.Doc, pi.Fset, trimPrefix, rootDir, outDir, apiShortnames); len(eErrs) > 0 {
errs = append(errs, fmt.Errorf("%v", eErrs))
}
}
}
if len(errs) > 0 {
log.Fatal(errs)
return fmt.Errorf("example errors: %v", errs)
}

if len(dirs) > 0 {
Expand Down Expand Up @@ -103,69 +105,90 @@ var skip = map[string]bool{
"cloud.google.com/go/translate": true, // Has newer version.
}

func processExamples(pkg *doc.Package, fset *token.FileSet, trimPrefix, outDir string, apiShortnames map[string]string) error {
func processExamples(pkg *doc.Package, fset *token.FileSet, trimPrefix, rootDir, outDir string, apiShortnames map[string]string) []error {
if skip[pkg.ImportPath] {
return nil
}
trimmed := strings.TrimPrefix(pkg.ImportPath, trimPrefix)
regionTags, err := computeRegionTags(rootDir, trimmed, apiShortnames)
if err != nil {
return []error{err}
}
if len(regionTags) == 0 {
// Nothing to do.
return nil
}
outDir = filepath.Join(outDir, trimmed)

shortname, ok := apiShortnames[pkg.ImportPath]
if !ok {
// Do our best to find a shortname. For example,
// cloud.google.com/go/bigtable/bttest should lead to
// cloud.google.com/go/bigtable.
bestMatch := ""
for path := range apiShortnames {
if strings.HasPrefix(pkg.ImportPath, path) {
if len(path) > len(bestMatch) {
bestMatch = path
}
// Note: only process methods because they correspond to RPCs.

var errs []error
for _, t := range pkg.Types {
for _, m := range t.Methods {
if len(m.Examples) == 0 {
// Nothing to do for this method.
continue
}
dir := filepath.Join(outDir, t.Name, m.Name)
regionTag, ok := regionTags[t.Name][m.Name]
if !ok {
errs = append(errs, fmt.Errorf("could not find region tag for %s %s.%s", pkg.ImportPath, t.Name, m.Name))
continue
}
if err := writeExamples(dir, m.Examples, fset, regionTag); err != nil {
errs = append(errs, err)
}
}
if bestMatch == "" {
return fmt.Errorf("could not find API shortname for %v", pkg.ImportPath)
}
log.Printf("The best match for %q is %q", pkg.ImportPath, bestMatch)
shortname = apiShortnames[bestMatch]
}
regionTag := shortname + "_generated" + strings.ReplaceAll(trimmed, "/", "_")

// Note: variables and constants don't have examples.
return errs
}

for _, f := range pkg.Funcs {
dir := filepath.Join(outDir, f.Name)
if err := writeExamples(dir, f.Examples, fset, regionTag); err != nil {
return err
}
// computeRegionTags gets the region tags for the given path, keyed by client name and method name.
func computeRegionTags(rootDir, path string, apiShortnames map[string]string) (regionTags map[string]map[string]string, err error) {
metadataPath := filepath.Join(rootDir, path, "gapic_metadata.json")
f, err := os.ReadFile(metadataPath)
if err != nil {
// If there is no gapic_metadata.json file, don't generate snippets.
// This isn't an error, though, because some packages aren't GAPICs and
// shouldn't get snippets in the first place.
return nil, nil
}
m := metadata.GapicMetadata{}
if err := protojson.Unmarshal(f, &m); err != nil {
return nil, err
}
shortname, ok := apiShortnames[m.GetLibraryPackage()]
if !ok {
return nil, fmt.Errorf("could not find shortname for %q", m.GetLibraryPackage())
}
protoParts := strings.Split(m.GetProtoPackage(), ".")
apiVersion := protoParts[len(protoParts)-1]

for _, t := range pkg.Types {
dir := filepath.Join(outDir, t.Name)
if err := writeExamples(dir, t.Examples, fset, regionTag); err != nil {
return err
}
for _, f := range t.Funcs {
fDir := filepath.Join(dir, f.Name)
if err := writeExamples(fDir, f.Examples, fset, regionTag); err != nil {
return err
}
}
for _, m := range t.Methods {
mDir := filepath.Join(dir, m.Name)
if err := writeExamples(mDir, m.Examples, fset, regionTag); err != nil {
return err
regionTags = map[string]map[string]string{}
for sName, s := range m.GetServices() {
for _, c := range s.GetClients() {
for rpc, methods := range c.GetRpcs() {
if len(methods.GetMethods()) != 1 {
return nil, fmt.Errorf("%s %s %s found %d methods", m.GetLibraryPackage(), sName, c.GetLibraryClient(), len(methods.GetMethods()))
}
if methods.GetMethods()[0] != rpc {
return nil, fmt.Errorf("%s %s %s %q does not match %q", m.GetLibraryPackage(), sName, c.GetLibraryClient(), methods.GetMethods()[0], rpc)
}

// Every Go method is synchronous.
regionTag := fmt.Sprintf("%s_%s_generated_%s_%s_sync", shortname, apiVersion, sName, rpc)

if regionTags[c.GetLibraryClient()] == nil {
regionTags[c.GetLibraryClient()] = map[string]string{}
}
regionTags[c.GetLibraryClient()][rpc] = regionTag
}
}
}
return nil
return regionTags, nil
}

func writeExamples(outDir string, exs []*doc.Example, fset *token.FileSet, regionTag string) error {
if len(exs) == 0 {
// Nothing to do.
return nil
}
for _, ex := range exs {
dir := outDir
if len(exs) > 1 {
Expand Down Expand Up @@ -207,7 +230,12 @@ func writeExamples(outDir string, exs []*doc.Example, fset *token.FileSet, regio
if _, err := f.WriteString(header()); err != nil {
return err
}
tag := regionTag + "_" + ex.Name

tag := regionTag
if len(ex.Suffix) > 0 {
tag += "_" + ex.Suffix
}

// Include an extra newline to keep separate from the package declaration.
if _, err := fmt.Fprintf(f, "// [START %v]\n\n", tag); err != nil {
return err
Expand Down
2 changes: 2 additions & 0 deletions internal/gapicgen/go.mod
Expand Up @@ -14,6 +14,8 @@ require (
golang.org/x/oauth2 v0.0.0-20210413134643-5e61552d6c78
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
golang.org/x/sys v0.0.0-20210415045647-66c3f260301c
google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1
google.golang.org/protobuf v1.26.0
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
gopkg.in/src-d/go-git.v4 v4.13.1
gopkg.in/yaml.v2 v2.4.0
Expand Down
1 change: 1 addition & 0 deletions internal/gapicgen/go.sum
Expand Up @@ -477,6 +477,7 @@ google.golang.org/genproto v0.0.0-20210222152913-aa3ee6e6a81c/go.mod h1:FWY/as6D
google.golang.org/genproto v0.0.0-20210303154014-9728d6b83eeb/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
google.golang.org/genproto v0.0.0-20210310155132-4ce2db91004e/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
google.golang.org/genproto v0.0.0-20210319143718-93e7006c17a6/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1 h1:E7wSQBXkH3T3diucK+9Z1kjn4+/9tNG7lZLr75oOhh8=
google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1/go.mod h1:9lPAdzaEmUacj36I+k7YKbEc5CXzPIeORRgDAUOu28A=
google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38=
Expand Down
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_ApproveApprovalRequest]
// [START accessapproval_v1_generated_AccessApproval_ApproveApprovalRequest_sync]

package main

Expand Down Expand Up @@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END accessapproval_generated_accessapproval_apiv1_Client_ApproveApprovalRequest]
// [END accessapproval_v1_generated_AccessApproval_ApproveApprovalRequest_sync]
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_DeleteAccessApprovalSettings]
// [START accessapproval_v1_generated_AccessApproval_DeleteAccessApprovalSettings_sync]

package main

Expand All @@ -39,4 +39,4 @@ func main() {
}
}

// [END accessapproval_generated_accessapproval_apiv1_Client_DeleteAccessApprovalSettings]
// [END accessapproval_v1_generated_AccessApproval_DeleteAccessApprovalSettings_sync]
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_DismissApprovalRequest]
// [START accessapproval_v1_generated_AccessApproval_DismissApprovalRequest_sync]

package main

Expand Down Expand Up @@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END accessapproval_generated_accessapproval_apiv1_Client_DismissApprovalRequest]
// [END accessapproval_v1_generated_AccessApproval_DismissApprovalRequest_sync]
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_GetAccessApprovalSettings]
// [START accessapproval_v1_generated_AccessApproval_GetAccessApprovalSettings_sync]

package main

Expand Down Expand Up @@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END accessapproval_generated_accessapproval_apiv1_Client_GetAccessApprovalSettings]
// [END accessapproval_v1_generated_AccessApproval_GetAccessApprovalSettings_sync]
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_GetApprovalRequest]
// [START accessapproval_v1_generated_AccessApproval_GetApprovalRequest_sync]

package main

Expand Down Expand Up @@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END accessapproval_generated_accessapproval_apiv1_Client_GetApprovalRequest]
// [END accessapproval_v1_generated_AccessApproval_GetApprovalRequest_sync]
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_ListApprovalRequests]
// [START accessapproval_v1_generated_AccessApproval_ListApprovalRequests_sync]

package main

Expand Down Expand Up @@ -51,4 +51,4 @@ func main() {
}
}

// [END accessapproval_generated_accessapproval_apiv1_Client_ListApprovalRequests]
// [END accessapproval_v1_generated_AccessApproval_ListApprovalRequests_sync]

This file was deleted.

Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_UpdateAccessApprovalSettings]
// [START accessapproval_v1_generated_AccessApproval_UpdateAccessApprovalSettings_sync]

package main

Expand Down Expand Up @@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END accessapproval_generated_accessapproval_apiv1_Client_UpdateAccessApprovalSettings]
// [END accessapproval_v1_generated_AccessApproval_UpdateAccessApprovalSettings_sync]
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_AuditUserLinks]
// [START analyticsadmin_v1alpha_generated_AnalyticsAdminService_AuditUserLinks_sync]

package main

Expand Down Expand Up @@ -51,4 +51,4 @@ func main() {
}
}

// [END analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_AuditUserLinks]
// [END analyticsadmin_v1alpha_generated_AnalyticsAdminService_AuditUserLinks_sync]
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchCreateUserLinks]
// [START analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchCreateUserLinks_sync]

package main

Expand Down Expand Up @@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchCreateUserLinks]
// [END analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchCreateUserLinks_sync]
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchDeleteUserLinks]
// [START analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchDeleteUserLinks_sync]

package main

Expand All @@ -39,4 +39,4 @@ func main() {
}
}

// [END analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchDeleteUserLinks]
// [END analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchDeleteUserLinks_sync]