From 47e583924e12216db1357e3426530e2bcd3a57e5 Mon Sep 17 00:00:00 2001 From: John Pedrie Date: Mon, 16 Oct 2017 08:48:25 -0400 Subject: [PATCH] Update discovery document and documentation for Requester Pays (#694) * Update discovery document and documentation for Requester Pays * Add requester pays to StorageClient methods * Fix unit tests * add coverage for requester pays on notifications * Address code review, update documentation * bump phpunit memory limit * Add end to end unit test for notification user project * Update userProject documentation * Update argument documentation --- phpunit.xml.dist | 3 + src/Storage/Bucket.php | 3 +- src/Storage/Connection/Rest.php | 6 +- .../ServiceDefinition/storage-v1.json | 6 +- src/Storage/StorageClient.php | 63 ++- tests/unit/Storage/BucketTest.php | 1 + tests/unit/Storage/RequesterPaysTest.php | 430 ++++++++++++++++++ 7 files changed, 491 insertions(+), 21 deletions(-) create mode 100644 tests/unit/Storage/RequesterPaysTest.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 61d8c07e5fc1..6b8f7126ba61 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -15,4 +15,7 @@ + + + diff --git a/src/Storage/Bucket.php b/src/Storage/Bucket.php index 30e1eb6fc6dd..1eff3c8a6282 100644 --- a/src/Storage/Bucket.php +++ b/src/Storage/Bucket.php @@ -592,8 +592,7 @@ function (array $object) { */ public function createNotification($topic, array $options = []) { - $res = $this->connection->insertNotification($options + [ - 'bucket' => $this->identity['bucket'], + $res = $this->connection->insertNotification($options + $this->identity + [ 'topic' => $this->getFormattedTopic($topic), 'payload_format' => 'JSON_API_V1' ]); diff --git a/src/Storage/Connection/Rest.php b/src/Storage/Connection/Rest.php index 946bf4bcbe67..19a65b756f73 100644 --- a/src/Storage/Connection/Rest.php +++ b/src/Storage/Connection/Rest.php @@ -265,7 +265,8 @@ public function insertObject(array $args = []) 'bucket' => $args['bucket'], 'query' => [ 'predefinedAcl' => $args['predefinedAcl'], - 'uploadType' => $uploadType + 'uploadType' => $uploadType, + 'userProject' => $args['userProject'] ] ]; @@ -289,7 +290,8 @@ private function resolveUploadOptions(array $args) 'resumable' => null, 'streamable' => null, 'predefinedAcl' => null, - 'metadata' => [] + 'metadata' => [], + 'userProject' => null, ]; $args['data'] = Psr7\stream_for($args['data']); diff --git a/src/Storage/Connection/ServiceDefinition/storage-v1.json b/src/Storage/Connection/ServiceDefinition/storage-v1.json index e6405663b7f8..83efd1958e62 100644 --- a/src/Storage/Connection/ServiceDefinition/storage-v1.json +++ b/src/Storage/Connection/ServiceDefinition/storage-v1.json @@ -1,11 +1,11 @@ { "kind": "discovery#restDescription", - "etag": "\"YWOzh2SDasdU84ArJnpYek-OMdg/aAU6-GJtzQTwC546w_DsCPIRIUA\"", + "etag": "\"YWOzh2SDasdU84ArJnpYek-OMdg/rE26AVrnFbD9orx-YtVO_pKNglE\"", "discoveryVersion": "v1", "id": "storage:v1", "name": "storage", "version": "v1", - "revision": "20170915", + "revision": "20170920", "title": "Cloud Storage JSON API", "description": "Stores and retrieves potentially large, immutable data objects.", "ownerDomain": "google.com", @@ -3708,4 +3708,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Storage/StorageClient.php b/src/Storage/StorageClient.php index 437b6f81e411..a0768447ba26 100644 --- a/src/Storage/StorageClient.php +++ b/src/Storage/StorageClient.php @@ -102,9 +102,9 @@ public function __construct(array $config = []) * point. To see the operations that can be performed on a bucket please * see {@see Google\Cloud\Storage\Bucket}. * - * If `$requesterPays` is set to true, the current project ID (used to + * If `$userProject` is set to true, the current project ID (used to * instantiate the client) will be billed for all requests. If - * `$requesterPays` is a project ID, given as a string, that project + * `$userProject` is a project ID, given as a string, that project * will be billed for all requests. This only has an effect when the bucket * is not owned by the current or given project ID. * @@ -114,21 +114,22 @@ public function __construct(array $config = []) * ``` * * @param string $name The name of the bucket to request. - * @param string|bool $requesterPays If true, the current Project ID - * will be used. If a string, that string will be used as the userProject - * argument. **Defaults to** `false`. + * @param string|bool $userProject If true, the current Project ID + * will be used. If a string, that string will be used as the + * userProject argument, and that project will be billed for the + * request. **Defaults to** `false`. * @return Bucket */ - public function bucket($name, $requesterPays = false) + public function bucket($name, $userProject = false) { - if (!$requesterPays) { - $requesterPays = null; - } elseif (!is_string($requesterPays)) { - $requesterPays = $this->projectId; + if (!$userProject) { + $userProject = null; + } elseif (!is_string($userProject)) { + $userProject = $this->projectId; } return new Bucket($this->connection, $name, [ - 'requesterProjectId' => $requesterPays + 'requesterProjectId' => $userProject ]); } @@ -167,6 +168,13 @@ public function bucket($name, $requesterPays = false) * be either 'full' or 'noAcl'. * @type string $fields Selector which will cause the response to only * return the specified fields. + * @type string $userProject If set, this is the ID of the project which + * will be billed for the request. + * @type bool $bucketUserProject If true, each returned instance will + * have `$userProject` set to the value of `$options.userProject`. + * If false, `$options.userProject` will be used ONLY for the + * listBuckets operation. If `$options.userProject` is not set, + * this option has no effect. **Defaults to** `true`. * } * @return ItemIterator */ @@ -174,13 +182,23 @@ public function buckets(array $options = []) { $resultLimit = $this->pluck('resultLimit', $options, false); + $bucketUserProject = $this->pluck('bucketUserProject', $options, false); + + $bucketUserProject = !is_null($bucketUserProject) + ? $bucketUserProject + : true; + + $userProject = (isset($options['userProject']) && $bucketUserProject) + ? $options['userProject'] + : null; + return new ItemIterator( new PageIterator( - function (array $bucket) { + function (array $bucket) use ($userProject) { return new Bucket( $this->connection, $bucket['name'], - $bucket + $bucket + ['requesterProjectId' => $userProject] ); }, [$this->connection, 'listBuckets'], @@ -258,16 +276,33 @@ function (array $bucket) { * @type array $labels The Bucket labels. Labels are represented as an * array of keys and values. To remove an existing label, set its * value to `null`. + * @type string $userProject If set, this is the ID of the project which + * will be billed for the request. + * @type bool $bucketUserProject If true, the returned instance will + * have `$userProject` set to the value of `$options.userProject`. + * If false, `$options.userProject` will be used ONLY for the + * createBucket operation. If `$options.userProject` is not set, + * this option has no effect. **Defaults to** `true`. * } * @return Bucket */ public function createBucket($name, array $options = []) { + $bucketUserProject = $this->pluck('bucketUserProject', $options, false); + + $bucketUserProject = !is_null($bucketUserProject) + ? $bucketUserProject + : true; + + $userProject = (isset($options['userProject']) && $bucketUserProject) + ? $options['userProject'] + : null; + $response = $this->connection->insertBucket($options + ['name' => $name, 'project' => $this->projectId]); return new Bucket( $this->connection, $name, - $response + $response + ['requesterProjectId' => $userProject] ); } diff --git a/tests/unit/Storage/BucketTest.php b/tests/unit/Storage/BucketTest.php index 6ffba09cf23a..ffe8237b315f 100644 --- a/tests/unit/Storage/BucketTest.php +++ b/tests/unit/Storage/BucketTest.php @@ -400,6 +400,7 @@ public function testCreatesNotification($topic, $expectedTopic) { $this->connection ->insertNotification([ + 'userProject' => null, 'bucket' => self::BUCKET_NAME, 'topic' => sprintf('//pubsub.googleapis.com/projects/%s/topics/%s', self::PROJECT_ID, $expectedTopic), 'payload_format' => 'JSON_API_V1' diff --git a/tests/unit/Storage/RequesterPaysTest.php b/tests/unit/Storage/RequesterPaysTest.php new file mode 100644 index 000000000000..fd81b892cb1a --- /dev/null +++ b/tests/unit/Storage/RequesterPaysTest.php @@ -0,0 +1,430 @@ +connection = new Rest(['projectId' => self::PROJECT]); + $this->client = \Google\Cloud\Dev\stub(StorageClient::class); + } + + /** + * @dataProvider methods + */ + public function testRequesterPaysMethods(callable $invoke, $res = []) + { + // we're using a real connection instance, but the request handler is stubbed out + // to throw the request query string back to us. + $this->connection->setRequestWrapper(new RequestWrapperStub); + $this->client->___setProperty('connection', $this->connection); + + $this->checkRequest($invoke); + } + + public function methods() + { + $uploader = $this->prophesize(AbstractUploader::class); + $uploader->upload()->willReturn(['name' => self::OBJ, 'generation' => 'foo']); + + return [ + [ + function ($client) { + return $this->bucket($client)->acl()->delete('foo'); + } + ], [ + function ($client) { + return $this->bucket($client)->acl()->get(['entity' => 'foo']); + } + ], [ + function ($client) { + return $this->bucket($client)->acl()->get(); + }, + ['items' => []] + ], [ + function ($client) { + return $this->bucket($client)->acl()->add('foo', 'bar'); + } + ], [ + function ($client) { + return $this->bucket($client)->acl()->update('foo', 'bar'); + } + ], [ + function ($client) { + return $this->bucket($client)->delete(); + } + ], [ + function ($client) { + return $client->createBucket('foo', ['userProject' => self::USER_PROJECT]); + } + ], [ + function ($client) { + return $client->buckets(['userProject' => self::USER_PROJECT])->current(); + } + ], [ + function ($client) { + return $this->bucket($client)->reload(); + } + ], [ + function ($client) { + return $this->bucket($client)->iam()->policy(); + } + ], [ + function ($client) { + return $this->bucket($client)->iam()->setPolicy([]); + } + ], [ + function ($client) { + return $this->bucket($client)->iam()->testPermissions([]); + } + ], [ + function ($client) { + return $this->bucket($client)->update(); + } + ], [ + function ($client) { + return $this->bucket($client)->defaultAcl()->delete('foo'); + } + ], [ + function ($client) { + return $this->bucket($client)->defaultAcl()->get(['entity' => 'foo']); + } + ], [ + function ($client) { + return $this->bucket($client)->defaultAcl()->get(); + }, + ['items' => []] + ], [ + function ($client) { + return $this->bucket($client)->defaultAcl()->add('foo', 'bar'); + } + ], [ + function ($client) { + return $this->bucket($client)->defaultAcl()->update('foo', 'bar'); + } + ], [ + function ($client) { + return $this->object($client)->acl()->delete('foo'); + } + ], [ + function ($client) { + return $this->object($client)->acl()->get(['entity' => 'foo']); + } + ], [ + function ($client) { + return $this->object($client)->acl()->get(); + }, + ['items' => []] + ], [ + function ($client) { + return $this->object($client)->acl()->add('foo', 'bar'); + } + ], [ + function ($client) { + return $this->object($client)->acl()->update('foo', 'bar'); + } + ], [ + function ($client) { + return $this->bucket($client)->compose([ + $this->object($client), + $this->object($client) + ], 'foo', [ + 'destination' => [ + 'contentType' => 'bar' + ] + ]); + }, [ + 'name' => 'foo', + 'generation' => 'bar' + ] + ], [ + function ($client) { + return $this->object($client)->copy(self::BUCKET); + }, [ + 'name' => 'foo', + 'bucket' => 'foo', + 'generation' => 'foo' + ] + ], [ + function ($client) { + return $this->object($client)->delete(); + } + ], [ + function ($client) { + return $this->object($client)->reload(); + } + ], [ + function ($client) { + return $this->bucket($client)->upload('foo', ['name' => self::OBJ]); + }, + $uploader->reveal() + ], [ + function ($client) { + return $this->bucket($client)->objects()->current(); + } + ], [ + function ($client) { + return $this->object($client)->update([]); + } + ], [ + function ($client) { + return $this->object($client)->rewrite(self::BUCKET); + }, [ + 'resource' => [ + 'name' => 'foo', + 'bucket' => 'foo', + 'generation' => 'foo' + ] + ] + ], [ + function ($client) { + return $this->notification($client)->reload(); + } + ], [ + function ($client) { + return $this->notification($client)->delete(); + } + ], [ + function ($client) { + return $this->bucket($client)->createNotification('topic'); + } + ], [ + function ($client) { + return $this->bucket($client)->notifications()->current(); + } + ] + ]; + } + + public function testUserProjectCreateBucket() + { + $connection = $this->prophesize(Rest::class); + $connection->projectId()->willReturn(self::PROJECT); + $connection->insertBucket(Argument::any()) + ->shouldBeCalled() + ->willReturn([]); + $connection->listObjects(['bucket' => 'foo', 'userProject' => self::USER_PROJECT]) + ->shouldBeCalled() + ->willReturn([ + 'objects' => [] + ]); + + $this->client->___setProperty('connection', $connection->reveal()); + + $bucket = $this->client->createBucket('foo', [ + 'userProject' => self::USER_PROJECT + ]); + + $bucket->objects()->current(); + } + + public function testUserProjectCreateBucketDisableUserProject() + { + $connection = $this->prophesize(Rest::class); + $connection->projectId()->willReturn(self::PROJECT); + $connection->insertBucket(Argument::any()) + ->shouldBeCalled() + ->willReturn([]); + $connection->listObjects(['bucket' => 'foo', 'userProject' => null]) + ->shouldBeCalled() + ->willReturn([ + 'objects' => [] + ]); + + $this->client->___setProperty('connection', $connection->reveal()); + + $bucket = $this->client->createBucket('foo', [ + 'userProject' => self::USER_PROJECT, + 'bucketUserProject' => false + ]); + + $bucket->objects()->current(); + } + + public function testUserProjectListBuckets() + { + $connection = $this->prophesize(Rest::class); + $connection->projectId()->willReturn(self::PROJECT); + $connection->listBuckets(Argument::any()) + ->shouldBeCalled() + ->willReturn([ + 'items' => [ + [ + 'name' => 'foo' + ] + ] + ]); + + $connection->listObjects(['bucket' => 'foo', 'userProject' => self::USER_PROJECT]) + ->shouldBeCalled() + ->willReturn([ + 'objects' => [] + ]); + + $this->client->___setProperty('connection', $connection->reveal()); + $bucket = $this->client->buckets(['userProject' => self::USER_PROJECT])->current(); + $bucket->objects()->current(); + } + + public function testUserProjectListBucketsDisableUserProject() + { + $connection = $this->prophesize(Rest::class); + $connection->projectId()->willReturn(self::PROJECT); + $connection->listBuckets(Argument::any()) + ->shouldBeCalled() + ->willReturn([ + 'items' => [ + [ + 'name' => 'foo' + ] + ] + ]); + + $connection->listObjects(['bucket' => 'foo', 'userProject' => null]) + ->shouldBeCalled() + ->willReturn([ + 'objects' => [] + ]); + + $this->client->___setProperty('connection', $connection->reveal()); + $bucket = $this->client->buckets(['userProject' => self::USER_PROJECT, 'bucketUserProject' => false])->current(); + $bucket->objects()->current(); + } + + public function testUserProjectCreateNotification() + { + $connection = $this->prophesize(Rest::class); + $connection->projectId()->willReturn(self::PROJECT); + + $connection->insertNotification(Argument::withEntry('userProject', self::USER_PROJECT)) + ->shouldBeCalled() + ->willReturn([ + 'id' => 'foo' + ]); + + $connection->getNotification(Argument::withEntry('userProject', self::USER_PROJECT)) + ->shouldBeCalled(); + + $this->client->___setProperty('connection', $connection->reveal()); + + $bucket = $this->client->bucket('foo', self::USER_PROJECT); + + $notification = $bucket->createNotification('foo'); + $notification->reload(); + } + + public function testUserProjectNotification() + { + $connection = $this->prophesize(Rest::class); + $connection->projectId()->willReturn(self::PROJECT); + + $connection->getNotification(Argument::withEntry('userProject', self::USER_PROJECT)) + ->shouldBeCalled(); + + $this->client->___setProperty('connection', $connection->reveal()); + + $bucket = $this->client->bucket('foo', self::USER_PROJECT); + + $notification = $bucket->notification('foo'); + $notification->reload(); + } + + public function testUserProjectListNotifications() + { + $connection = $this->prophesize(Rest::class); + $connection->projectId()->willReturn(self::PROJECT); + + $connection->listNotifications(Argument::withEntry('userProject', self::USER_PROJECT)) + ->shouldBeCalled() + ->willReturn([ + 'items' => [ + [ + 'id' => 'foo' + ] + ] + ]); + + $connection->getNotification(Argument::withEntry('userProject', self::USER_PROJECT)) + ->shouldBeCalled(); + + $this->client->___setProperty('connection', $connection->reveal()); + + $bucket = $this->client->bucket('foo', self::USER_PROJECT); + + $notification = $bucket->notifications()->current(); + $notification->reload(); + } + + private function checkRequest(callable $invoke) + { + try { + $invoke($this->client); + + // if no exception, something is wrong. + $this->assertTrue(false); + } catch(\Exception $e) { + parse_str($e->getMessage(), $query); + $this->assertEquals(self::USER_PROJECT, $query['userProject']); + } + } + + private function bucket(StorageClient $client) + { + return $client->bucket(self::BUCKET, self::USER_PROJECT); + } + + private function object(StorageClient $client) + { + return $this->bucket($client)->object(self::OBJ); + } + + private function notification(StorageClient $client) + { + return $this->bucket($client)->notification(self::NOTIFICATION); + } +} + +class RequestWrapperStub extends RequestWrapper +{ + public function send(RequestInterface $request, array $options = []) + { + // to short circuit all the response handling and get back to the assertion with the query string. + throw new \Exception($request->getUri()->getQuery()); + } +}