From 7dae5416522b5bb9a2475040904fc141d29e5bf6 Mon Sep 17 00:00:00 2001 From: Charlotte Y <38296042+cy-yun@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:39:40 -0700 Subject: [PATCH 1/4] fix(dev): ensure Ads and Common Protos new component parameters are derived correctly - Support Google\Ads namespace by using vendor googleads/, hyphenating project name, and setting github repo to googleapis/php-ads-* - Support CommonProtos by detecting missing protobuf service definitions, stripping type/common subpackages, appending CommonProtos to component name, and setting repo metadata library_type to CORE - Allow empty api_shortname when no default_host is defined Fixes #9387 --- dev/src/Command/ComponentNewCommand.php | 2 +- dev/src/NewComponent.php | 134 +++++++++++++++--- .../Unit/Command/ComponentNewCommandTest.php | 51 ++++++- dev/tests/Unit/NewComponentTest.php | 112 ++++++++++++++- 4 files changed, 274 insertions(+), 25 deletions(-) diff --git a/dev/src/Command/ComponentNewCommand.php b/dev/src/Command/ComponentNewCommand.php index 02245d74a568..a5765ae470bf 100644 --- a/dev/src/Command/ComponentNewCommand.php +++ b/dev/src/Command/ComponentNewCommand.php @@ -297,7 +297,7 @@ protected function execute(InputInterface $input, OutputInterface $output) 'distribution_name' => $new->composerPackage, 'release_level' => 'preview', 'client_documentation' => $documentationUrl, - 'library_type' => 'GAPIC_AUTO', + 'library_type' => str_ends_with($new->componentName, 'CommonProtos') ? 'CORE' : 'GAPIC_AUTO', 'api_shortname' => $new->shortName ]; $repoMetadataFullPath = $this->rootPath . '/.repo-metadata-full.json'; diff --git a/dev/src/NewComponent.php b/dev/src/NewComponent.php index bf63397de283..2779f8cbf7b0 100644 --- a/dev/src/NewComponent.php +++ b/dev/src/NewComponent.php @@ -37,22 +37,38 @@ class NewComponent public string $protoPath; public ?string $version; + /** + * @param array $options + */ public static function fromProto(string $protoContents, string $protoFilename, array $options = []): self { $new = new self(); + $hasGapicClient = self::hasGapicClient($protoContents); $new->protoPackage = $options['proto-package'] - ?? self::extractPackageNameFromProtoContents($protoContents); + ?? self::extractPackageNameFromProtoContents($protoContents, $hasGapicClient); $new->phpNamespace = $options['php-namespace'] - ?? (self::extractPhpNamespaceFromProtoContents($protoContents) + ?? (self::extractPhpNamespaceFromProtoContents($protoContents, $hasGapicClient) ?: self::derivePhpNamespaceFromProtoPackage($new->protoPackage)); $new->displayName = self::getDisplayName($new->phpNamespace); $new->componentName = $options['component-name'] ?? self::getComponentName($new->displayName); - $new->composerPackage = self::getComposerPackageFromProtoPackage($new->protoPackage); + if (!$hasGapicClient) { + if (!str_ends_with($new->componentName, 'CommonProtos')) { + $new->componentName .= 'CommonProtos'; + } + if (!str_ends_with($new->displayName, 'Common Protos')) { + $new->displayName .= ' Common Protos'; + } + } + $new->composerPackage = self::getComposerPackage( + $new->protoPackage, + $new->phpNamespace, + $hasGapicClient + ); $new->githubRepo = self::getGithubRepo($new->composerPackage); $new->gpbMetadataNamespace = self::getGpbMetadataNamespace($new->protoPackage); $new->shortName = $options['api-short-name'] - ?? self::extractShortNameFromProtoContents($protoContents); + ?? self::extractShortNameFromProtoContents($protoContents, $hasGapicClient); $new->version = array_key_exists('api-version', $options) ? $options['api-version'] : self::extractVersionFromProtoFilename($protoFilename); @@ -61,6 +77,9 @@ public static function fromProto(string $protoContents, string $protoFilename, a return $new; } + /** + * @param array $options + */ public static function fromOptions(array $options): self { $new = new self(); @@ -68,7 +87,15 @@ public static function fromOptions(array $options): self $new->phpNamespace = $options['php-namespace'] ?? ''; $new->displayName = self::getDisplayName($new->phpNamespace); $new->componentName = $options['component-name'] ?? ''; - $new->composerPackage = self::getComposerPackageFromProtoPackage($new->protoPackage); + $hasGapicClient = !str_ends_with($new->componentName, 'CommonProtos'); + if (!$hasGapicClient && !str_ends_with($new->displayName, 'Common Protos')) { + $new->displayName .= ' Common Protos'; + } + $new->composerPackage = self::getComposerPackage( + $new->protoPackage, + $new->phpNamespace, + $hasGapicClient + ); $new->githubRepo = self::getGithubRepo($new->composerPackage); $new->gpbMetadataNamespace = self::getGpbMetadataNamespace($new->protoPackage); $new->shortName = $options['api-short-name'] ?? ''; @@ -88,6 +115,9 @@ public function getDocumentationUrl(): string private static function getGithubRepo(string $composerPackage): string { + if (str_starts_with($composerPackage, 'googleads/')) { + return 'googleapis/php-ads-' . substr($composerPackage, 10); + } return 0 === strpos($composerPackage, 'google/cloud-') ? 'googleapis/google-cloud-php-' . substr($composerPackage, 13) : 'googleapis/php-' . substr($composerPackage, 7); @@ -117,13 +147,47 @@ private static function derivePhpNamespaceFromProtoPackage(string $protoPackage) return implode('\\', array_map('ucfirst', explode('.', $protoPackage))); } - private static function getComposerPackageFromProtoPackage(string $protoPackage): string - { - return 'google/' . str_replace( + private static function getComposerPackage( + string $protoPackage, + string $phpNamespace, + bool $hasGapicClient = true + ): string { + $parts = explode('.', $protoPackage); + if (str_starts_with($phpNamespace, 'Google\\Ads')) { + if (count($parts) > 1 && 'v' === strtolower($parts[count($parts) - 1][0] ?? '')) { + array_pop($parts); + } + $name = str_replace( + ['google.', 'devtools.cloud', '.'], + ['', 'cloud-', '-'], + implode('.', $parts) + ); + if (str_starts_with($name, 'ads-')) { + $name = substr($name, 4); + } + if (str_ends_with($name, 'manager') && !str_ends_with($name, '-manager')) { + $name = substr($name, 0, -7) . '-manager'; + } + if (!$hasGapicClient && !str_ends_with($name, '-common-protos')) { + $name .= '-common-protos'; + } + return 'googleads/' . $name; + } + + if (!$hasGapicClient && count($parts) > 2 && in_array(end($parts), ['type', 'common'])) { + array_pop($parts); + } + + $name = str_replace( ['google.', 'devtools.cloud', '.'], ['', 'cloud-', '-'], - $protoPackage + implode('.', $parts) ); + + if (!$hasGapicClient && !str_ends_with($name, '-common-protos')) { + $name .= '-common-protos'; + } + return 'google/' . $name; } private static function getProtoPath(string $protoFilename, ?string $version): string @@ -140,41 +204,67 @@ private static function getProtoPath(string $protoFilename, ?string $version): s return implode('/', $parts); } - private static function extractPackageNameFromProtoContents(string $protoContents): string + private static function hasGapicClient(string $protoContents): bool { + return (bool) preg_match('/^\s*service\s+[A-Za-z0-9_]+/m', $protoContents); + } + + private static function extractPackageNameFromProtoContents( + string $protoContents, + bool $hasGapicClient = true + ): string { if (!preg_match('/package (.*);/', $protoContents, $matches)) { throw new RuntimeException('package name not found in proto file '); } $parts = explode('.', $matches[1]); - $version = array_pop($parts); - if ('v' !== $version[0]) { - $parts[] = $version; + while (count($parts) > 1) { + $last = end($parts); + if ('v' === $last[0]) { + array_pop($parts); + } elseif (!$hasGapicClient && count($parts) > 2 && in_array($last, ['type', 'common'])) { + array_pop($parts); + } else { + break; + } } return implode('.', $parts); } - private static function extractShortNameFromProtoContents(string $protoContents): string - { + private static function extractShortNameFromProtoContents( + string $protoContents, + bool $hasGapicClient = true + ): string { if (!preg_match( '/option \(google.api.default_host\) =[\n\r\s]+"(.*).googleapis.com";/', $protoContents, - $matches) - ) { + $matches + )) { + if (!$hasGapicClient) { + return ''; + } throw new RuntimeException('short name not found in proto file'); } return $matches[1]; } - private static function extractPhpNamespaceFromProtoContents(string $protoContents): ?string - { + private static function extractPhpNamespaceFromProtoContents( + string $protoContents, + bool $hasGapicClient = true + ): ?string { if (!preg_match('/option php_namespace = "(.*)";/', $protoContents, $matches)) { return null; } // Remove version from namespace $parts = explode('\\\\', $matches[1]); - $version = array_pop($parts); - if ('v' !== strtolower($version[0])) { - $parts[] = $version; + while (count($parts) > 1) { + $last = end($parts); + if ('v' === strtolower($last[0])) { + array_pop($parts); + } elseif (!$hasGapicClient && count($parts) > 2 && in_array(strtolower($last), ['type', 'common'])) { + array_pop($parts); + } else { + break; + } } return implode('\\', $parts); diff --git a/dev/tests/Unit/Command/ComponentNewCommandTest.php b/dev/tests/Unit/Command/ComponentNewCommandTest.php index d774c2238af4..4d46708e138d 100644 --- a/dev/tests/Unit/Command/ComponentNewCommandTest.php +++ b/dev/tests/Unit/Command/ComponentNewCommandTest.php @@ -27,7 +27,6 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; - /** * @group dev */ @@ -417,6 +416,56 @@ public function testNewComponentWithPartialOptionsAndProtoPath() $this->assertFileExists(self::$tmpDir . '/CustomSecretManagerName/README.md'); } + public function testNewComponentCommonProtosWithAllOptions() + { + $application = new Application(); + $application->add(new ComponentNewCommand(self::$tmpDir)); + + $commandTester = new CommandTester($application->get('component:new')); + + $commandTester->execute([ + '--no-update' => true, + '--component-name' => 'GeoCommonProtos', + '--php-namespace' => 'Google\Geo', + '--proto-package' => 'google.geo', + '--api-short-name' => '', + '--api-version' => '', + '--product-docs' => 'https://cloud.google.com/geo/docs', + '--product-homepage' => 'https://cloud.google.com/geo', + ]); + + $repoMetadataFull = json_decode(file_get_contents(self::$tmpDir . '/.repo-metadata-full.json'), true); + $this->assertArrayHasKey('GeoCommonProtos', $repoMetadataFull); + $this->assertEquals('CORE', $repoMetadataFull['GeoCommonProtos']['library_type']); + $this->assertEquals('google/geo-common-protos', $repoMetadataFull['GeoCommonProtos']['distribution_name']); + $this->assertEquals('', $repoMetadataFull['GeoCommonProtos']['api_shortname']); + } + + public function testNewComponentAdsWithAllOptions() + { + $application = new Application(); + $application->add(new ComponentNewCommand(self::$tmpDir)); + + $commandTester = new CommandTester($application->get('component:new')); + + $commandTester->execute([ + '--no-update' => true, + '--component-name' => 'AdsAdManager', + '--php-namespace' => 'Google\Ads\AdManager\V1', + '--proto-package' => 'google.ads.admanager.v1', + '--api-short-name' => 'admanager', + '--api-version' => 'v1', + '--product-docs' => 'https://developers.google.com/ad-manager/api', + '--product-homepage' => 'https://developers.google.com/ad-manager', + ]); + + $repoMetadataFull = json_decode(file_get_contents(self::$tmpDir . '/.repo-metadata-full.json'), true); + $this->assertArrayHasKey('AdsAdManager', $repoMetadataFull); + $this->assertEquals('GAPIC_AUTO', $repoMetadataFull['AdsAdManager']['library_type']); + $this->assertEquals('googleads/ad-manager', $repoMetadataFull['AdsAdManager']['distribution_name']); + $this->assertEquals('admanager', $repoMetadataFull['AdsAdManager']['api_shortname']); + } + private function assertComposerJson(string $componentName) { $composerPath = sprintf('%s/../../fixtures/component/%s/composer.json', __DIR__, $componentName); diff --git a/dev/tests/Unit/NewComponentTest.php b/dev/tests/Unit/NewComponentTest.php index b440c3fdab7e..50c9711a7028 100644 --- a/dev/tests/Unit/NewComponentTest.php +++ b/dev/tests/Unit/NewComponentTest.php @@ -27,6 +27,7 @@ class NewComponentTest extends TestCase private string $protoMinimum = <<assertEquals('CustomSpeechName', $new->componentName); @@ -167,4 +169,112 @@ public function testFromProtoWithOptionOverrides() $this->assertEquals('v2', $new->version); $this->assertEquals('google/cloud/speech/(v2)', $new->protoPath); } -} \ No newline at end of file + + public function testFromProtoWithAds() + { + $protoContents = <<assertEquals('google.ads.admanager', $new->protoPackage); + $this->assertEquals('Google\Ads\AdManager', $new->phpNamespace); + $this->assertEquals('Google Ads Ad Manager', $new->displayName); + $this->assertEquals('AdsAdManager', $new->componentName); + $this->assertEquals('googleads/ad-manager', $new->composerPackage); + $this->assertEquals('googleapis/php-ads-ad-manager', $new->githubRepo); + $this->assertEquals('GPBMetadata\Google\Ads\Admanager', $new->gpbMetadataNamespace); + $this->assertEquals('admanager', $new->shortName); + $this->assertEquals('v1', $new->version); + $this->assertEquals('google/ads/admanager/(v1)', $new->protoPath); + + // DataManager + $protoContents = <<assertEquals('AdsDataManager', $new->componentName); + $this->assertEquals('googleads/data-manager', $new->composerPackage); + $this->assertEquals('googleapis/php-ads-data-manager', $new->githubRepo); + + // MarketingPlatform Admin + $protoContents = <<assertEquals('AdsMarketingPlatformAdmin', $new->componentName); + $this->assertEquals('googleads/marketingplatform-admin', $new->composerPackage); + $this->assertEquals('googleapis/php-ads-marketingplatform-admin', $new->githubRepo); + } + + public function testFromProtoWithCommonProtos() + { + $protoContents = <<assertEquals('google.geo', $new->protoPackage); + $this->assertEquals('Google\Geo', $new->phpNamespace); + $this->assertEquals('Google Geo Common Protos', $new->displayName); + $this->assertEquals('GeoCommonProtos', $new->componentName); + $this->assertEquals('google/geo-common-protos', $new->composerPackage); + $this->assertEquals('googleapis/php-geo-common-protos', $new->githubRepo); + $this->assertEquals('', $new->shortName); + $this->assertNull($new->version); + $this->assertEquals('google/geo/type', $new->protoPath); + + // Shopping common protos + $protoContents = <<assertEquals('google.shopping', $new->protoPackage); + $this->assertEquals('Google\Shopping', $new->phpNamespace); + $this->assertEquals('Google Shopping Common Protos', $new->displayName); + $this->assertEquals('ShoppingCommonProtos', $new->componentName); + $this->assertEquals('google/shopping-common-protos', $new->composerPackage); + $this->assertEquals('googleapis/php-shopping-common-protos', $new->githubRepo); + $this->assertEquals('', $new->shortName); + } + + public function testFromOptionsWithAds() + { + $options = [ + 'component-name' => 'AdsAdManager', + 'php-namespace' => 'Google\Ads\AdManager\V1', + 'proto-package' => 'google.ads.admanager.v1', + 'api-short-name' => 'admanager', + 'api-version' => 'v1', + ]; + $new = NewComponent::fromOptions($options); + $this->assertEquals('googleads/ad-manager', $new->composerPackage); + $this->assertEquals('googleapis/php-ads-ad-manager', $new->githubRepo); + } + + public function testFromOptionsWithCommonProtos() + { + $options = [ + 'component-name' => 'GeoCommonProtos', + 'php-namespace' => 'Google\Geo', + 'proto-package' => 'google.geo', + 'api-short-name' => '', + 'api-version' => null, + ]; + $new = NewComponent::fromOptions($options); + $this->assertEquals('Google Geo Common Protos', $new->displayName); + $this->assertEquals('GeoCommonProtos', $new->componentName); + $this->assertEquals('google/geo-common-protos', $new->composerPackage); + $this->assertEquals('googleapis/php-geo-common-protos', $new->githubRepo); + $this->assertEquals('', $new->shortName); + } +} From e6eddbd16d6d3656b99d0bcc22a8ffdc1622a56d Mon Sep 17 00:00:00 2001 From: Charlotte Y <38296042+cy-yun@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:13:04 -0700 Subject: [PATCH 2/4] refactor(dev): simplify component parameter derivation per review feedback --- dev/src/NewComponent.php | 122 ++++++++++++---------------- dev/tests/Unit/NewComponentTest.php | 2 +- 2 files changed, 52 insertions(+), 72 deletions(-) diff --git a/dev/src/NewComponent.php b/dev/src/NewComponent.php index 2779f8cbf7b0..f6d7e9a3c012 100644 --- a/dev/src/NewComponent.php +++ b/dev/src/NewComponent.php @@ -43,32 +43,24 @@ class NewComponent public static function fromProto(string $protoContents, string $protoFilename, array $options = []): self { $new = new self(); - $hasGapicClient = self::hasGapicClient($protoContents); + $isCommonProtos = self::isCommonProtos($protoContents); $new->protoPackage = $options['proto-package'] - ?? self::extractPackageNameFromProtoContents($protoContents, $hasGapicClient); + ?? self::extractPackageNameFromProtoContents($protoContents, $isCommonProtos); $new->phpNamespace = $options['php-namespace'] - ?? (self::extractPhpNamespaceFromProtoContents($protoContents, $hasGapicClient) + ?? (self::extractPhpNamespaceFromProtoContents($protoContents, $isCommonProtos) ?: self::derivePhpNamespaceFromProtoPackage($new->protoPackage)); - $new->displayName = self::getDisplayName($new->phpNamespace); + $new->displayName = self::getDisplayName($new->phpNamespace, $isCommonProtos); $new->componentName = $options['component-name'] - ?? self::getComponentName($new->displayName); - if (!$hasGapicClient) { - if (!str_ends_with($new->componentName, 'CommonProtos')) { - $new->componentName .= 'CommonProtos'; - } - if (!str_ends_with($new->displayName, 'Common Protos')) { - $new->displayName .= ' Common Protos'; - } - } + ?? self::getComponentName($new->displayName, $isCommonProtos); $new->composerPackage = self::getComposerPackage( $new->protoPackage, $new->phpNamespace, - $hasGapicClient + $isCommonProtos ); $new->githubRepo = self::getGithubRepo($new->composerPackage); $new->gpbMetadataNamespace = self::getGpbMetadataNamespace($new->protoPackage); $new->shortName = $options['api-short-name'] - ?? self::extractShortNameFromProtoContents($protoContents, $hasGapicClient); + ?? self::extractShortNameFromProtoContents($protoContents, $isCommonProtos); $new->version = array_key_exists('api-version', $options) ? $options['api-version'] : self::extractVersionFromProtoFilename($protoFilename); @@ -87,14 +79,10 @@ public static function fromOptions(array $options): self $new->phpNamespace = $options['php-namespace'] ?? ''; $new->displayName = self::getDisplayName($new->phpNamespace); $new->componentName = $options['component-name'] ?? ''; - $hasGapicClient = !str_ends_with($new->componentName, 'CommonProtos'); - if (!$hasGapicClient && !str_ends_with($new->displayName, 'Common Protos')) { - $new->displayName .= ' Common Protos'; - } $new->composerPackage = self::getComposerPackage( $new->protoPackage, $new->phpNamespace, - $hasGapicClient + str_ends_with($new->componentName, 'CommonProtos') ); $new->githubRepo = self::getGithubRepo($new->composerPackage); $new->gpbMetadataNamespace = self::getGpbMetadataNamespace($new->protoPackage); @@ -123,18 +111,26 @@ private static function getGithubRepo(string $composerPackage): string : 'googleapis/php-' . substr($composerPackage, 7); } - private static function getComponentName(string $displayName): string + private static function getComponentName(string $displayName, bool $isCommonProtos = false): string { - return trim(str_replace(['Google', 'Cloud', ' '], '', $displayName)); + $componentName = trim(str_replace(['Google', 'Cloud', ' '], '', $displayName)); + if ($isCommonProtos && !str_ends_with($componentName, 'CommonProtos')) { + $componentName .= 'CommonProtos'; + } + return $componentName; } - private static function getDisplayName(string $phpNamespace): string + private static function getDisplayName(string $phpNamespace, bool $isCommonProtos = false): string { $nameParts = explode('\\', $phpNamespace); foreach ($nameParts as $i => $part) { $nameParts[$i] = ltrim(strtolower(preg_replace('/[A-Z]([A-Z](?![a-z]))*/', ' $0', $part))); } - return ucwords(implode(' ', $nameParts)); + $displayName = ucwords(implode(' ', $nameParts)); + if ($isCommonProtos && !str_ends_with($displayName, 'Common Protos')) { + $displayName .= ' Common Protos'; + } + return $displayName; } private static function getGpbMetadataNamespace(string $protoPackage): string @@ -150,44 +146,38 @@ private static function derivePhpNamespaceFromProtoPackage(string $protoPackage) private static function getComposerPackage( string $protoPackage, string $phpNamespace, - bool $hasGapicClient = true + bool $isCommonProtos = false ): string { $parts = explode('.', $protoPackage); + if ($isCommonProtos && in_array(end($parts), ['type', 'common'])) { + array_pop($parts); + } if (str_starts_with($phpNamespace, 'Google\\Ads')) { - if (count($parts) > 1 && 'v' === strtolower($parts[count($parts) - 1][0] ?? '')) { + if (count($parts) > 1 && 'v' === strtolower(end($parts)[0])) { array_pop($parts); } $name = str_replace( - ['google.', 'devtools.cloud', '.'], - ['', 'cloud-', '-'], + ['google.ads.', 'ads.', 'google.', '.'], + ['', '', '', '-'], implode('.', $parts) ); - if (str_starts_with($name, 'ads-')) { - $name = substr($name, 4); - } if (str_ends_with($name, 'manager') && !str_ends_with($name, '-manager')) { $name = substr($name, 0, -7) . '-manager'; } - if (!$hasGapicClient && !str_ends_with($name, '-common-protos')) { - $name .= '-common-protos'; - } - return 'googleads/' . $name; - } - - if (!$hasGapicClient && count($parts) > 2 && in_array(end($parts), ['type', 'common'])) { - array_pop($parts); + $vendor = 'googleads'; + } else { + $name = str_replace( + ['google.', 'devtools.cloud', '.'], + ['', 'cloud-', '-'], + implode('.', $parts) + ); + $vendor = 'google'; } - $name = str_replace( - ['google.', 'devtools.cloud', '.'], - ['', 'cloud-', '-'], - implode('.', $parts) - ); - - if (!$hasGapicClient && !str_ends_with($name, '-common-protos')) { + if ($isCommonProtos && !str_ends_with($name, '-common-protos')) { $name .= '-common-protos'; } - return 'google/' . $name; + return $vendor . '/' . $name; } private static function getProtoPath(string $protoFilename, ?string $version): string @@ -204,42 +194,36 @@ private static function getProtoPath(string $protoFilename, ?string $version): s return implode('/', $parts); } - private static function hasGapicClient(string $protoContents): bool + private static function isCommonProtos(string $protoContents): bool { - return (bool) preg_match('/^\s*service\s+[A-Za-z0-9_]+/m', $protoContents); + return !preg_match('/^\s*service\s+[A-Za-z0-9_]+/m', $protoContents); } private static function extractPackageNameFromProtoContents( string $protoContents, - bool $hasGapicClient = true + bool $isCommonProtos = false ): string { if (!preg_match('/package (.*);/', $protoContents, $matches)) { throw new RuntimeException('package name not found in proto file '); } $parts = explode('.', $matches[1]); - while (count($parts) > 1) { - $last = end($parts); - if ('v' === $last[0]) { - array_pop($parts); - } elseif (!$hasGapicClient && count($parts) > 2 && in_array($last, ['type', 'common'])) { - array_pop($parts); - } else { - break; - } + $version = array_pop($parts); + if ('v' !== $version[0] && (!$isCommonProtos || !in_array($version, ['type', 'common']))) { + $parts[] = $version; } return implode('.', $parts); } private static function extractShortNameFromProtoContents( string $protoContents, - bool $hasGapicClient = true + bool $isCommonProtos = false ): string { if (!preg_match( '/option \(google.api.default_host\) =[\n\r\s]+"(.*).googleapis.com";/', $protoContents, $matches )) { - if (!$hasGapicClient) { + if ($isCommonProtos) { return ''; } throw new RuntimeException('short name not found in proto file'); @@ -249,22 +233,18 @@ private static function extractShortNameFromProtoContents( private static function extractPhpNamespaceFromProtoContents( string $protoContents, - bool $hasGapicClient = true + bool $isCommonProtos = false ): ?string { if (!preg_match('/option php_namespace = "(.*)";/', $protoContents, $matches)) { return null; } // Remove version from namespace $parts = explode('\\\\', $matches[1]); - while (count($parts) > 1) { - $last = end($parts); - if ('v' === strtolower($last[0])) { - array_pop($parts); - } elseif (!$hasGapicClient && count($parts) > 2 && in_array(strtolower($last), ['type', 'common'])) { - array_pop($parts); - } else { - break; - } + $version = array_pop($parts); + if ('v' !== strtolower($version[0]) + && (!$isCommonProtos || !in_array(strtolower($version), ['type', 'common'])) + ) { + $parts[] = $version; } return implode('\\', $parts); diff --git a/dev/tests/Unit/NewComponentTest.php b/dev/tests/Unit/NewComponentTest.php index 50c9711a7028..d622bb9d1ad3 100644 --- a/dev/tests/Unit/NewComponentTest.php +++ b/dev/tests/Unit/NewComponentTest.php @@ -271,7 +271,7 @@ public function testFromOptionsWithCommonProtos() 'api-version' => null, ]; $new = NewComponent::fromOptions($options); - $this->assertEquals('Google Geo Common Protos', $new->displayName); + $this->assertEquals('Google Geo', $new->displayName); $this->assertEquals('GeoCommonProtos', $new->componentName); $this->assertEquals('google/geo-common-protos', $new->composerPackage); $this->assertEquals('googleapis/php-geo-common-protos', $new->githubRepo); From 2950643e27b66e6d9b55ffc97f99747ed28a01c6 Mon Sep 17 00:00:00 2001 From: Charlotte Y <38296042+cy-yun@users.noreply.github.com> Date: Thu, 17 Sep 2026 12:48:37 -0700 Subject: [PATCH 3/4] refactor(dev): simplify composer package derivation per review feedback --- dev/src/NewComponent.php | 31 ++++++------------- .../Unit/Command/ComponentNewCommandTest.php | 4 +-- dev/tests/Unit/NewComponentTest.php | 14 ++++----- 3 files changed, 18 insertions(+), 31 deletions(-) diff --git a/dev/src/NewComponent.php b/dev/src/NewComponent.php index f6d7e9a3c012..6213cad8716e 100644 --- a/dev/src/NewComponent.php +++ b/dev/src/NewComponent.php @@ -148,35 +148,22 @@ private static function getComposerPackage( string $phpNamespace, bool $isCommonProtos = false ): string { - $parts = explode('.', $protoPackage); - if ($isCommonProtos && in_array(end($parts), ['type', 'common'])) { - array_pop($parts); - } + $vendor = 'google'; + if (str_starts_with($phpNamespace, 'Google\\Ads')) { - if (count($parts) > 1 && 'v' === strtolower(end($parts)[0])) { - array_pop($parts); - } - $name = str_replace( - ['google.ads.', 'ads.', 'google.', '.'], - ['', '', '', '-'], - implode('.', $parts) - ); - if (str_ends_with($name, 'manager') && !str_ends_with($name, '-manager')) { - $name = substr($name, 0, -7) . '-manager'; - } $vendor = 'googleads'; - } else { - $name = str_replace( - ['google.', 'devtools.cloud', '.'], - ['', 'cloud-', '-'], - implode('.', $parts) - ); - $vendor = 'google'; } + $name = str_replace( + ['google.ads.', 'google.', 'devtools.cloud', '.'], + ['', '', 'cloud-', '-'], + $protoPackage + ); + if ($isCommonProtos && !str_ends_with($name, '-common-protos')) { $name .= '-common-protos'; } + return $vendor . '/' . $name; } diff --git a/dev/tests/Unit/Command/ComponentNewCommandTest.php b/dev/tests/Unit/Command/ComponentNewCommandTest.php index 4d46708e138d..c067d0b82f84 100644 --- a/dev/tests/Unit/Command/ComponentNewCommandTest.php +++ b/dev/tests/Unit/Command/ComponentNewCommandTest.php @@ -452,7 +452,7 @@ public function testNewComponentAdsWithAllOptions() '--no-update' => true, '--component-name' => 'AdsAdManager', '--php-namespace' => 'Google\Ads\AdManager\V1', - '--proto-package' => 'google.ads.admanager.v1', + '--proto-package' => 'google.ads.admanager', '--api-short-name' => 'admanager', '--api-version' => 'v1', '--product-docs' => 'https://developers.google.com/ad-manager/api', @@ -462,7 +462,7 @@ public function testNewComponentAdsWithAllOptions() $repoMetadataFull = json_decode(file_get_contents(self::$tmpDir . '/.repo-metadata-full.json'), true); $this->assertArrayHasKey('AdsAdManager', $repoMetadataFull); $this->assertEquals('GAPIC_AUTO', $repoMetadataFull['AdsAdManager']['library_type']); - $this->assertEquals('googleads/ad-manager', $repoMetadataFull['AdsAdManager']['distribution_name']); + $this->assertEquals('googleads/admanager', $repoMetadataFull['AdsAdManager']['distribution_name']); $this->assertEquals('admanager', $repoMetadataFull['AdsAdManager']['api_shortname']); } diff --git a/dev/tests/Unit/NewComponentTest.php b/dev/tests/Unit/NewComponentTest.php index d622bb9d1ad3..8645b344f5f6 100644 --- a/dev/tests/Unit/NewComponentTest.php +++ b/dev/tests/Unit/NewComponentTest.php @@ -183,8 +183,8 @@ public function testFromProtoWithAds() $this->assertEquals('Google\Ads\AdManager', $new->phpNamespace); $this->assertEquals('Google Ads Ad Manager', $new->displayName); $this->assertEquals('AdsAdManager', $new->componentName); - $this->assertEquals('googleads/ad-manager', $new->composerPackage); - $this->assertEquals('googleapis/php-ads-ad-manager', $new->githubRepo); + $this->assertEquals('googleads/admanager', $new->composerPackage); + $this->assertEquals('googleapis/php-ads-admanager', $new->githubRepo); $this->assertEquals('GPBMetadata\Google\Ads\Admanager', $new->gpbMetadataNamespace); $this->assertEquals('admanager', $new->shortName); $this->assertEquals('v1', $new->version); @@ -199,8 +199,8 @@ public function testFromProtoWithAds() EOF; $new = NewComponent::fromProto($protoContents, 'google/ads/datamanager/v1/data_manager.proto'); $this->assertEquals('AdsDataManager', $new->componentName); - $this->assertEquals('googleads/data-manager', $new->composerPackage); - $this->assertEquals('googleapis/php-ads-data-manager', $new->githubRepo); + $this->assertEquals('googleads/datamanager', $new->composerPackage); + $this->assertEquals('googleapis/php-ads-datamanager', $new->githubRepo); // MarketingPlatform Admin $protoContents = << 'AdsAdManager', 'php-namespace' => 'Google\Ads\AdManager\V1', - 'proto-package' => 'google.ads.admanager.v1', + 'proto-package' => 'google.ads.admanager', 'api-short-name' => 'admanager', 'api-version' => 'v1', ]; $new = NewComponent::fromOptions($options); - $this->assertEquals('googleads/ad-manager', $new->composerPackage); - $this->assertEquals('googleapis/php-ads-ad-manager', $new->githubRepo); + $this->assertEquals('googleads/admanager', $new->composerPackage); + $this->assertEquals('googleapis/php-ads-admanager', $new->githubRepo); } public function testFromOptionsWithCommonProtos() From 69363c084e7cb31bb529b26d355ba576eb68c1e0 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 21 Sep 2026 13:38:36 -0700 Subject: [PATCH 4/4] Apply batched suggestions from code review Co-authored-by: Brent Shaffer --- dev/src/NewComponent.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev/src/NewComponent.php b/dev/src/NewComponent.php index 6213cad8716e..18495ae6e370 100644 --- a/dev/src/NewComponent.php +++ b/dev/src/NewComponent.php @@ -183,6 +183,7 @@ private static function getProtoPath(string $protoFilename, ?string $version): s private static function isCommonProtos(string $protoContents): bool { + // Any package which does not contain service clients is considered "Common Protos" return !preg_match('/^\s*service\s+[A-Za-z0-9_]+/m', $protoContents); } @@ -211,6 +212,7 @@ private static function extractShortNameFromProtoContents( $matches )) { if ($isCommonProtos) { + // common protos do not have an API shortname return ''; } throw new RuntimeException('short name not found in proto file');