From 5b76ea05f2edbc95aa9d87fcf6d33f80972ab99c Mon Sep 17 00:00:00 2001 From: Ajumal Date: Tue, 1 Aug 2023 16:50:31 +0530 Subject: [PATCH] resolve pr comments --- PubSub/src/Subscription.php | 29 ++++++++++ .../tests/System/ManageSubscriptionsTest.php | 53 ++++++++++++++++--- PubSub/tests/Unit/SubscriptionTest.php | 33 +++++++++++- 3 files changed, 108 insertions(+), 7 deletions(-) diff --git a/PubSub/src/Subscription.php b/PubSub/src/Subscription.php index d2178e1f66cd..a800fa751677 100644 --- 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 index bf3f8a421453..411626147eb1 100644 --- 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 index fc7684fcff38..01237ecf97b1 100644 --- 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')