diff --git a/tool/cmd/migrate/php.go b/tool/cmd/migrate/php.go index a3e259b592c..6a0ce33d3d8 100644 --- a/tool/cmd/migrate/php.go +++ b/tool/cmd/migrate/php.go @@ -66,7 +66,7 @@ func runPHPMigration(ctx context.Context, repoPath string) error { } var ( - owlbotSourceWithVersionRegexp = regexp.MustCompile(`^/([a-zA-Z0-9_/]+)/\((v[0-9a-zA-Z]+)\)/.*-php/.*$`) + owlbotSourceWithVersionRegexp = regexp.MustCompile(`^/([a-zA-Z0-9_/]+)/\((v[0-9a-zA-Z|]+)\)/.*-php/.*$`) owlbotSourceWithoutVersionRegexp = regexp.MustCompile(`^/([a-zA-Z0-9_/]+)/.*-php/.*$`) ) @@ -80,14 +80,27 @@ type deepCopyRegexSpec struct { Dest string `yaml:"dest"` } -func extractAPIPath(source string) (string, bool) { +// extractAPIPaths extracts target API paths from an OwlBot source matcher pattern. +// It supports both unversioned paths and versioned paths, including union matchers +// (e.g. "(v1|v1beta2)") which are expanded into separate versioned paths. +// Returns nil if the pattern is invalid. +func extractAPIPaths(source string) []string { if matches := owlbotSourceWithVersionRegexp.FindStringSubmatch(source); len(matches) == 3 { - return matches[1] + "/" + matches[2], true + // matches[1] is the base path (e.g. "google/cloud/secretmanager") + // matches[2] is the version or version union (e.g. "v1" or "v1|v1beta2") + base := matches[1] + versions := strings.Split(matches[2], "|") + var paths []string + for _, v := range versions { + paths = append(paths, base+"/"+v) + } + return paths } if matches := owlbotSourceWithoutVersionRegexp.FindStringSubmatch(source); len(matches) == 2 { - return matches[1], true + // matches[1] is the full path without a version suffix (e.g. "google/identity/accesscontextmanager/type") + return []string{matches[1]} } - return "", false + return nil } func extractAPIsFromOwlBot(owlbotPath string) ([]*config.API, error) { @@ -101,7 +114,7 @@ func extractAPIsFromOwlBot(owlbotPath string) ([]*config.API, error) { var apis []*config.API seenAPIs := make(map[string]bool) for _, spec := range owlbot.DeepCopyRegex { - if path, ok := extractAPIPath(spec.Source); ok { + for _, path := range extractAPIPaths(spec.Source) { if !seenAPIs[path] { seenAPIs[path] = true apis = append(apis, &config.API{Path: path}) diff --git a/tool/cmd/migrate/php_test.go b/tool/cmd/migrate/php_test.go index c8037f9193c..68d567f0495 100644 --- a/tool/cmd/migrate/php_test.go +++ b/tool/cmd/migrate/php_test.go @@ -103,45 +103,42 @@ func TestRunPHPMigration(t *testing.T) { } } -func TestExtractAPIPath(t *testing.T) { +func TestExtractAPIPaths(t *testing.T) { for _, test := range []struct { - name string - source string - wantPath string - wantOk bool + name string + source string + wantPaths []string }{ { - name: "versioned api", - source: "/google/cloud/ces/(v1)/.*-php/(.*)", - wantPath: "google/cloud/ces/v1", - wantOk: true, + name: "versioned api", + source: "/google/cloud/ces/(v1)/.*-php/(.*)", + wantPaths: []string{"google/cloud/ces/v1"}, + }, + { + name: "unversioned api", + source: "/google/identity/accesscontextmanager/type/.*-php/(.*)", + wantPaths: []string{"google/identity/accesscontextmanager/type"}, }, { - name: "unversioned api", - source: "/google/identity/accesscontextmanager/type/.*-php/(.*)", - wantPath: "google/identity/accesscontextmanager/type", - wantOk: true, + name: "non-matching path", + source: "/some/other/path", + wantPaths: nil, }, { - name: "non-matching path", - source: "/some/other/path", - wantPath: "", - wantOk: false, + name: "grafeas versioned", + source: "/grafeas/(v1)/.*-php/(.*)", + wantPaths: []string{"grafeas/v1"}, }, { - name: "grafeas versioned", - source: "/grafeas/(v1)/.*-php/(.*)", - wantPath: "grafeas/v1", - wantOk: true, + name: "union versioned api", + source: "/google/cloud/secretmanager/(v1|v1beta2)/.*-php/(.*)", + wantPaths: []string{"google/cloud/secretmanager/v1", "google/cloud/secretmanager/v1beta2"}, }, } { t.Run(test.name, func(t *testing.T) { - gotPath, gotOk := extractAPIPath(test.source) - if gotOk != test.wantOk { - t.Fatal("extractAPIPath ok =", gotOk, ", want", test.wantOk) - } - if gotPath != test.wantPath { - t.Error("extractAPIPath path =", gotPath, ", want", test.wantPath) + gotPaths := extractAPIPaths(test.source) + if diff := cmp.Diff(test.wantPaths, gotPaths); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) } }) }