From 715b0173e1cb370298b4798f0b01a1e55096e6a9 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Fri, 10 Jul 2026 10:06:08 -0400 Subject: [PATCH 1/2] fix(tool/cmd/migrate): populate API paths from .OwlBot.yaml during migrate for php --- tool/cmd/migrate/php.go | 56 ++++++++++++++++ tool/cmd/migrate/php_test.go | 126 +++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+) diff --git a/tool/cmd/migrate/php.go b/tool/cmd/migrate/php.go index 4143439d939..c8250f8bb5a 100644 --- a/tool/cmd/migrate/php.go +++ b/tool/cmd/migrate/php.go @@ -20,10 +20,12 @@ import ( "log" "os" "path/filepath" + "regexp" "strings" "github.com/googleapis/librarian/internal/config" "github.com/googleapis/librarian/internal/librarian" + "github.com/googleapis/librarian/internal/yaml" ) func runPHPMigration(ctx context.Context, repoPath string) error { @@ -52,10 +54,57 @@ func runPHPMigration(ctx context.Context, repoPath string) error { return nil } +var ( + owlbotSourceWithVersionRegexp = regexp.MustCompile(`^/([a-zA-Z0-9_/]+)/\((v[0-9a-zA-Z]+)\)/.*-php/.*$`) + owlbotSourceWithoutVersionRegexp = regexp.MustCompile(`^/([a-zA-Z0-9_/]+)/.*-php/.*$`) +) + +type OwlBotConfig struct { + DeepCopyRegex []DeepCopyRegexSpec `yaml:"deep-copy-regex"` + APIName string `yaml:"api-name"` +} + +type DeepCopyRegexSpec struct { + Source string `yaml:"source"` + Dest string `yaml:"dest"` +} + +func extractAPIPath(source string) (string, bool) { + if matches := owlbotSourceWithVersionRegexp.FindStringSubmatch(source); len(matches) == 3 { + return matches[1] + "/" + matches[2], true + } + if matches := owlbotSourceWithoutVersionRegexp.FindStringSubmatch(source); len(matches) == 2 { + return matches[1], true + } + return "", false +} + +func extractAPIsFromOwlBot(owlbotPath string) ([]*config.API, error) { + if !fileExists(owlbotPath) { + return nil, nil + } + owlbot, err := yaml.Read[OwlBotConfig](owlbotPath) + if err != nil { + return nil, err + } + var apis []*config.API + seenAPIs := make(map[string]bool) + for _, spec := range owlbot.DeepCopyRegex { + if path, ok := extractAPIPath(spec.Source); ok { + if !seenAPIs[path] { + seenAPIs[path] = true + apis = append(apis, &config.API{Path: path}) + } + } + } + return apis, nil +} + // findPHPLibraries scans the repository root directory for subdirectories containing // both a VERSION file and a composer.json file. It assumes each matching subdirectory // represents a PHP library, where the library name is the subdirectory's name and // the version is extracted from the VERSION file. +// It also attempts to parse .OwlBot.yaml to extract API paths. func findPHPLibraries(repoPath string) ([]*config.Library, error) { entries, err := os.ReadDir(repoPath) if err != nil { @@ -78,9 +127,16 @@ func findPHPLibraries(repoPath string) ([]*config.Library, error) { return nil, fmt.Errorf("reading version for %s: %w", name, err) } version := strings.TrimSpace(string(versionBytes)) + + apis, err := extractAPIsFromOwlBot(filepath.Join(repoPath, name, ".OwlBot.yaml")) + if err != nil { + return nil, fmt.Errorf("extracting APIs from OwlBot config for %s: %w", name, err) + } + libs = append(libs, &config.Library{ Name: name, Version: version, + APIs: apis, }) } return libs, nil diff --git a/tool/cmd/migrate/php_test.go b/tool/cmd/migrate/php_test.go index c28428ccaf3..09e868d9da9 100644 --- a/tool/cmd/migrate/php_test.go +++ b/tool/cmd/migrate/php_test.go @@ -91,3 +91,129 @@ func TestRunPHPMigration(t *testing.T) { t.Errorf("mismatch (-want +got):\n%s", diff) } } + +func TestExtractAPIPath(t *testing.T) { + tests := []struct { + name string + source string + wantPath string + wantOk bool + }{ + { + name: "versioned api", + source: "/google/cloud/ces/(v1)/.*-php/(.*)", + wantPath: "google/cloud/ces/v1", + wantOk: true, + }, + { + name: "unversioned api", + source: "/google/identity/accesscontextmanager/type/.*-php/(.*)", + wantPath: "google/identity/accesscontextmanager/type", + wantOk: true, + }, + { + name: "non-matching path", + source: "/some/other/path", + wantPath: "", + wantOk: false, + }, + { + name: "grafeas versioned", + source: "/grafeas/(v1)/.*-php/(.*)", + wantPath: "grafeas/v1", + wantOk: true, + }, + } + + for _, test := range tests { + 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) + } + }) + } +} + +func TestExtractAPIsFromOwlBot(t *testing.T) { + for _, test := range []struct { + name string + setupFile func(dir string) string + want []*config.API + }{ + { + name: "missing owlbot.yaml", + setupFile: func(dir string) string { + return filepath.Join(dir, "missing.yaml") + }, + want: nil, + }, + { + name: "valid file", + setupFile: func(dir string) string { + content := ` +deep-copy-regex: + - source: /google/cloud/ces/(v1)/.*-php/(.*) + dest: /owl-bot-staging/Ces/$1/$2 + - source: /google/identity/accesscontextmanager/type/.*-php/(.*) + dest: /owl-bot-staging/AccessContextManager/type-protos/$1 + - source: /google/cloud/ces/(v1)/.*-php/(.*) + dest: /owl-bot-staging/Ces/$1/$2 +api-name: Ces +` + path := filepath.Join(dir, ".OwlBot.yaml") + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatal(err) + } + return path + }, + want: []*config.API{ + {Path: "google/cloud/ces/v1"}, + {Path: "google/identity/accesscontextmanager/type"}, + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + dir := t.TempDir() + path := test.setupFile(dir) + got, err := extractAPIsFromOwlBot(path) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(test.want, got); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestExtractAPIsFromOwlBot_Error(t *testing.T) { + for _, test := range []struct { + name string + setupFile func(dir string) string + }{ + { + name: "invalid file", + setupFile: func(dir string) string { + content := `{invalid` + path := filepath.Join(dir, ".OwlBot.yaml") + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatal(err) + } + return path + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + dir := t.TempDir() + path := test.setupFile(dir) + _, err := extractAPIsFromOwlBot(path) + if err == nil { + t.Fatal("expected error, got nil") + } + }) + } +} From 65794ee26205a5878c6b30bf45ce93134405179d Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Fri, 10 Jul 2026 10:12:04 -0400 Subject: [PATCH 2/2] fix lint, test style --- tool/cmd/migrate/php.go | 8 ++++---- tool/cmd/migrate/php_test.go | 8 +++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/tool/cmd/migrate/php.go b/tool/cmd/migrate/php.go index c8250f8bb5a..43cb66eee5c 100644 --- a/tool/cmd/migrate/php.go +++ b/tool/cmd/migrate/php.go @@ -59,12 +59,12 @@ var ( owlbotSourceWithoutVersionRegexp = regexp.MustCompile(`^/([a-zA-Z0-9_/]+)/.*-php/.*$`) ) -type OwlBotConfig struct { - DeepCopyRegex []DeepCopyRegexSpec `yaml:"deep-copy-regex"` +type owlBotConfig struct { + DeepCopyRegex []deepCopyRegexSpec `yaml:"deep-copy-regex"` APIName string `yaml:"api-name"` } -type DeepCopyRegexSpec struct { +type deepCopyRegexSpec struct { Source string `yaml:"source"` Dest string `yaml:"dest"` } @@ -83,7 +83,7 @@ func extractAPIsFromOwlBot(owlbotPath string) ([]*config.API, error) { if !fileExists(owlbotPath) { return nil, nil } - owlbot, err := yaml.Read[OwlBotConfig](owlbotPath) + owlbot, err := yaml.Read[owlBotConfig](owlbotPath) if err != nil { return nil, err } diff --git a/tool/cmd/migrate/php_test.go b/tool/cmd/migrate/php_test.go index 09e868d9da9..b683e0e2cc2 100644 --- a/tool/cmd/migrate/php_test.go +++ b/tool/cmd/migrate/php_test.go @@ -93,7 +93,7 @@ func TestRunPHPMigration(t *testing.T) { } func TestExtractAPIPath(t *testing.T) { - tests := []struct { + for _, test := range []struct { name string source string wantPath string @@ -123,9 +123,7 @@ func TestExtractAPIPath(t *testing.T) { wantPath: "grafeas/v1", wantOk: true, }, - } - - for _, test := range tests { + } { t.Run(test.name, func(t *testing.T) { gotPath, gotOk := extractAPIPath(test.source) if gotOk != test.wantOk { @@ -149,7 +147,7 @@ func TestExtractAPIsFromOwlBot(t *testing.T) { setupFile: func(dir string) string { return filepath.Join(dir, "missing.yaml") }, - want: nil, + want: nil, }, { name: "valid file",