resolve pr comments

googleapis · Aug 1, 2023 · 5b76ea0 · 5b76ea0
1 parent 4a530ca
commit 5b76ea0
Show file tree

Hide file tree

Showing 3 changed files with 108 additions and 7 deletions.
diff --git a/PubSub/src/Subscription.php b/PubSub/src/Subscription.php
@@ -528,6 +528,35 @@ public function create(array $options = [])
      *           be between 0 and 600 seconds. Defaults to 600 seconds.
      *     @type bool $enableExactlyOnceDelivery Indicates whether to enable
      *           'Exactly Once Delivery' on the subscription.
+     *     @type array $cloudStorageConfig If provided, messages will be delivered to Google Cloud Storage.
+     *     @type string $cloudStorageConfig.bucket User-provided name for the Cloud Storage bucket.
+     *           The bucket must be created by the user. The bucket name must be without
+     *           any prefix like "gs://". See the [bucket naming
+     *           requirements] (https://cloud.google.com/storage/docs/buckets#naming).
+     *     @type string $cloudStorageConfig.filenamePrefix
+     *           User-provided prefix for Cloud Storage filename. See the [object naming
+     *           requirements](https://cloud.google.com/storage/docs/objects#naming).
+     *     @type string $cloudStorageConfig.filenameSuffix
+     *           User-provided suffix for Cloud Storage filename. See the [object naming
+     *           requirements](https://cloud.google.com/storage/docs/objects#naming). Must
+     *           not end in "/".
+     *     @type array $cloudStorageConfig.textConfig If present, payloads will be written
+     *           to Cloud Storage as raw text, separated by a newline.
+     *     @type array $cloudStorageConfig.avroConfig If set, message payloads and metadata
+     *           will be written to Cloud Storage in Avro format.
+     *     @type bool $cloudStorageConfig.avroConfig.writeMetadata
+     *           When true, write the subscription name, message_id, publish_time,
+     *           attributes, and ordering_key as additional fields in the output.
+     *     @type Duration|string $cloudStorageConfig.maxDuration The maximum duration
+     *           that can elapse before a new Cloud Storage file is created.
+     *           Min 1 minute, max 10 minutes, default 5 minutes. May not exceed the
+     *           subscription's acknowledgement deadline. If a string is provided,
+     *           it should be as a duration in seconds with up to nine fractional digits,
+     *           terminated by 's', e.g "3.5s"
+     *     @type int|string $cloudStorageConfig.maxBytes The maximum bytes that can be
+     *           written to a Cloud Storage file before a new file is created.
+     *           Min 1 KB, max 10 GiB. The max_bytes limit may be exceeded in cases where
+     *           messages are larger than the limit.
      * }
      * @param array $options [optional] {
      *     Configuration options.

diff --git a/PubSub/tests/System/ManageSubscriptionsTest.php b/PubSub/tests/System/ManageSubscriptionsTest.php
@@ -53,11 +53,17 @@ public function testCreateAndListSubscriptions($client)
      */
     public function testCreateSubscriptionWithCloudStorageConfig($client)
     {
-        self::markTestSkipped('This test needs a GCS bucket');
-        $client = self::$restClient;
+        $gcsBucket = getenv('GCP_PHP_PUBSUB_TEST_CLOUD_STORAGE_BUCKET');
+        if (!$gcsBucket) {
+            $this->markTestSkipped(
+                'Must provide `GCP_PHP_PUBSUB_TEST_CLOUD_STORAGE_BUCKET` to run this test.'
+            );
+            return;
+        }
+
         $topic = self::topic($client);
         $bucket = [
-            'bucket' => 'pubsub-test1-bucket',
+            'bucket' => $gcsBucket,
             'avroConfig' => ['writeMetadata' => false],
             'maxDuration' => new Duration(150, 1e+9),
             'maxBytes' => '2000'
@@ -78,6 +84,35 @@ public function testCreateSubscriptionWithCloudStorageConfig($client)
         $this->assertSubsFound($client, $subsToCreate, $bucket);
     }
 
+    /**
+     * @dataProvider clientProvider
+     */
+    public function testUpdateSubscriptionWithCloudStorageConfig($client)
+    {
+        $gcsBucket = getenv('GCP_PHP_PUBSUB_TEST_CLOUD_STORAGE_BUCKET');
+        if (!$gcsBucket) {
+            $this->markTestSkipped(
+                'Must provide `GCP_PHP_PUBSUB_TEST_CLOUD_STORAGE_BUCKET` to run this test.'
+            );
+            return;
+        }
+
+        $topic = self::topic($client);
+        $subToCreate = uniqid(self::TESTING_PREFIX);
+        $sub = $client->subscribe($subToCreate, $topic);
+        self::$deletionQueue->add($sub);
+
+        $isSetCloudStorageConfig = isset($sub->info()['cloudStorageConfig']) ?? false;
+        $bucket = ['bucket' => $gcsBucket];
+
+        $sub->update([
+            'cloudStorageConfig' => $bucket
+        ]);
+
+        $this->assertEquals(false, $isSetCloudStorageConfig);
+        $this->assertEquals(true, $sub->info()['cloudStorageConfig'] ? true : false);
+    }
+
     /**
      * @dataProvider clientProvider
      */
@@ -429,10 +464,10 @@ public function testDetach($client)
         $this->assertTrue($sub->detached());
     }
 
-    private function assertSubsFound($class, $expectedSubs)
+    private function assertSubsFound($class, $expectedSubs, $bucket = [])
     {
         $backoff = new ExponentialBackoff(8);
-        $hasFoundSubs = $backoff->execute(function () use ($class, $expectedSubs) {
+        $hasFoundSubs = $backoff->execute(function () use ($class, $expectedSubs, $bucket) {
             $foundSubs = [];
             $subs = $class->subscriptions();
 
@@ -441,7 +476,13 @@ private function assertSubsFound($class, $expectedSubs)
                 $sName = end($nameParts);
                 foreach ($expectedSubs as $key => $expectedSub) {
                     if ($sName === $expectedSub) {
-                        $foundSubs[$key] = $sName;
+                        if ($bucket) {
+                            if (isset($sub->info()['cloudStorageConfig'])) {
+                                $foundSubs[$key] = $sName;
+                            }
+                        } else {
+                            $foundSubs[$key] = $sName;
+                        }
                     }
                 }
             }

diff --git a/PubSub/tests/Unit/SubscriptionTest.php b/PubSub/tests/Unit/SubscriptionTest.php
@@ -32,7 +32,6 @@
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
-use Google\Cloud\PubSub\V1\CloudStorageConfig;
 
 /**
  * @group pubsub
@@ -1007,6 +1006,38 @@ public function testCreateSubscriptionWithCloudStorageConfig()
         $this->assertEquals($sub['topic'], self::TOPIC);
     }
 
+    public function testUpdateSubscriptionWithCloudStorageConfig()
+    {
+        $bucket = [
+            'bucket' => 'pubsub-test-bucket',
+            'maxDuration' => new Duration(3, 1e+9)
+        ];
+        $bucketString = [
+            'name' => 'projects/project-id/subscriptions/subscription-name',
+            'cloudStorageConfig' => [
+                'bucket' => 'pubsub-test-bucket',
+                'maxDuration' => '3.1s'
+                ]
+            ];
+        $this->connection->updateSubscription(
+            Argument::containing($bucketString)
+        )->willReturn([
+            'name' => self::SUBSCRIPTION,
+            'topic' => self::TOPIC
+        ])->shouldBeCalledTimes(1);
+
+        $this->connection->getSubscription()->shouldNotBeCalled();
+
+        $this->subscription->___setProperty('connection', $this->connection->reveal());
+
+        $sub = $this->subscription->update([
+            'cloudStorageConfig' => $bucket
+        ]);
+
+        $this->assertEquals($sub['name'], self::SUBSCRIPTION);
+        $this->assertEquals($sub['topic'], self::TOPIC);
+    }
+
     // Helper method to generate the exception sent during an invalid EOD operation
     // like acknowledge or modifyAckDeadline
     private function generateEodException($metadata, $failureReason = 'EXACTLY_ONCE_ACKID_FAILURE')