fix: better error handling and cleaner testing for imagemagick sample (GoogleCloudPlatform#1341)

Author: bshaffer

functions/imagemagick/index.php

@@ -20,13 +20,15 @@
 use Google\Cloud\Storage\StorageClient;
 use Google\Cloud\Vision\V1\ImageAnnotatorClient;
 use Google\Cloud\Vision\V1\Likelihood;
+use Google\Rpc\Code;
 
 // [END functions_imagemagick_setup]
 
 // [START functions_imagemagick_analyze]
 function blurOffensiveImages(CloudEvent $cloudevent): void
 {
     $log = fopen(getenv('LOGGER_OUTPUT') ?: 'php://stderr', 'wb');
+
     $storage = new StorageClient();
     $data = $cloudevent->getData();
 
@@ -38,19 +40,21 @@ function blurOffensiveImages(CloudEvent $cloudevent): void
     $storage = new StorageClient();
 
     try {
-        $request = $annotator->safeSearchDetection($filePath);
-        $response = $request->getSafeSearchAnnotation();
+        $response = $annotator->safeSearchDetection($filePath);
 
-        // Handle missing files
-        // (This is uncommon, but can happen if race conditions occur)
-        if ($response === null) {
-            fwrite($log, 'Could not find ' . $filePath . PHP_EOL);
+        // Handle error
+        if ($response->hasError()) {
+            $code = Code::name($response->getError()->getCode());
+            $message = $response->getError()->getMessage();
+            fwrite($log, sprintf('%s: %s' . PHP_EOL, $code, $message));
             return;
         }
 
+        $annotation = $response->getSafeSearchAnnotation();
+
         $isInappropriate =
-            $response->getAdult() === Likelihood::VERY_LIKELY ||
-            $response->getViolence() === Likelihood::VERY_LIKELY;
+            $annotation->getAdult() === Likelihood::VERY_LIKELY ||
+            $annotation->getViolence() === Likelihood::VERY_LIKELY;
 
         if ($isInappropriate) {
             fwrite($log, 'Detected ' . $data['name'] . ' as inappropriate.' . PHP_EOL);

functions/imagemagick/test/DeployTest.php

@@ -20,7 +20,6 @@
 namespace Google\Cloud\Samples\Functions\ImageMagick\Test;
 
 use Google\Cloud\Storage\StorageClient;
-use Google\Cloud\Logging\LoggingClient;
 use Google\Cloud\TestUtils\CloudFunctionDeploymentTrait;
 use PHPUnit\Framework\TestCase;
 
@@ -40,31 +39,15 @@ class DeployTest extends TestCase
     use CloudFunctionDeploymentTrait;
     use TestCasesTrait;
 
-    /** @var string */
-    private static $entryPoint = 'blurOffensiveImages';
-
-    /** @var string */
-    private static $functionSignatureType = 'cloudevent';
-
-    /** @var string */
-    // The test starts by copying images from this bucket.
+    // The test uses this bucket to copy images.
     private const FIXTURE_SOURCE_BUCKET = 'cloud-devrel-public';
 
-    /** @var string */
-    // This is the bucket the deployed function monitors.
-    // The test copies image from FIXTURE_SOURCE_BUCKET to this one.
+    /**
+     * This is the bucket the deployed function monitors.
+     * The test copies image from FIXTURE_SOURCE_BUCKET to this one.
+     */
     private static $monitoredBucket;
 
-    /** @var string */
-    // The function saves any blurred images to this bucket.
-    private static $blurredBucket;
-
-    /** @var StorageClient */
-    private static $storageClient;
-
-    /** @var LoggingClient */
-    private static $loggingClient;
-
     /**
      * @dataProvider cases
      */
@@ -75,16 +58,13 @@ public function testFunction(
         $expected,
         $statusCode
     ): void {
-        // Upload target file.
-        $fixtureBucket = self::$storageClient->bucket(self::FIXTURE_SOURCE_BUCKET);
-        $object = $fixtureBucket->object($fileName);
+        // Trigger the cloud storage event by copying the image over
+        $storageClient = new StorageClient();
+        $fixtureBucket = $storageClient->bucket(self::FIXTURE_SOURCE_BUCKET);
 
+        $object = $fixtureBucket->object($fileName);
         $object->copy(self::$monitoredBucket, ['name' => $fileName]);
 
-        // Give event and log systems a head start.
-        // If log retrieval fails to find logs for our function within retry limit, increase sleep time.
-        sleep(5);
-
         $fiveMinAgo = date(\DateTime::RFC3339, strtotime('-5 minutes'));
         $this->processFunctionLogs($fiveMinAgo, function (\Iterator $logs) use ($expected, $label) {
             // Concatenate all relevant log messages.
@@ -108,20 +88,11 @@ public function testFunction(
     private static function doDeploy()
     {
         // Initialize variables
-        if (empty(self::$monitoredBucket)) {
-            self::$monitoredBucket = self::requireEnv('GOOGLE_STORAGE_BUCKET');
-        }
-        if (empty(self::$blurredBucket)) {
-            self::$blurredBucket = self::requireEnv('BLURRED_BUCKET_NAME');
-        }
-
-        if (empty(self::$storageClient)) {
-            self::$storageClient = new StorageClient();
-        }
+        self::$monitoredBucket = self::requireEnv('GOOGLE_STORAGE_BUCKET');
+        $blurredBucket = self::requireEnv('BLURRED_BUCKET_NAME');
 
         // Forward required env variables to Cloud Functions.
-        $envVars = 'GOOGLE_STORAGE_BUCKET=' . self::$monitoredBucket . ',';
-        $envVars .= 'BLURRED_BUCKET_NAME=' . self::$blurredBucket;
+        $envVars = sprintf('BLURRED_BUCKET_NAME=%s', $blurredBucket);
 
         self::$fn->deploy(
             ['--update-env-vars' => $envVars],

functions/imagemagick/test/IntegrationTest.php

@@ -33,12 +33,6 @@ class IntegrationTest extends TestCase
     use CloudFunctionLocalTestTrait;
     use TestCasesTrait;
 
-    /** @var string */
-    private static $entryPoint = 'blurOffensiveImages';
-
-    /** @var string */
-    private static $functionSignatureType = 'cloudevent';
-
     /**
       * @dataProvider cases
       * @dataProvider integrationCases

functions/imagemagick/test/TestCasesTrait.php

@@ -25,6 +25,12 @@ trait TestCasesTrait
 {
     use TestTrait;
 
+    /** @var string */
+    private static $entryPoint = 'blurOffensiveImages';
+
+    /** @var string */
+    private static $functionSignatureType = 'cloudevent';
+
     public static function getDataForFile($fileName): array
     {
         return [
@@ -90,7 +96,7 @@ public static function integrationCases(): array
                 'label' => 'Labels missing images as safe',
                 'filename' => 'does-not-exist.jpg',
                 'expected' => sprintf(
-                    'Could not find gs://%s/does-not-exist.jpg',
+                    'NOT_FOUND: Error opening file: gs://%s/does-not-exist.jpg',
                     $bucketName
                 ),
                 'statusCode' => '200'