From 2b60be30aa6195c54dee29f858966ef340434204 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 26 Aug 2025 15:24:09 -0700 Subject: [PATCH 1/7] feat: add "database" request param for Firestore::listen --- .../FIrestoreRequestParamProcessor.php | 131 ++++++++++++++++++ .../FirestoreRequestParamProcessorTest.php | 104 ++++++++++++++ 2 files changed, 235 insertions(+) create mode 100644 src/PostProcessor/FIrestoreRequestParamProcessor.php create mode 100644 tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php diff --git a/src/PostProcessor/FIrestoreRequestParamProcessor.php b/src/PostProcessor/FIrestoreRequestParamProcessor.php new file mode 100644 index 000000000..6ab88f180 --- /dev/null +++ b/src/PostProcessor/FIrestoreRequestParamProcessor.php @@ -0,0 +1,131 @@ +addDatabaseRequestParamToListenMethod(); + + // Write the new contents to the class file. + file_put_contents($classFile, $processor->getContents()); + print("Fragment written to $classFile\n"); + } + + private static function fromCode(string $contents): ClassDeclaration + { + $parser = new Parser(); + $astNode = $parser->parseSourceFile($contents); + if ($errors = DiagnosticsProvider::getDiagnostics($astNode)) { + throw new ParseError('Provided contents contains a PHP syntax error'); + } + + foreach ($astNode->getDescendantNodes() as $childNode) { + if ($childNode instanceof ClassDeclaration) { + return $childNode; + } + } + + throw new LogicException('Provided contents does not contain a PHP class'); + } + + public function __construct(string $contents) + { + $this->classNode = self::fromCode($contents); + } + + /** + * @throws LogicException + * @throws ParseError + */ + public function addDatabaseRequestParamToListenMethod(): void + { + $listenMethod = $this->getMethodDeclaration('listen'); + + // update PHPDoc + $lineToReplace = ' * @type int $timeoutMillis'; + $newLines = [ + ' * @type string $datbase', + ' * Set the database of the call, to be added as a routing header', + $lineToReplace, + ]; + + $phpdoc = $listenMethod->getDocCommentText(); + $newPhpdoc = str_replace($lineToReplace, implode(PHP_EOL, $newLines), $phpdoc); + $newContents = str_replace($phpdoc, $newPhpdoc, $this->classNode->getFileContents()); + + // update listen method + $lineToReplace = ' return $this->startApiCall(\'Listen\', null, $callOptions);'; + $newLines = [ + ' $requestParamHeaders = [];', + ' if (isset($callOptions[\'database\'])) {', + ' $requestParamHeaders[\'database\'] = $callOptions[\'database\'];', + ' }', + ' $requestParams = new \Google\ApiCore\RequestParamsHeaderDescriptor($requestParamHeaders);', + ' $callOptions[\'headers\'] = isset($optionalArgs[\'headers\']) ? array_merge($requestParams->getHeader(), $callOptions[\'headers\']) : $requestParams->getHeader();', + $lineToReplace, + ]; + $methodText = $listenMethod->compoundStatementOrSemicolon->getText(); + $newMethodText = str_replace($lineToReplace, implode(PHP_EOL, $newLines), $methodText); + $newContents = str_replace($methodText, $newMethodText, $newContents); + + $this->classNode = self::fromCode($newContents); + } + + public function getContents(): string + { + return $this->classNode->getFileContents(); + } + + private function getMethodDeclaration(string $insertBeforeMethod): MethodDeclaration + { + foreach ($this->classNode->getDescendantNodes() as $childNode) { + if ($childNode instanceof MethodDeclaration) { + if ($childNode->getName() === $insertBeforeMethod) { + return $childNode; + } + } + } + + throw new LogicException( + 'Provided contents does not contain method ' . $insertBeforeMethod + ); + } +} diff --git a/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php b/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php new file mode 100644 index 000000000..8214887bb --- /dev/null +++ b/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php @@ -0,0 +1,104 @@ + self::SERVICE_NAME, + ]; + } + + public function __construct(array \$options = []) + { + \$clientOptions = \$this->buildClientOptions(\$options); + \$this->setClientOptions(\$clientOptions); + } + + /** + * Listens to changes. This method is only available via gRPC or WebChannel + * (not REST). + * + * @example samples/V1/FirestoreClient/listen.php + * + * @param array \$callOptions { + * Optional. + * + * @type int \$timeoutMillis + * Timeout to use for this call. + * } + * + * @return BidiStream + * + * @throws ApiException Thrown if the API call fails. + */ + public function listen(array \$callOptions = []): BidiStream + { + return \$this->startApiCall('Listen', null, \$callOptions); + } +} +EOL; + + public function testFirestoreRequestParamProcessor() + { + $firestorePostProcessor = new FirestoreRequestParamProcessor(self::$classContents); + + // Insert the function before this one + $firestorePostProcessor->addDatabaseRequestParamToListenMethod(); + $newClassContents = $firestorePostProcessor->getContents(); + + $this->assertStringContainsString('@type string $datbase', $newClassContents); + $this->assertStringContainsString('new \Google\ApiCore\RequestParamsHeaderDescriptor', $newClassContents); + } + + public function testFirestoreRequestParamDoesNotContainSyntaxErrors() + { + $firestorePostProcessor = new FirestoreRequestParamProcessor(self::$classContents); + + $codeString = $firestorePostProcessor->getContents(); + $tempFile = tempnam(sys_get_temp_dir(), 'phpunit_check_syntax_'); + file_put_contents($tempFile, $codeString); + + $command = 'php -l ' . escapeshellarg($tempFile); + $output = []; + $returnVar = 0; + exec($command, $output, $returnVar); + + $this->assertEquals(0, $returnVar, 'The code output contains a syntax error'); + + unlink($tempFile); // Clean up the temporary file + } +} From 906efe0ef760fa90d7f8eae59083959492ee8bc4 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 26 Aug 2025 15:28:45 -0700 Subject: [PATCH 2/7] fix filename casing --- ...questParamProcessor.php => FirestoreRequestParamProcessor.php} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/PostProcessor/{FIrestoreRequestParamProcessor.php => FirestoreRequestParamProcessor.php} (100%) diff --git a/src/PostProcessor/FIrestoreRequestParamProcessor.php b/src/PostProcessor/FirestoreRequestParamProcessor.php similarity index 100% rename from src/PostProcessor/FIrestoreRequestParamProcessor.php rename to src/PostProcessor/FirestoreRequestParamProcessor.php From 83b5977a301713b7ed5e7d79610112c60fe88385 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Wed, 27 Aug 2025 08:58:56 -0700 Subject: [PATCH 3/7] cleanup gencode --- .../FirestoreRequestParamProcessor.php | 76 +++++++++++++------ 1 file changed, 51 insertions(+), 25 deletions(-) diff --git a/src/PostProcessor/FirestoreRequestParamProcessor.php b/src/PostProcessor/FirestoreRequestParamProcessor.php index 6ab88f180..6932083fe 100644 --- a/src/PostProcessor/FirestoreRequestParamProcessor.php +++ b/src/PostProcessor/FirestoreRequestParamProcessor.php @@ -21,13 +21,36 @@ use Microsoft\PhpParser\Node\Statement\ClassDeclaration; use Microsoft\PhpParser\Node\MethodDeclaration; use Microsoft\PhpParser\Parser; -use Microsoft\PhpParser\PositionUtilities; use Microsoft\PhpParser\DiagnosticsProvider; use LogicException; use ParseError; class FirestoreRequestParamProcessor implements ProcessorInterface { + // Line to insert the PHP doc param above + private const DATABASE_PHPDOC_INSERT_AT = + ' * @type int $timeoutMillis'; + + // PHPdoc param to insert + private const DATABASE_PHPDOC_PARAM = <<<'EOL' + * @type string $datbase + * Set the database of the call, to be added as a routing header +EOL; + + // Line to insert the request param code above + private const DATABASE_REQUEST_PARAM_INSERT_AT = + ' return $this->startApiCall(\'Listen\', null, $callOptions);'; + + // Request param code to insert + private const DATABASE_REQUEST_PARAM_CODE = <<<'EOL' +$requestParamHeaders = []; +if (isset($callOptions['database'])) { + $requestParamHeaders['database'] = $callOptions['database']; +} +$requestParams = new \Google\ApiCore\RequestParamsHeaderDescriptor($requestParamHeaders); +$callOptions['headers'] = isset($callOptions['headers']) ? array_merge($requestParams->getHeader(), $callOptions['headers']) : $requestParams->getHeader(); +EOL; + private ClassDeclaration $classNode; public static function run(string $inputDir): void @@ -77,36 +100,39 @@ public function __construct(string $contents) */ public function addDatabaseRequestParamToListenMethod(): void { + $contents = $this->classNode->getFileContents(); $listenMethod = $this->getMethodDeclaration('listen'); // update PHPDoc - $lineToReplace = ' * @type int $timeoutMillis'; - $newLines = [ - ' * @type string $datbase', - ' * Set the database of the call, to be added as a routing header', - $lineToReplace, - ]; - $phpdoc = $listenMethod->getDocCommentText(); - $newPhpdoc = str_replace($lineToReplace, implode(PHP_EOL, $newLines), $phpdoc); - $newContents = str_replace($phpdoc, $newPhpdoc, $this->classNode->getFileContents()); - - // update listen method - $lineToReplace = ' return $this->startApiCall(\'Listen\', null, $callOptions);'; - $newLines = [ - ' $requestParamHeaders = [];', - ' if (isset($callOptions[\'database\'])) {', - ' $requestParamHeaders[\'database\'] = $callOptions[\'database\'];', - ' }', - ' $requestParams = new \Google\ApiCore\RequestParamsHeaderDescriptor($requestParamHeaders);', - ' $callOptions[\'headers\'] = isset($optionalArgs[\'headers\']) ? array_merge($requestParams->getHeader(), $callOptions[\'headers\']) : $requestParams->getHeader();', - $lineToReplace, - ]; + if (false === strpos($phpdoc, self::DATABASE_PHPDOC_PARAM)) { + $newLines = explode(PHP_EOL, self::DATABASE_PHPDOC_PARAM); + $newLines[] = self::DATABASE_PHPDOC_INSERT_AT; + + + $newPhpdoc = str_replace(self::DATABASE_PHPDOC_INSERT_AT, implode(PHP_EOL, $newLines), $phpdoc); + $contents = str_replace($phpdoc, $newPhpdoc, $contents); + } + + // Update param code $methodText = $listenMethod->compoundStatementOrSemicolon->getText(); - $newMethodText = str_replace($lineToReplace, implode(PHP_EOL, $newLines), $methodText); - $newContents = str_replace($methodText, $newMethodText, $newContents); + // indent each line 8 spaces + $indent = str_repeat(' ', 8); + $newLines = array_map( + fn ($line) => $indent . $line, + explode(PHP_EOL, self::DATABASE_REQUEST_PARAM_CODE) + ); + if (false === strpos($methodText, $newLines[0])) { + $newLines[] = self::DATABASE_REQUEST_PARAM_INSERT_AT; + + $newMethodText = str_replace( + self::DATABASE_REQUEST_PARAM_INSERT_AT, + implode(PHP_EOL, $newLines), $methodText + ); + $contents = str_replace($methodText, $newMethodText, $contents); + } - $this->classNode = self::fromCode($newContents); + $this->classNode = self::fromCode($contents); } public function getContents(): string From db5ed2ef8757d83ccd942af1e9f6dd82ebe3fe08 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Wed, 27 Aug 2025 09:37:35 -0700 Subject: [PATCH 4/7] add PostProcessorTrait --- .../FirestoreRequestParamProcessor.php | 41 ++----------- .../FragmentInjectionProcessor.php | 32 +--------- src/PostProcessor/PostProcessor.php | 5 +- src/PostProcessor/PostProcessorTrait.php | 59 +++++++++++++++++++ .../FirestoreRequestParamProcessorTest.php | 17 ++---- 5 files changed, 74 insertions(+), 80 deletions(-) create mode 100644 src/PostProcessor/PostProcessorTrait.php diff --git a/src/PostProcessor/FirestoreRequestParamProcessor.php b/src/PostProcessor/FirestoreRequestParamProcessor.php index 6932083fe..85e084ead 100644 --- a/src/PostProcessor/FirestoreRequestParamProcessor.php +++ b/src/PostProcessor/FirestoreRequestParamProcessor.php @@ -1,6 +1,6 @@ getHeader(), $callOptions['headers']) : $requestParams->getHeader(); EOL; - private ClassDeclaration $classNode; - public static function run(string $inputDir): void { - $firestoreClientFile = $inputDir . '/src/V1/Client/FirestoreClient.php'; - self::inject($firestoreClientFile); + self::inject($inputDir . '/src/V1/Client/FirestoreClient.php'); } private static function inject(string $classFile): void @@ -72,28 +68,6 @@ private static function inject(string $classFile): void print("Fragment written to $classFile\n"); } - private static function fromCode(string $contents): ClassDeclaration - { - $parser = new Parser(); - $astNode = $parser->parseSourceFile($contents); - if ($errors = DiagnosticsProvider::getDiagnostics($astNode)) { - throw new ParseError('Provided contents contains a PHP syntax error'); - } - - foreach ($astNode->getDescendantNodes() as $childNode) { - if ($childNode instanceof ClassDeclaration) { - return $childNode; - } - } - - throw new LogicException('Provided contents does not contain a PHP class'); - } - - public function __construct(string $contents) - { - $this->classNode = self::fromCode($contents); - } - /** * @throws LogicException * @throws ParseError @@ -135,11 +109,6 @@ public function addDatabaseRequestParamToListenMethod(): void $this->classNode = self::fromCode($contents); } - public function getContents(): string - { - return $this->classNode->getFileContents(); - } - private function getMethodDeclaration(string $insertBeforeMethod): MethodDeclaration { foreach ($this->classNode->getDescendantNodes() as $childNode) { diff --git a/src/PostProcessor/FragmentInjectionProcessor.php b/src/PostProcessor/FragmentInjectionProcessor.php index 3c1e1afcb..02da3a238 100644 --- a/src/PostProcessor/FragmentInjectionProcessor.php +++ b/src/PostProcessor/FragmentInjectionProcessor.php @@ -18,17 +18,14 @@ namespace Google\PostProcessor; -use Microsoft\PhpParser\Node\Statement\ClassDeclaration; use Microsoft\PhpParser\Node\MethodDeclaration; -use Microsoft\PhpParser\Parser; use Microsoft\PhpParser\PositionUtilities; -use Microsoft\PhpParser\DiagnosticsProvider; use LogicException; use ParseError; class FragmentInjectionProcessor implements ProcessorInterface { - private ClassDeclaration $classNode; + use PostProcessorTrait; public static function run(string $inputDir): void { @@ -62,28 +59,6 @@ private static function inject(string $fragmentFile, string $classFile): void print("Fragment written to $classFile\n"); } - private static function fromCode(string $contents): ClassDeclaration - { - $parser = new Parser(); - $astNode = $parser->parseSourceFile($contents); - if ($errors = DiagnosticsProvider::getDiagnostics($astNode)) { - throw new ParseError('Provided contents contains a PHP syntax error'); - } - - foreach ($astNode->getDescendantNodes() as $childNode) { - if ($childNode instanceof ClassDeclaration) { - return $childNode; - } - } - - throw new LogicException('Provided contents does not contain a PHP class'); - } - - public function __construct(string $contents) - { - $this->classNode = self::fromCode($contents); - } - /** * @throws LogicException * @throws ParseError @@ -100,11 +75,6 @@ public function insert(string $newContent, ?string $insertBeforeMethod = null): $this->classNode = self::fromCode($contents); } - public function getContents(): string - { - return $this->classNode->getFileContents(); - } - private function getLineNumberFromPosition(int $startPosition): int { return PositionUtilities::getLineCharacterPositionFromPosition( diff --git a/src/PostProcessor/PostProcessor.php b/src/PostProcessor/PostProcessor.php index caa50d4a2..0610045b5 100644 --- a/src/PostProcessor/PostProcessor.php +++ b/src/PostProcessor/PostProcessor.php @@ -45,6 +45,9 @@ private function execute(): void private static function loadProcessors(array $opts): Vector { - return Vector::new([FragmentInjectionProcessor::class]); + return Vector::new([ + FragmentInjectionProcessor::class, + FirestoreRequestParamProcessor::class, + ]); } } diff --git a/src/PostProcessor/PostProcessorTrait.php b/src/PostProcessor/PostProcessorTrait.php new file mode 100644 index 000000000..9c8021662 --- /dev/null +++ b/src/PostProcessor/PostProcessorTrait.php @@ -0,0 +1,59 @@ +classNode = self::fromCode($contents); + } + + public function getContents(): string + { + return $this->classNode->getFileContents(); + } + + public static abstract function run(string $inputDir): void; + + private static function fromCode(string $contents): ClassDeclaration + { + $parser = new Parser(); + $astNode = $parser->parseSourceFile($contents); + if ($errors = DiagnosticsProvider::getDiagnostics($astNode)) { + throw new ParseError('Provided contents contains a PHP syntax error'); + } + + foreach ($astNode->getDescendantNodes() as $childNode) { + if ($childNode instanceof ClassDeclaration) { + return $childNode; + } + } + + throw new LogicException('Provided contents does not contain a PHP class'); + } +} \ No newline at end of file diff --git a/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php b/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php index 8214887bb..a5393932c 100644 --- a/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php +++ b/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php @@ -1,6 +1,6 @@ addDatabaseRequestParamToListenMethod(); $newClassContents = $firestorePostProcessor->getContents(); - $this->assertStringContainsString('@type string $datbase', $newClassContents); + $this->assertStringContainsString('@type string $database', $newClassContents); $this->assertStringContainsString('new \Google\ApiCore\RequestParamsHeaderDescriptor', $newClassContents); } @@ -89,16 +89,9 @@ public function testFirestoreRequestParamDoesNotContainSyntaxErrors() $firestorePostProcessor = new FirestoreRequestParamProcessor(self::$classContents); $codeString = $firestorePostProcessor->getContents(); - $tempFile = tempnam(sys_get_temp_dir(), 'phpunit_check_syntax_'); - file_put_contents($tempFile, $codeString); - $command = 'php -l ' . escapeshellarg($tempFile); - $output = []; - $returnVar = 0; - exec($command, $output, $returnVar); - - $this->assertEquals(0, $returnVar, 'The code output contains a syntax error'); - - unlink($tempFile); // Clean up the temporary file + // This will throw a ParseError if the syntax is invalid + $tokens = token_get_all("assertNotNull($tokens); } } From 647d2f703efd22a9e3f2480a185414e1a42e9f40 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Wed, 27 Aug 2025 09:39:35 -0700 Subject: [PATCH 5/7] fix syntax error --- src/PostProcessor/FirestoreRequestParamProcessor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PostProcessor/FirestoreRequestParamProcessor.php b/src/PostProcessor/FirestoreRequestParamProcessor.php index 85e084ead..5050266ef 100644 --- a/src/PostProcessor/FirestoreRequestParamProcessor.php +++ b/src/PostProcessor/FirestoreRequestParamProcessor.php @@ -50,7 +50,7 @@ class FirestoreRequestParamProcessor implements ProcessorInterface $callOptions['headers'] = isset($callOptions['headers']) ? array_merge($requestParams->getHeader(), $callOptions['headers']) : $requestParams->getHeader(); EOL; - + public static function run(string $inputDir): void { self::inject($inputDir . '/src/V1/Client/FirestoreClient.php'); } From e69006f635482a847e4c11a0c294a385d6d84977 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Wed, 27 Aug 2025 09:46:39 -0700 Subject: [PATCH 6/7] only run Firestore post processor in Firestore --- src/PostProcessor/FirestoreRequestParamProcessor.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/PostProcessor/FirestoreRequestParamProcessor.php b/src/PostProcessor/FirestoreRequestParamProcessor.php index 5050266ef..e7fcc527d 100644 --- a/src/PostProcessor/FirestoreRequestParamProcessor.php +++ b/src/PostProcessor/FirestoreRequestParamProcessor.php @@ -52,7 +52,10 @@ class FirestoreRequestParamProcessor implements ProcessorInterface public static function run(string $inputDir): void { - self::inject($inputDir . '/src/V1/Client/FirestoreClient.php'); + $firestoreClientFile = $inputDir . '/src/V1/Client/FirestoreClient.php'; + if (file_exists($firestoreClientFile)) { + self::inject($firestoreClientFile); + } } private static function inject(string $classFile): void From 991d065ec3ed9c4b7631beb7611c2cd990289738 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Wed, 27 Aug 2025 09:51:10 -0700 Subject: [PATCH 7/7] add test --- .../PostProcessor/FirestoreRequestParamProcessorTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php b/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php index a5393932c..e45825c4c 100644 --- a/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php +++ b/tests/Unit/PostProcessor/FirestoreRequestParamProcessorTest.php @@ -82,6 +82,12 @@ public function testFirestoreRequestParamProcessor() $this->assertStringContainsString('@type string $database', $newClassContents); $this->assertStringContainsString('new \Google\ApiCore\RequestParamsHeaderDescriptor', $newClassContents); + + // Insert again and ensure it's not added twice + $firestorePostProcessor->addDatabaseRequestParamToListenMethod(); + $newClassContents = $firestorePostProcessor->getContents(); + $this->assertEquals(1, substr_count($newClassContents, '@type string $database')); + $this->assertEquals(1, substr_count($newClassContents, 'new \Google\ApiCore\RequestParamsHeaderDescriptor')); } public function testFirestoreRequestParamDoesNotContainSyntaxErrors()