From 153f33399b21e568adc78e06cafd6dcbb96b4090 Mon Sep 17 00:00:00 2001 From: jdpedrie Date: Thu, 12 Oct 2017 11:27:26 -0400 Subject: [PATCH 1/9] Update discovery document and documentation for Requester Pays --- .../ServiceDefinition/storage-v1.json | 6 +- src/Storage/StorageClient.php | 4 + tests/unit/Storage/RequesterPaysTest.php | 256 ++++++++++++++++++ 3 files changed, 263 insertions(+), 3 deletions(-) create mode 100644 tests/unit/Storage/RequesterPaysTest.php 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..caa9233b6beb 100644 --- a/src/Storage/StorageClient.php +++ b/src/Storage/StorageClient.php @@ -167,6 +167,8 @@ 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 The user project to be used for + * requester pays operations. * } * @return ItemIterator */ @@ -258,6 +260,8 @@ 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 The user project to be used for + * requester pays operations. * } * @return Bucket */ diff --git a/tests/unit/Storage/RequesterPaysTest.php b/tests/unit/Storage/RequesterPaysTest.php new file mode 100644 index 000000000000..716236dcc64c --- /dev/null +++ b/tests/unit/Storage/RequesterPaysTest.php @@ -0,0 +1,256 @@ +connection = $this->prophesize(ConnectionInterface::class); + $this->client = \Google\Cloud\Dev\stub(StorageClient::class); + } + + /** + * @dataProvider methods + */ + public function testRequesterPaysMethods($mockedMethod, callable $invoke, $res = []) + { + $this->connection->$mockedMethod(Argument::withEntry('userProject', self::USER_PROJECT)) + ->shouldBeCalled() + ->willReturn($res); + + $this->client->___setProperty('connection', $this->connection->reveal()); + + $invoke($this->client); + } + + public function methods() + { + $uploader = $this->prophesize(AbstractUploader::class); + $uploader->upload()->willReturn(['name' => self::OBJ, 'generation' => 'foo']); + + return [ + [ + 'deleteAcl', + function ($client) { + $this->bucket($client)->acl()->delete('foo'); + } + ], [ + 'getAcl', + function ($client) { + $this->bucket($client)->acl()->get(['entity' => 'foo']); + } + ], [ + 'listAcl', + function ($client) { + $this->bucket($client)->acl()->get(); + }, + ['items' => []] + ], [ + 'insertAcl', + function ($client) { + $this->bucket($client)->acl()->add('foo', 'bar'); + } + ], [ + 'patchAcl', + function ($client) { + $this->bucket($client)->acl()->update('foo', 'bar'); + } + ], [ + 'deleteBucket', + function ($client) { + $this->bucket($client)->delete(); + } + ], [ + 'insertBucket', + function ($client) { + $client->createBucket('foo', ['userProject' => self::USER_PROJECT]); + } + ], [ + 'listBuckets', + function ($client) { + $client->buckets(['userProject' => self::USER_PROJECT])->current(); + } + ], [ + 'getBucket', + function ($client) { + $this->bucket($client)->reload(); + } + ], [ + 'getBucketIamPolicy', + function ($client) { + $this->bucket($client)->iam()->policy(); + } + ], [ + 'setBucketIamPolicy', + function ($client) { + $this->bucket($client)->iam()->setPolicy([]); + } + ], [ + 'testBucketIamPermissions', + function ($client) { + $this->bucket($client)->iam()->testPermissions([]); + } + ], [ + 'patchBucket', + function ($client) { + $this->bucket($client)->update(); + } + ], [ + 'deleteAcl', + function ($client) { + $this->bucket($client)->defaultAcl()->delete('foo'); + } + ], [ + 'getAcl', + function ($client) { + $this->bucket($client)->defaultAcl()->get(['entity' => 'foo']); + } + ], [ + 'listAcl', + function ($client) { + $this->bucket($client)->defaultAcl()->get(); + }, + ['items' => []] + ], [ + 'insertAcl', + function ($client) { + $this->bucket($client)->defaultAcl()->add('foo', 'bar'); + } + ], [ + 'patchAcl', + function ($client) { + $this->bucket($client)->defaultAcl()->update('foo', 'bar'); + } + ], [ + 'deleteAcl', + function ($client) { + $this->object($client)->acl()->delete('foo'); + } + ], [ + 'getAcl', + function ($client) { + $this->object($client)->acl()->get(['entity' => 'foo']); + } + ], [ + 'listAcl', + function ($client) { + $this->object($client)->acl()->get(); + }, + ['items' => []] + ], [ + 'insertAcl', + function ($client) { + $this->object($client)->acl()->add('foo', 'bar'); + } + ], [ + 'patchAcl', + function ($client) { + $this->object($client)->acl()->update('foo', 'bar'); + } + ], [ + 'composeObject', + function ($client) { + $this->bucket($client)->compose([ + $this->object($client), + $this->object($client) + ], 'foo', [ + 'destination' => [ + 'contentType' => 'bar' + ] + ]); + }, [ + 'name' => 'foo', + 'generation' => 'bar' + ] + ], [ + 'copyObject', + function ($client) { + $this->object($client)->copy(self::BUCKET); + }, [ + 'name' => 'foo', + 'bucket' => 'foo', + 'generation' => 'foo' + ] + ], [ + 'deleteObject', + function ($client) { + $this->object($client)->delete(); + } + ], [ + 'getObject', + function ($client) { + $this->object($client)->reload(); + } + ], [ + 'insertObject', + function ($client) { + $this->bucket($client)->upload('foo', ['name' => self::OBJ]); + }, + $uploader->reveal() + ], [ + 'listObjects', + function ($client) { + $this->bucket($client)->objects()->current(); + } + ], [ + 'patchObject', + function ($client) { + $this->object($client)->update([]); + } + ], [ + 'rewriteObject', + function ($client) { + $this->object($client)->rewrite(self::BUCKET); + }, [ + 'resource' => [ + 'name' => 'foo', + 'bucket' => 'foo', + 'generation' => 'foo' + ] + ] + ] + ]; + } + + 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); + } +} From 8fa9573f4c30848808dc319bebdfeb0a4c381428 Mon Sep 17 00:00:00 2001 From: jdpedrie Date: Thu, 12 Oct 2017 11:28:45 -0400 Subject: [PATCH 2/9] Add requester pays to StorageClient methods --- src/Storage/StorageClient.php | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/Storage/StorageClient.php b/src/Storage/StorageClient.php index caa9233b6beb..61698a6e0b68 100644 --- a/src/Storage/StorageClient.php +++ b/src/Storage/StorageClient.php @@ -169,21 +169,25 @@ public function bucket($name, $requesterPays = false) * return the specified fields. * @type string $userProject The user project to be used for * requester pays operations. + * @type bool $bucketRequesterPays Whether each bucket should enable + * requester pays. Only applies if `$options.userProject` is set. + * **Defaults to** `true`. * } * @return ItemIterator */ public function buckets(array $options = []) { $resultLimit = $this->pluck('resultLimit', $options, false); + $bucketRequesterPays = $this->pluck('bucketRequesterPays', $options, false) ?: false; + + $requesterPays = (isset($options['userProject']) && $bucketRequesterPays) + ? $options['userProject'] + : false; return new ItemIterator( new PageIterator( - function (array $bucket) { - return new Bucket( - $this->connection, - $bucket['name'], - $bucket - ); + function (array $bucket) use ($requesterPays) { + return new Bucket($this->connection, $bucket['name'], $bucket, $requesterPays); }, [$this->connection, 'listBuckets'], $options + ['project' => $this->projectId], @@ -262,16 +266,26 @@ function (array $bucket) { * value to `null`. * @type string $userProject The user project to be used for * requester pays operations. + * @type bool $bucketRequesterPays Whether the new bucket instance should enable + * requester pays. Only applies if `$options.userProject` is set. + * **Defaults to** `true`. * } * @return Bucket */ public function createBucket($name, array $options = []) { + $bucketRequesterPays = $this->pluck('bucketRequesterPays', $options, false) ?: false; + + $requesterPays = (isset($options['userProject']) && $bucketRequesterPays) + ? $options['userProject'] + : false; + $response = $this->connection->insertBucket($options + ['name' => $name, 'project' => $this->projectId]); return new Bucket( $this->connection, $name, - $response + $response, + $requesterPays ); } From fcebc14f3161aa1106c3c4f447af07d3f6d5ab1a Mon Sep 17 00:00:00 2001 From: jdpedrie Date: Thu, 12 Oct 2017 12:16:33 -0400 Subject: [PATCH 3/9] Fix unit tests --- tests/unit/Storage/RequesterPaysTest.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/unit/Storage/RequesterPaysTest.php b/tests/unit/Storage/RequesterPaysTest.php index 716236dcc64c..20af3aa7c601 100644 --- a/tests/unit/Storage/RequesterPaysTest.php +++ b/tests/unit/Storage/RequesterPaysTest.php @@ -18,9 +18,9 @@ namespace Google\Cloud\Tests\Unit\Storage; use Prophecy\Argument; -use Google\Cloud\Storage\StorageClient; use Google\Cloud\Core\Upload\AbstractUploader; -use Google\Cloud\Storage\Connection\ConnectionInterface; +use Google\Cloud\Storage\StorageClient; +use Google\Cloud\Storage\Connection\Rest; /** * @group storage @@ -37,7 +37,7 @@ class RequesterPaysTest extends \PHPUnit_Framework_TestCase public function setUp() { - $this->connection = $this->prophesize(ConnectionInterface::class); + $this->connection = $this->prophesize(Rest::class); $this->client = \Google\Cloud\Dev\stub(StorageClient::class); } @@ -46,6 +46,8 @@ public function setUp() */ public function testRequesterPaysMethods($mockedMethod, callable $invoke, $res = []) { + $this->connection->projectId()->willReturn(self::USER_PROJECT); + $this->connection->$mockedMethod(Argument::withEntry('userProject', self::USER_PROJECT)) ->shouldBeCalled() ->willReturn($res); From 2f5a817e27be6b0084894695c4d2896946903a63 Mon Sep 17 00:00:00 2001 From: jdpedrie Date: Thu, 12 Oct 2017 12:43:55 -0400 Subject: [PATCH 4/9] add coverage for requester pays on notifications --- src/Storage/Bucket.php | 4 +- src/Storage/Connection/Rest.php | 6 +- tests/unit/Storage/RequesterPaysTest.php | 123 +++++++++++++++-------- 3 files changed, 90 insertions(+), 43 deletions(-) diff --git a/src/Storage/Bucket.php b/src/Storage/Bucket.php index 30e1eb6fc6dd..a1840605c630 100644 --- a/src/Storage/Bucket.php +++ b/src/Storage/Bucket.php @@ -592,7 +592,9 @@ function (array $object) { */ public function createNotification($topic, array $options = []) { - $res = $this->connection->insertNotification($options + [ + $res = $this->connection->insertNotification($options + array_filter([ + 'userProject' => $this->identity['userProject'], + ]) + [ 'bucket' => $this->identity['bucket'], '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/tests/unit/Storage/RequesterPaysTest.php b/tests/unit/Storage/RequesterPaysTest.php index 20af3aa7c601..746f922b8236 100644 --- a/tests/unit/Storage/RequesterPaysTest.php +++ b/tests/unit/Storage/RequesterPaysTest.php @@ -18,9 +18,11 @@ namespace Google\Cloud\Tests\Unit\Storage; use Prophecy\Argument; -use Google\Cloud\Core\Upload\AbstractUploader; +use Google\Cloud\Core\RequestWrapper; +use Psr\Http\Message\RequestInterface; use Google\Cloud\Storage\StorageClient; use Google\Cloud\Storage\Connection\Rest; +use Google\Cloud\Core\Upload\AbstractUploader; /** * @group storage @@ -28,16 +30,18 @@ */ class RequesterPaysTest extends \PHPUnit_Framework_TestCase { + const PROJECT = 'example_project'; const USER_PROJECT = 'foobar'; const BUCKET = 'bucket'; const OBJ = 'object'; + const NOTIFICATION = 'notification'; private $connection; private $client; public function setUp() { - $this->connection = $this->prophesize(Rest::class); + $this->connection = new Rest(['projectId' => self::PROJECT]); $this->client = \Google\Cloud\Dev\stub(StorageClient::class); } @@ -46,15 +50,20 @@ public function setUp() */ public function testRequesterPaysMethods($mockedMethod, callable $invoke, $res = []) { - $this->connection->projectId()->willReturn(self::USER_PROJECT); - - $this->connection->$mockedMethod(Argument::withEntry('userProject', self::USER_PROJECT)) - ->shouldBeCalled() - ->willReturn($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->client->___setProperty('connection', $this->connection->reveal()); + try { + $invoke($this->client); - $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']); + } } public function methods() @@ -66,125 +75,125 @@ public function methods() [ 'deleteAcl', function ($client) { - $this->bucket($client)->acl()->delete('foo'); + return $this->bucket($client)->acl()->delete('foo'); } ], [ 'getAcl', function ($client) { - $this->bucket($client)->acl()->get(['entity' => 'foo']); + return $this->bucket($client)->acl()->get(['entity' => 'foo']); } ], [ 'listAcl', function ($client) { - $this->bucket($client)->acl()->get(); + return $this->bucket($client)->acl()->get(); }, ['items' => []] ], [ 'insertAcl', function ($client) { - $this->bucket($client)->acl()->add('foo', 'bar'); + return $this->bucket($client)->acl()->add('foo', 'bar'); } ], [ 'patchAcl', function ($client) { - $this->bucket($client)->acl()->update('foo', 'bar'); + return $this->bucket($client)->acl()->update('foo', 'bar'); } ], [ 'deleteBucket', function ($client) { - $this->bucket($client)->delete(); + return $this->bucket($client)->delete(); } ], [ 'insertBucket', function ($client) { - $client->createBucket('foo', ['userProject' => self::USER_PROJECT]); + return $client->createBucket('foo', ['userProject' => self::USER_PROJECT]); } ], [ 'listBuckets', function ($client) { - $client->buckets(['userProject' => self::USER_PROJECT])->current(); + return $client->buckets(['userProject' => self::USER_PROJECT])->current(); } ], [ 'getBucket', function ($client) { - $this->bucket($client)->reload(); + return $this->bucket($client)->reload(); } ], [ 'getBucketIamPolicy', function ($client) { - $this->bucket($client)->iam()->policy(); + return $this->bucket($client)->iam()->policy(); } ], [ 'setBucketIamPolicy', function ($client) { - $this->bucket($client)->iam()->setPolicy([]); + return $this->bucket($client)->iam()->setPolicy([]); } ], [ 'testBucketIamPermissions', function ($client) { - $this->bucket($client)->iam()->testPermissions([]); + return $this->bucket($client)->iam()->testPermissions([]); } ], [ 'patchBucket', function ($client) { - $this->bucket($client)->update(); + return $this->bucket($client)->update(); } ], [ 'deleteAcl', function ($client) { - $this->bucket($client)->defaultAcl()->delete('foo'); + return $this->bucket($client)->defaultAcl()->delete('foo'); } ], [ 'getAcl', function ($client) { - $this->bucket($client)->defaultAcl()->get(['entity' => 'foo']); + return $this->bucket($client)->defaultAcl()->get(['entity' => 'foo']); } ], [ 'listAcl', function ($client) { - $this->bucket($client)->defaultAcl()->get(); + return $this->bucket($client)->defaultAcl()->get(); }, ['items' => []] ], [ 'insertAcl', function ($client) { - $this->bucket($client)->defaultAcl()->add('foo', 'bar'); + return $this->bucket($client)->defaultAcl()->add('foo', 'bar'); } ], [ 'patchAcl', function ($client) { - $this->bucket($client)->defaultAcl()->update('foo', 'bar'); + return $this->bucket($client)->defaultAcl()->update('foo', 'bar'); } ], [ 'deleteAcl', function ($client) { - $this->object($client)->acl()->delete('foo'); + return $this->object($client)->acl()->delete('foo'); } ], [ 'getAcl', function ($client) { - $this->object($client)->acl()->get(['entity' => 'foo']); + return $this->object($client)->acl()->get(['entity' => 'foo']); } ], [ 'listAcl', function ($client) { - $this->object($client)->acl()->get(); + return $this->object($client)->acl()->get(); }, ['items' => []] ], [ 'insertAcl', function ($client) { - $this->object($client)->acl()->add('foo', 'bar'); + return $this->object($client)->acl()->add('foo', 'bar'); } ], [ 'patchAcl', function ($client) { - $this->object($client)->acl()->update('foo', 'bar'); + return $this->object($client)->acl()->update('foo', 'bar'); } ], [ 'composeObject', function ($client) { - $this->bucket($client)->compose([ + return $this->bucket($client)->compose([ $this->object($client), $this->object($client) ], 'foo', [ @@ -199,7 +208,7 @@ function ($client) { ], [ 'copyObject', function ($client) { - $this->object($client)->copy(self::BUCKET); + return $this->object($client)->copy(self::BUCKET); }, [ 'name' => 'foo', 'bucket' => 'foo', @@ -208,33 +217,33 @@ function ($client) { ], [ 'deleteObject', function ($client) { - $this->object($client)->delete(); + return $this->object($client)->delete(); } ], [ 'getObject', function ($client) { - $this->object($client)->reload(); + return $this->object($client)->reload(); } ], [ 'insertObject', function ($client) { - $this->bucket($client)->upload('foo', ['name' => self::OBJ]); + return $this->bucket($client)->upload('foo', ['name' => self::OBJ]); }, $uploader->reveal() ], [ 'listObjects', function ($client) { - $this->bucket($client)->objects()->current(); + return $this->bucket($client)->objects()->current(); } ], [ 'patchObject', function ($client) { - $this->object($client)->update([]); + return $this->object($client)->update([]); } ], [ 'rewriteObject', function ($client) { - $this->object($client)->rewrite(self::BUCKET); + return $this->object($client)->rewrite(self::BUCKET); }, [ 'resource' => [ 'name' => 'foo', @@ -242,6 +251,26 @@ function ($client) { 'generation' => 'foo' ] ] + ], [ + 'getNotification', + function ($client) { + return $this->notification($client)->reload(); + } + ], [ + 'deleteNotification', + function ($client) { + return $this->notification($client)->delete(); + } + ], [ + 'insertNotification', + function ($client) { + return $this->bucket($client)->createNotification('topic'); + } + ], [ + 'listNotifications', + function ($client) { + return $this->bucket($client)->notifications()->current(); + } ] ]; } @@ -255,4 +284,18 @@ 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()); + } +} \ No newline at end of file From 65f08d83c91948efd36e510f9d356b2784f4ce11 Mon Sep 17 00:00:00 2001 From: jdpedrie Date: Thu, 12 Oct 2017 14:28:44 -0400 Subject: [PATCH 5/9] Address code review, update documentation --- src/Storage/Bucket.php | 5 +- src/Storage/StorageClient.php | 66 ++++++---- tests/unit/Storage/BucketTest.php | 1 + tests/unit/Storage/RequesterPaysTest.php | 155 ++++++++++++++++------- 4 files changed, 153 insertions(+), 74 deletions(-) diff --git a/src/Storage/Bucket.php b/src/Storage/Bucket.php index a1840605c630..1eff3c8a6282 100644 --- a/src/Storage/Bucket.php +++ b/src/Storage/Bucket.php @@ -592,10 +592,7 @@ function (array $object) { */ public function createNotification($topic, array $options = []) { - $res = $this->connection->insertNotification($options + array_filter([ - 'userProject' => $this->identity['userProject'], - ]) + [ - '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/StorageClient.php b/src/Storage/StorageClient.php index 61698a6e0b68..5992e9e8cee4 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,21 @@ 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 + * @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. **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 ]); } @@ -169,25 +169,36 @@ public function bucket($name, $requesterPays = false) * return the specified fields. * @type string $userProject The user project to be used for * requester pays operations. - * @type bool $bucketRequesterPays Whether each bucket should enable - * requester pays. Only applies if `$options.userProject` is set. - * **Defaults to** `true`. + * @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 */ public function buckets(array $options = []) { $resultLimit = $this->pluck('resultLimit', $options, false); - $bucketRequesterPays = $this->pluck('bucketRequesterPays', $options, false) ?: false; - $requesterPays = (isset($options['userProject']) && $bucketRequesterPays) + $bucketUserProject = $this->pluck('bucketUserProject', $options, false); + + $bucketUserProject = !is_null($bucketUserProject) + ? $bucketUserProject + : true; + + $userProject = (isset($options['userProject']) && $bucketUserProject) ? $options['userProject'] - : false; + : null; return new ItemIterator( new PageIterator( - function (array $bucket) use ($requesterPays) { - return new Bucket($this->connection, $bucket['name'], $bucket, $requesterPays); + function (array $bucket) use ($userProject) { + return new Bucket( + $this->connection, + $bucket['name'], + $bucket + ['requesterProjectId' => $userProject] + ); }, [$this->connection, 'listBuckets'], $options + ['project' => $this->projectId], @@ -266,26 +277,31 @@ function (array $bucket) use ($requesterPays) { * value to `null`. * @type string $userProject The user project to be used for * requester pays operations. - * @type bool $bucketRequesterPays Whether the new bucket instance should enable - * requester pays. Only applies if `$options.userProject` is set. - * **Defaults to** `true`. + * @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 = []) { - $bucketRequesterPays = $this->pluck('bucketRequesterPays', $options, false) ?: false; + $bucketUserProject = $this->pluck('bucketUserProject', $options, false); + + $bucketUserProject = !is_null($bucketUserProject) + ? $bucketUserProject + : true; - $requesterPays = (isset($options['userProject']) && $bucketRequesterPays) + $userProject = (isset($options['userProject']) && $bucketUserProject) ? $options['userProject'] - : false; + : null; $response = $this->connection->insertBucket($options + ['name' => $name, 'project' => $this->projectId]); return new Bucket( $this->connection, $name, - $response, - $requesterPays + $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 index 746f922b8236..8885eb745aa3 100644 --- a/tests/unit/Storage/RequesterPaysTest.php +++ b/tests/unit/Storage/RequesterPaysTest.php @@ -48,22 +48,14 @@ public function setUp() /** * @dataProvider methods */ - public function testRequesterPaysMethods($mockedMethod, callable $invoke, $res = []) + 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); - 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']); - } + $this->checkRequest($invoke); } public function methods() @@ -73,125 +65,101 @@ public function methods() return [ [ - 'deleteAcl', function ($client) { return $this->bucket($client)->acl()->delete('foo'); } ], [ - 'getAcl', function ($client) { return $this->bucket($client)->acl()->get(['entity' => 'foo']); } ], [ - 'listAcl', function ($client) { return $this->bucket($client)->acl()->get(); }, ['items' => []] ], [ - 'insertAcl', function ($client) { return $this->bucket($client)->acl()->add('foo', 'bar'); } ], [ - 'patchAcl', function ($client) { return $this->bucket($client)->acl()->update('foo', 'bar'); } ], [ - 'deleteBucket', function ($client) { return $this->bucket($client)->delete(); } ], [ - 'insertBucket', function ($client) { return $client->createBucket('foo', ['userProject' => self::USER_PROJECT]); } ], [ - 'listBuckets', function ($client) { return $client->buckets(['userProject' => self::USER_PROJECT])->current(); } ], [ - 'getBucket', function ($client) { return $this->bucket($client)->reload(); } ], [ - 'getBucketIamPolicy', function ($client) { return $this->bucket($client)->iam()->policy(); } ], [ - 'setBucketIamPolicy', function ($client) { return $this->bucket($client)->iam()->setPolicy([]); } ], [ - 'testBucketIamPermissions', function ($client) { return $this->bucket($client)->iam()->testPermissions([]); } ], [ - 'patchBucket', function ($client) { return $this->bucket($client)->update(); } ], [ - 'deleteAcl', function ($client) { return $this->bucket($client)->defaultAcl()->delete('foo'); } ], [ - 'getAcl', function ($client) { return $this->bucket($client)->defaultAcl()->get(['entity' => 'foo']); } ], [ - 'listAcl', function ($client) { return $this->bucket($client)->defaultAcl()->get(); }, ['items' => []] ], [ - 'insertAcl', function ($client) { return $this->bucket($client)->defaultAcl()->add('foo', 'bar'); } ], [ - 'patchAcl', function ($client) { return $this->bucket($client)->defaultAcl()->update('foo', 'bar'); } ], [ - 'deleteAcl', function ($client) { return $this->object($client)->acl()->delete('foo'); } ], [ - 'getAcl', function ($client) { return $this->object($client)->acl()->get(['entity' => 'foo']); } ], [ - 'listAcl', function ($client) { return $this->object($client)->acl()->get(); }, ['items' => []] ], [ - 'insertAcl', function ($client) { return $this->object($client)->acl()->add('foo', 'bar'); } ], [ - 'patchAcl', function ($client) { return $this->object($client)->acl()->update('foo', 'bar'); } ], [ - 'composeObject', function ($client) { return $this->bucket($client)->compose([ $this->object($client), @@ -206,7 +174,6 @@ function ($client) { 'generation' => 'bar' ] ], [ - 'copyObject', function ($client) { return $this->object($client)->copy(self::BUCKET); }, [ @@ -215,33 +182,27 @@ function ($client) { 'generation' => 'foo' ] ], [ - 'deleteObject', function ($client) { return $this->object($client)->delete(); } ], [ - 'getObject', function ($client) { return $this->object($client)->reload(); } ], [ - 'insertObject', function ($client) { return $this->bucket($client)->upload('foo', ['name' => self::OBJ]); }, $uploader->reveal() ], [ - 'listObjects', function ($client) { return $this->bucket($client)->objects()->current(); } ], [ - 'patchObject', function ($client) { return $this->object($client)->update([]); } ], [ - 'rewriteObject', function ($client) { return $this->object($client)->rewrite(self::BUCKET); }, [ @@ -252,22 +213,18 @@ function ($client) { ] ] ], [ - 'getNotification', function ($client) { return $this->notification($client)->reload(); } ], [ - 'deleteNotification', function ($client) { return $this->notification($client)->delete(); } ], [ - 'insertNotification', function ($client) { return $this->bucket($client)->createNotification('topic'); } ], [ - 'listNotifications', function ($client) { return $this->bucket($client)->notifications()->current(); } @@ -275,6 +232,114 @@ function ($client) { ]; } + 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(); + } + + 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); From 9300db6f69a8c8c9129f07220748aad3564c7877 Mon Sep 17 00:00:00 2001 From: jdpedrie Date: Thu, 12 Oct 2017 15:55:32 -0400 Subject: [PATCH 6/9] bump phpunit memory limit --- phpunit.xml.dist | 3 +++ 1 file changed, 3 insertions(+) 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 @@ + + + From 52072a58a1d547ddd2ec388b30c1f2cee4b612a6 Mon Sep 17 00:00:00 2001 From: jdpedrie Date: Fri, 13 Oct 2017 10:15:09 -0400 Subject: [PATCH 7/9] Add end to end unit test for notification user project --- tests/unit/Storage/RequesterPaysTest.php | 64 ++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/unit/Storage/RequesterPaysTest.php b/tests/unit/Storage/RequesterPaysTest.php index 8885eb745aa3..5b450dba59e3 100644 --- a/tests/unit/Storage/RequesterPaysTest.php +++ b/tests/unit/Storage/RequesterPaysTest.php @@ -327,6 +327,70 @@ public function testUserProjectListBucketsDisableUserProject() $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 { From d73c8f497f033d011d49998f4e9bede0feee6772 Mon Sep 17 00:00:00 2001 From: jdpedrie Date: Fri, 13 Oct 2017 10:17:15 -0400 Subject: [PATCH 8/9] Update userProject documentation --- src/Storage/StorageClient.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Storage/StorageClient.php b/src/Storage/StorageClient.php index 5992e9e8cee4..974e4190f84a 100644 --- a/src/Storage/StorageClient.php +++ b/src/Storage/StorageClient.php @@ -167,8 +167,8 @@ public function bucket($name, $userProject = false) * be either 'full' or 'noAcl'. * @type string $fields Selector which will cause the response to only * return the specified fields. - * @type string $userProject The user project to be used for - * requester pays operations. + * @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 @@ -275,8 +275,8 @@ function (array $bucket) use ($userProject) { * @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 The user project to be used for - * requester pays operations. + * @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 From b30debafb0edcef6637ad4599343bc72e1fb30d1 Mon Sep 17 00:00:00 2001 From: jdpedrie Date: Fri, 13 Oct 2017 18:30:58 -0400 Subject: [PATCH 9/9] Update argument documentation --- src/Storage/StorageClient.php | 5 +++-- tests/unit/Storage/RequesterPaysTest.php | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Storage/StorageClient.php b/src/Storage/StorageClient.php index 974e4190f84a..a0768447ba26 100644 --- a/src/Storage/StorageClient.php +++ b/src/Storage/StorageClient.php @@ -115,8 +115,9 @@ public function __construct(array $config = []) * * @param string $name The name of the bucket to request. * @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. **Defaults to** `false`. + * 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, $userProject = false) diff --git a/tests/unit/Storage/RequesterPaysTest.php b/tests/unit/Storage/RequesterPaysTest.php index 5b450dba59e3..fd81b892cb1a 100644 --- a/tests/unit/Storage/RequesterPaysTest.php +++ b/tests/unit/Storage/RequesterPaysTest.php @@ -427,4 +427,4 @@ 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()); } -} \ No newline at end of file +}