From 3c2c6bc7320b644ddc5bc9ff4455c05e012dc023 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 13 Oct 2020 12:04:25 -0700 Subject: [PATCH 1/3] test: remove travis env var --- tests/exampleTest.php | 4 ++-- tests/vendorTest.php | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/exampleTest.php b/tests/exampleTest.php index dbf38161..0c63cd17 100644 --- a/tests/exampleTest.php +++ b/tests/exampleTest.php @@ -31,8 +31,8 @@ class exampleTest extends TestCase public static function setUpBeforeClass(): void { - if ('true' === getenv('TRAVIS')) { - self::markTestSkipped('These tests do not pass on travis'); + if ('true' === getenv('SKIP_EXAMPLE_TESTS')) { + self::markTestSkipped('Explicitly skipping the example tests'); } $exampleDir = __DIR__ . '/../examples/hello'; diff --git a/tests/vendorTest.php b/tests/vendorTest.php index 008c12c6..625a9d64 100644 --- a/tests/vendorTest.php +++ b/tests/vendorTest.php @@ -31,9 +31,10 @@ class vendorTest extends TestCase public static function setUpBeforeClass(): void { - if ('true' === getenv('TRAVIS')) { - self::markTestSkipped('These tests do not pass on travis'); + if ('true' === getenv('SKIP_EXAMPLE_TESTS')) { + self::markTestSkipped('Explicitly skipping the example tests'); } + mkdir($tmpDir = sys_get_temp_dir() . '/ff-php-test-' . rand()); chdir($tmpDir); From d5c938c4f3226bfbe639e41ed89a5874fbf2a71e Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 13 Oct 2020 12:47:00 -0700 Subject: [PATCH 2/3] fix: tests, exception messages, and readme --- README.md | 24 ++------------- src/FunctionWrapper.php | 37 ++++++++++++++++-------- tests/CloudEventFunctionsWrapperTest.php | 12 ++++++-- tests/HttpFunctionWrapperTest.php | 12 ++++++-- tests/fixtures/absolute.php | 4 ++- tests/fixtures/index.php | 4 ++- tests/fixtures/relative.php | 4 ++- 7 files changed, 55 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 2b51f84c..ce440f52 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,9 @@ different environments, including: The framework allows you to go from: ```php -function helloHttp() +use Psr\Http\Message\ServerRequestInterface; + +function helloHttp(ServerRequestInterface $request) { return "Hello World from a PHP HTTP function!" . PHP_EOL; } @@ -235,26 +237,6 @@ You can configure the Functions Framework using the environment variables shown | `FUNCTION_SOURCE` | The name of the file containing the source code for your function to load. Default: **`index.php`** (if it exists) | `FUNCTION_SIGNATURE_TYPE` | The signature used when writing your function. Controls unmarshalling rules and determines which arguments are used to invoke your function. Can be either `http`, `event`, or `cloudevent`. Default: **`http`** -# Enable Background Events - -The Functions Framework can unmarshall incoming event payloads to `data` and -`context` objects. These will be passed as arguments to your function when it -receives a request. Note that your function must use the event-style function -signature: - -```php -function helloEvents($data, $context) -{ - var_dump($data); - var_dump($context); -} -``` - -To enable automatic unmarshalling, set the `FUNCTION_SIGNATURE_TYPE` environment -variable to `event`. For more details on this signature type, check out the Google Cloud Functions -documentation on -[background functions](https://cloud.google.com/functions/docs/writing/background#cloud_pubsub_example). - # Enable CloudEvents The Functions Framework can unmarshall incoming [CloudEvents](http://cloudevents.io) diff --git a/src/FunctionWrapper.php b/src/FunctionWrapper.php index 4f6eb547..61fa50fe 100644 --- a/src/FunctionWrapper.php +++ b/src/FunctionWrapper.php @@ -69,21 +69,34 @@ private function validateFunctionSignature( ReflectionFunctionAbstract $reflection ) { $parameters = $reflection->getParameters(); - if (count($parameters) != 1) { - throw new LogicException( - 'Wrong number of parameters to your function, must be exactly 1' - ); + // Check there is at least one parameter + if (count($parameters) < 1) { + $this->throwInvalidFunctionException(); } - - $class = $this->getFunctionParameterClassName(); + // Check the first parameter has the proper typehint $type = $parameters[0]->getType(); + $class = $this->getFunctionParameterClassName(); if (!$type || $type->getName() !== $class) { - throw new LogicException( - sprintf( - 'Your function must have "%s" as the typehint for the first argument', - $class - ) - ); + $this->throwInvalidFunctionException(); + } + + if (count($parameters) > 1) { + for ($i = 1; $i < count($parameters); $i++) { + if (!$parameters[$i]->isOptional()) { + throw new LogicException( + 'If your function acceps more than one parameter the ' + . 'additional parameters must be optional' + ); + } + } } } + + private function throwInvalidFunctionException() + { + throw new LogicException(sprintf( + 'Your function must have "%s" as the typehint for the first argument', + $this->getFunctionParameterClassName() + )); + } } diff --git a/tests/CloudEventFunctionsWrapperTest.php b/tests/CloudEventFunctionsWrapperTest.php index 5d96bdb9..50c3a9fe 100644 --- a/tests/CloudEventFunctionsWrapperTest.php +++ b/tests/CloudEventFunctionsWrapperTest.php @@ -58,7 +58,7 @@ public function testNoFunctionParameters() { $this->expectException('LogicException'); $this->expectExceptionMessage( - 'Wrong number of parameters to your function, must be exactly 1' + 'Your function must have "Google\CloudFunctions\CloudEvent" as the typehint for the first argument' ); $request = new ServerRequest('POST', '/', []); $cloudEventFunctionWrapper = new CloudEventFunctionWrapper( @@ -71,10 +71,10 @@ public function testTooManyFunctionParameters() { $this->expectException('LogicException'); $this->expectExceptionMessage( - 'Wrong number of parameters to your function, must be exactly 1' + 'If your function acceps more than one parameter the additional parameters must be optional' ); $cloudEventFunctionWrapper = new CloudEventFunctionWrapper( - function ($foo, $bar) { + function (CloudEvent $foo, $bar) { } ); } @@ -117,6 +117,12 @@ function (CloudEvent $foo = null) { } ); $this->assertTrue(true, 'No exception was thrown'); + // additional optional parameters are ok + $cloudEventFunctionWrapper = new CloudEventFunctionWrapper( + function (CloudEvent $foo, $bar = null) { + } + ); + $this->assertTrue(true, 'No exception was thrown'); } public function testWithFullCloudEvent() diff --git a/tests/HttpFunctionWrapperTest.php b/tests/HttpFunctionWrapperTest.php index b03803ba..6ade3b17 100644 --- a/tests/HttpFunctionWrapperTest.php +++ b/tests/HttpFunctionWrapperTest.php @@ -31,7 +31,7 @@ public function testNoFunctionParameters() { $this->expectException('LogicException'); $this->expectExceptionMessage( - 'Wrong number of parameters to your function, must be exactly 1' + 'Your function must have "Psr\Http\Message\ServerRequestInterface" as the typehint for the first argument' ); $request = new ServerRequest('POST', '/', []); $httpFunctionWrapper = new HttpFunctionWrapper( @@ -44,10 +44,10 @@ public function testTooManyFunctionParameters() { $this->expectException('LogicException'); $this->expectExceptionMessage( - 'Wrong number of parameters to your function, must be exactly 1' + 'If your function acceps more than one parameter the additional parameters must be optional' ); $httpFunctionWrapper = new HttpFunctionWrapper( - function ($foo, $bar) { + function (ServerRequestInterface $foo, $bar) { } ); } @@ -90,6 +90,12 @@ function (ServerRequestInterface $foo = null) { } ); $this->assertTrue(true, 'No exception was thrown'); + // additional optional parameters are ok + $httpFunctionWrapper = new HttpFunctionWrapper( + function (ServerRequestInterface $foo, $bar = null) { + } + ); + $this->assertTrue(true, 'No exception was thrown'); } public function testHttpHttpFunctionWrapper() diff --git a/tests/fixtures/absolute.php b/tests/fixtures/absolute.php index c126b108..db23f988 100644 --- a/tests/fixtures/absolute.php +++ b/tests/fixtures/absolute.php @@ -1,6 +1,8 @@ Date: Tue, 13 Oct 2020 12:48:04 -0700 Subject: [PATCH 3/3] typo fix --- src/FunctionWrapper.php | 2 +- tests/CloudEventFunctionsWrapperTest.php | 2 +- tests/HttpFunctionWrapperTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/FunctionWrapper.php b/src/FunctionWrapper.php index 61fa50fe..616c13a2 100644 --- a/src/FunctionWrapper.php +++ b/src/FunctionWrapper.php @@ -84,7 +84,7 @@ private function validateFunctionSignature( for ($i = 1; $i < count($parameters); $i++) { if (!$parameters[$i]->isOptional()) { throw new LogicException( - 'If your function acceps more than one parameter the ' + 'If your function accepts more than one parameter the ' . 'additional parameters must be optional' ); } diff --git a/tests/CloudEventFunctionsWrapperTest.php b/tests/CloudEventFunctionsWrapperTest.php index 50c3a9fe..1897b371 100644 --- a/tests/CloudEventFunctionsWrapperTest.php +++ b/tests/CloudEventFunctionsWrapperTest.php @@ -71,7 +71,7 @@ public function testTooManyFunctionParameters() { $this->expectException('LogicException'); $this->expectExceptionMessage( - 'If your function acceps more than one parameter the additional parameters must be optional' + 'If your function accepts more than one parameter the additional parameters must be optional' ); $cloudEventFunctionWrapper = new CloudEventFunctionWrapper( function (CloudEvent $foo, $bar) { diff --git a/tests/HttpFunctionWrapperTest.php b/tests/HttpFunctionWrapperTest.php index 6ade3b17..58f83921 100644 --- a/tests/HttpFunctionWrapperTest.php +++ b/tests/HttpFunctionWrapperTest.php @@ -44,7 +44,7 @@ public function testTooManyFunctionParameters() { $this->expectException('LogicException'); $this->expectExceptionMessage( - 'If your function acceps more than one parameter the additional parameters must be optional' + 'If your function accepts more than one parameter the additional parameters must be optional' ); $httpFunctionWrapper = new HttpFunctionWrapper( function (ServerRequestInterface $foo, $bar) {