From 9efcad7874c96ed8607163155d6685a27a40e58b Mon Sep 17 00:00:00 2001 From: David Supplee Date: Wed, 9 Aug 2017 15:08:43 -0400 Subject: [PATCH 1/7] temporarily require grpc-1.4.1 (#624) --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index a809ab6932d1..dd6ee9cc55d8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,7 +13,7 @@ matrix: fast_finish: true before_script: - - pecl install grpc || echo 'Failed to install grpc' + - pecl install grpc-1.4.1 || echo 'Failed to install grpc' - composer install - if [[ $TRAVIS_PHP_VERSION =~ ^hhvm ]]; then composer --no-interaction --dev remove google/protobuf google/gax google/proto-client; fi - ./dev/sh/system-test-credentials From 5ea749a2cf268ab23a245e877d175483962e6d1d Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Tue, 8 Aug 2017 11:26:13 -0700 Subject: [PATCH 2/7] Add options for entity calls in DatastoreSessionHandler --- src/Datastore/DatastoreSessionHandler.php | 31 +++- .../Datastore/DatastoreSessionHandlerTest.php | 141 +++++++++++++++++- 2 files changed, 167 insertions(+), 5 deletions(-) diff --git a/src/Datastore/DatastoreSessionHandler.php b/src/Datastore/DatastoreSessionHandler.php index d1793db25ce4..25cbac30f99f 100644 --- a/src/Datastore/DatastoreSessionHandler.php +++ b/src/Datastore/DatastoreSessionHandler.php @@ -130,6 +130,9 @@ class DatastoreSessionHandler implements SessionHandlerInterface /* @var Transaction */ private $transaction; + /* @var array */ + private $options; + /** * Create a custom session handler backed by Cloud Datastore. * @@ -137,14 +140,23 @@ class DatastoreSessionHandler implements SessionHandlerInterface * @param int $gcLimit [optional] A number of entities to delete in the * garbage collection. Defaults to 0 which means it does nothing. * The value larger than 1000 will be cut down to 1000. + * @param array $options [optional] { + * Configuration Options + * + * @type array $defaultEntityOptions Default options to be passed to the + * {@see Datastore::entity} method when writing session data to Datastore. + * If not specified, defaults to []. + * } */ public function __construct( DatastoreClient $datastore, - $gcLimit = self::DEFAULT_GC_LIMIT + $gcLimit = self::DEFAULT_GC_LIMIT, + $options = [] ) { $this->datastore = $datastore; // Cut down to 1000 $this->gcLimit = min($gcLimit, 1000); + $this->options = $options; } /** @@ -204,9 +216,21 @@ public function read($id) /** * Write the session data to Cloud Datastore. + * + * @param string $id Identifier used to construct a {@see Key} for the {@see Entity} to be written. + * @param string $data The session data to write to the {@see Entity}. + * @param array $entityOptions Optional arguments that will be passed to the {@see DatastoreClient::entity} method + * @return bool */ - public function write($id, $data) + public function write($id, $data, $entityOptions = null) { + // Null default and check allows the user to override defaultEntityOptions with [] + if (is_null($entityOptions)) { + $entityOptions = isset($this->options['defaultEntityOptions']) + ? $this->options['defaultEntityOptions'] + : []; + } + try { $key = $this->datastore->key( $this->kind, @@ -218,7 +242,8 @@ public function write($id, $data) [ 'data' => $data, 't' => time() - ] + ], + $entityOptions ); $this->transaction->upsert($entity); $this->transaction->commit(); diff --git a/tests/unit/Datastore/DatastoreSessionHandlerTest.php b/tests/unit/Datastore/DatastoreSessionHandlerTest.php index 55be63a759c3..b437619e3003 100644 --- a/tests/unit/Datastore/DatastoreSessionHandlerTest.php +++ b/tests/unit/Datastore/DatastoreSessionHandlerTest.php @@ -190,7 +190,7 @@ public function testWrite() ->shouldBeCalledTimes(1) ->willReturn($key); $that = $this; - $this->datastore->entity($key, Argument::type('array')) + $this->datastore->entity($key, Argument::type('array'), Argument::type('array')) ->will(function($args) use ($that, $key, $entity) { $that->assertEquals($key, $args[0]); $that->assertEquals('sessiondata', $args[1]['data']); @@ -198,6 +198,7 @@ public function testWrite() $that->assertTrue(time() >= $args[1]['t']); // 2 seconds grace period should be enough $that->assertTrue(time() - $args[1]['t'] <= 2); + $that->assertEquals([], $args[2]); return $entity; }); $datastoreSessionHandler = new DatastoreSessionHandler( @@ -234,7 +235,7 @@ public function testWriteWithException() ->shouldBeCalledTimes(1) ->willReturn($key); $that = $this; - $this->datastore->entity($key, Argument::type('array')) + $this->datastore->entity($key, Argument::type('array'), Argument::type('array')) ->will(function($args) use ($that, $key, $entity) { $that->assertEquals($key, $args[0]); $that->assertEquals('sessiondata', $args[1]['data']); @@ -242,6 +243,7 @@ public function testWriteWithException() $that->assertTrue(time() >= $args[1]['t']); // 2 seconds grace period should be enough $that->assertTrue(time() - $args[1]['t'] <= 2); + $that->assertEquals([], $args[2]); return $entity; }); @@ -254,6 +256,141 @@ public function testWriteWithException() $this->assertEquals(false, $ret); } + public function testWriteWithEntityOptions() + { + $data = 'sessiondata'; + $key = new Key('projectid'); + $key->pathElement(self::KIND, 'sessionid'); + $entityOptions = ['excludeFromIndexes' => ['data']]; + $entity = new Entity($key, ['data' => $data]); + $this->transaction->upsert($entity) + ->shouldBeCalledTimes(1); + $this->transaction->commit() + ->shouldBeCalledTimes(1); + $this->datastore->transaction() + ->shouldBeCalledTimes(1) + ->willReturn($this->transaction->reveal()); + $this->datastore->key( + self::KIND, + 'sessionid', + ['namespaceId' => self::NAMESPACE_ID] + ) + ->shouldBeCalledTimes(1) + ->willReturn($key); + $that = $this; + $this->datastore->entity($key, Argument::type('array'), Argument::type('array')) + ->will(function($args) use ($that, $key, $entity) { + $that->assertEquals($key, $args[0]); + $that->assertEquals('sessiondata', $args[1]['data']); + $that->assertInternalType('int', $args[1]['t']); + $that->assertTrue(time() >= $args[1]['t']); + // 2 seconds grace period should be enough + $that->assertTrue(time() - $args[1]['t'] <= 2); + $that->assertEquals(['excludeFromIndexes' => ['data']], $args[2]); + return $entity; + }); + $datastoreSessionHandler = new DatastoreSessionHandler( + $this->datastore->reveal() + ); + $datastoreSessionHandler->open(self::NAMESPACE_ID, self::KIND); + $ret = $datastoreSessionHandler->write('sessionid', $data, $entityOptions); + + $this->assertEquals(true, $ret); + } + + public function testWriteWithDefaultEntityOptions() + { + $data = 'sessiondata'; + $key = new Key('projectid'); + $key->pathElement(self::KIND, 'sessionid'); + $datastoreSessionHandlerOptions = [ + 'defaultEntityOptions' => ['excludeFromIndexes' => ['data']], + ]; + $entity = new Entity($key, ['data' => $data]); + $this->transaction->upsert($entity) + ->shouldBeCalledTimes(1); + $this->transaction->commit() + ->shouldBeCalledTimes(1); + $this->datastore->transaction() + ->shouldBeCalledTimes(1) + ->willReturn($this->transaction->reveal()); + $this->datastore->key( + self::KIND, + 'sessionid', + ['namespaceId' => self::NAMESPACE_ID] + ) + ->shouldBeCalledTimes(1) + ->willReturn($key); + $that = $this; + $this->datastore->entity($key, Argument::type('array'), Argument::type('array')) + ->will(function($args) use ($that, $key, $entity) { + $that->assertEquals($key, $args[0]); + $that->assertEquals('sessiondata', $args[1]['data']); + $that->assertInternalType('int', $args[1]['t']); + $that->assertTrue(time() >= $args[1]['t']); + // 2 seconds grace period should be enough + $that->assertTrue(time() - $args[1]['t'] <= 2); + $that->assertEquals(['excludeFromIndexes' => ['data']], $args[2]); + return $entity; + }); + $datastoreSessionHandler = new DatastoreSessionHandler( + $this->datastore->reveal(), + DatastoreSessionHandler::DEFAULT_GC_LIMIT, + $datastoreSessionHandlerOptions + ); + $datastoreSessionHandler->open(self::NAMESPACE_ID, self::KIND); + $ret = $datastoreSessionHandler->write('sessionid', $data); + + $this->assertEquals(true, $ret); + } + + public function testWriteWithOverrideDefaultEntityOptions() + { + $data = 'sessiondata'; + $key = new Key('projectid'); + $key->pathElement(self::KIND, 'sessionid'); + $entityOptions = []; + $datastoreSessionHandlerOptions = [ + 'defaultEntityOptions' => ['excludeFromIndexes' => ['data']], + ]; + $entity = new Entity($key, ['data' => $data]); + $this->transaction->upsert($entity) + ->shouldBeCalledTimes(1); + $this->transaction->commit() + ->shouldBeCalledTimes(1); + $this->datastore->transaction() + ->shouldBeCalledTimes(1) + ->willReturn($this->transaction->reveal()); + $this->datastore->key( + self::KIND, + 'sessionid', + ['namespaceId' => self::NAMESPACE_ID] + ) + ->shouldBeCalledTimes(1) + ->willReturn($key); + $that = $this; + $this->datastore->entity($key, Argument::type('array'), Argument::type('array')) + ->will(function($args) use ($that, $key, $entity) { + $that->assertEquals($key, $args[0]); + $that->assertEquals('sessiondata', $args[1]['data']); + $that->assertInternalType('int', $args[1]['t']); + $that->assertTrue(time() >= $args[1]['t']); + // 2 seconds grace period should be enough + $that->assertTrue(time() - $args[1]['t'] <= 2); + $that->assertEquals([], $args[2]); + return $entity; + }); + $datastoreSessionHandler = new DatastoreSessionHandler( + $this->datastore->reveal(), + DatastoreSessionHandler::DEFAULT_GC_LIMIT, + $datastoreSessionHandlerOptions + ); + $datastoreSessionHandler->open(self::NAMESPACE_ID, self::KIND); + $ret = $datastoreSessionHandler->write('sessionid', $data, $entityOptions = []); + + $this->assertEquals(true, $ret); + } + public function testDestroy() { $key = new Key('projectid'); From 888af3d5687700f5cb40909780f746f99a062e85 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Tue, 8 Aug 2017 13:00:12 -0700 Subject: [PATCH 3/7] Update @see docs --- src/Datastore/DatastoreSessionHandler.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Datastore/DatastoreSessionHandler.php b/src/Datastore/DatastoreSessionHandler.php index 25cbac30f99f..2dde250d398c 100644 --- a/src/Datastore/DatastoreSessionHandler.php +++ b/src/Datastore/DatastoreSessionHandler.php @@ -144,7 +144,7 @@ class DatastoreSessionHandler implements SessionHandlerInterface * Configuration Options * * @type array $defaultEntityOptions Default options to be passed to the - * {@see Datastore::entity} method when writing session data to Datastore. + * {@see \Google\Cloud\Datastore\DatastoreClient::entity} method when writing session data to Datastore. * If not specified, defaults to []. * } */ @@ -217,9 +217,9 @@ public function read($id) /** * Write the session data to Cloud Datastore. * - * @param string $id Identifier used to construct a {@see Key} for the {@see Entity} to be written. - * @param string $data The session data to write to the {@see Entity}. - * @param array $entityOptions Optional arguments that will be passed to the {@see DatastoreClient::entity} method + * @param string $id Identifier used to construct a {@see \Google\Cloud\Datastore\Key} for the {@see \Google\Cloud\Datastore\Entity} to be written. + * @param string $data The session data to write to the {@see \Google\Cloud\Datastore\Entity}. + * @param array $entityOptions Optional arguments that will be passed to the {@see \Google\Cloud\Datastore\DatastoreClient::entity} method * @return bool */ public function write($id, $data, $entityOptions = null) From 7835fe6f99c00782500b2368166c4df84c788220 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Thu, 10 Aug 2017 08:45:29 -0700 Subject: [PATCH 4/7] Address PR comments --- src/Datastore/DatastoreSessionHandler.php | 27 +++++---- .../Datastore/DatastoreSessionHandlerTest.php | 57 +++---------------- 2 files changed, 20 insertions(+), 64 deletions(-) diff --git a/src/Datastore/DatastoreSessionHandler.php b/src/Datastore/DatastoreSessionHandler.php index 2dde250d398c..3722b86e3ec1 100644 --- a/src/Datastore/DatastoreSessionHandler.php +++ b/src/Datastore/DatastoreSessionHandler.php @@ -143,19 +143,26 @@ class DatastoreSessionHandler implements SessionHandlerInterface * @param array $options [optional] { * Configuration Options * - * @type array $defaultEntityOptions Default options to be passed to the - * {@see \Google\Cloud\Datastore\DatastoreClient::entity} method when writing session data to Datastore. - * If not specified, defaults to []. + * @type array $entityOptions Default options to be passed to the + * {@see \Google\Cloud\Datastore\DatastoreClient::entity()} method when writing session data to Datastore. + * If not specified, defaults to ['excludeFromIndexes' => ['data']]. * } */ public function __construct( DatastoreClient $datastore, $gcLimit = self::DEFAULT_GC_LIMIT, - $options = [] + array $options = [] ) { $this->datastore = $datastore; // Cut down to 1000 $this->gcLimit = min($gcLimit, 1000); + + if (!isset($options['entityOptions'])) { + $options['entityOptions'] = [ + 'excludeFromIndexes' => ['data'] + ]; + } + $this->options = $options; } @@ -219,18 +226,10 @@ public function read($id) * * @param string $id Identifier used to construct a {@see \Google\Cloud\Datastore\Key} for the {@see \Google\Cloud\Datastore\Entity} to be written. * @param string $data The session data to write to the {@see \Google\Cloud\Datastore\Entity}. - * @param array $entityOptions Optional arguments that will be passed to the {@see \Google\Cloud\Datastore\DatastoreClient::entity} method * @return bool */ - public function write($id, $data, $entityOptions = null) + public function write($id, $data) { - // Null default and check allows the user to override defaultEntityOptions with [] - if (is_null($entityOptions)) { - $entityOptions = isset($this->options['defaultEntityOptions']) - ? $this->options['defaultEntityOptions'] - : []; - } - try { $key = $this->datastore->key( $this->kind, @@ -243,7 +242,7 @@ public function write($id, $data, $entityOptions = null) 'data' => $data, 't' => time() ], - $entityOptions + $this->options['entityOptions'] ); $this->transaction->upsert($entity); $this->transaction->commit(); diff --git a/tests/unit/Datastore/DatastoreSessionHandlerTest.php b/tests/unit/Datastore/DatastoreSessionHandlerTest.php index b437619e3003..b46fdf386ac2 100644 --- a/tests/unit/Datastore/DatastoreSessionHandlerTest.php +++ b/tests/unit/Datastore/DatastoreSessionHandlerTest.php @@ -198,7 +198,7 @@ public function testWrite() $that->assertTrue(time() >= $args[1]['t']); // 2 seconds grace period should be enough $that->assertTrue(time() - $args[1]['t'] <= 2); - $that->assertEquals([], $args[2]); + $that->assertEquals(['excludeFromIndexes' => ['data']], $args[2]); return $entity; }); $datastoreSessionHandler = new DatastoreSessionHandler( @@ -243,7 +243,7 @@ public function testWriteWithException() $that->assertTrue(time() >= $args[1]['t']); // 2 seconds grace period should be enough $that->assertTrue(time() - $args[1]['t'] <= 2); - $that->assertEquals([], $args[2]); + $that->assertEquals(['excludeFromIndexes' => ['data']], $args[2]); return $entity; }); @@ -257,54 +257,12 @@ public function testWriteWithException() } public function testWriteWithEntityOptions() - { - $data = 'sessiondata'; - $key = new Key('projectid'); - $key->pathElement(self::KIND, 'sessionid'); - $entityOptions = ['excludeFromIndexes' => ['data']]; - $entity = new Entity($key, ['data' => $data]); - $this->transaction->upsert($entity) - ->shouldBeCalledTimes(1); - $this->transaction->commit() - ->shouldBeCalledTimes(1); - $this->datastore->transaction() - ->shouldBeCalledTimes(1) - ->willReturn($this->transaction->reveal()); - $this->datastore->key( - self::KIND, - 'sessionid', - ['namespaceId' => self::NAMESPACE_ID] - ) - ->shouldBeCalledTimes(1) - ->willReturn($key); - $that = $this; - $this->datastore->entity($key, Argument::type('array'), Argument::type('array')) - ->will(function($args) use ($that, $key, $entity) { - $that->assertEquals($key, $args[0]); - $that->assertEquals('sessiondata', $args[1]['data']); - $that->assertInternalType('int', $args[1]['t']); - $that->assertTrue(time() >= $args[1]['t']); - // 2 seconds grace period should be enough - $that->assertTrue(time() - $args[1]['t'] <= 2); - $that->assertEquals(['excludeFromIndexes' => ['data']], $args[2]); - return $entity; - }); - $datastoreSessionHandler = new DatastoreSessionHandler( - $this->datastore->reveal() - ); - $datastoreSessionHandler->open(self::NAMESPACE_ID, self::KIND); - $ret = $datastoreSessionHandler->write('sessionid', $data, $entityOptions); - - $this->assertEquals(true, $ret); - } - - public function testWriteWithDefaultEntityOptions() { $data = 'sessiondata'; $key = new Key('projectid'); $key->pathElement(self::KIND, 'sessionid'); $datastoreSessionHandlerOptions = [ - 'defaultEntityOptions' => ['excludeFromIndexes' => ['data']], + 'entityOptions' => ['excludeFromIndexes' => ['data', 'additional']], ]; $entity = new Entity($key, ['data' => $data]); $this->transaction->upsert($entity) @@ -330,7 +288,7 @@ public function testWriteWithDefaultEntityOptions() $that->assertTrue(time() >= $args[1]['t']); // 2 seconds grace period should be enough $that->assertTrue(time() - $args[1]['t'] <= 2); - $that->assertEquals(['excludeFromIndexes' => ['data']], $args[2]); + $that->assertEquals(['excludeFromIndexes' => ['data', 'additional']], $args[2]); return $entity; }); $datastoreSessionHandler = new DatastoreSessionHandler( @@ -344,14 +302,13 @@ public function testWriteWithDefaultEntityOptions() $this->assertEquals(true, $ret); } - public function testWriteWithOverrideDefaultEntityOptions() + public function testWriteWithEmptyEntityOptions() { $data = 'sessiondata'; $key = new Key('projectid'); $key->pathElement(self::KIND, 'sessionid'); - $entityOptions = []; $datastoreSessionHandlerOptions = [ - 'defaultEntityOptions' => ['excludeFromIndexes' => ['data']], + 'entityOptions' => [], ]; $entity = new Entity($key, ['data' => $data]); $this->transaction->upsert($entity) @@ -386,7 +343,7 @@ public function testWriteWithOverrideDefaultEntityOptions() $datastoreSessionHandlerOptions ); $datastoreSessionHandler->open(self::NAMESPACE_ID, self::KIND); - $ret = $datastoreSessionHandler->write('sessionid', $data, $entityOptions = []); + $ret = $datastoreSessionHandler->write('sessionid', $data); $this->assertEquals(true, $ret); } From d598641d35d9cc1aa468a90fcd4286f278d72fd7 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Thu, 10 Aug 2017 08:56:36 -0700 Subject: [PATCH 5/7] Fix line length --- src/Datastore/DatastoreSessionHandler.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Datastore/DatastoreSessionHandler.php b/src/Datastore/DatastoreSessionHandler.php index 3722b86e3ec1..0ac739741917 100644 --- a/src/Datastore/DatastoreSessionHandler.php +++ b/src/Datastore/DatastoreSessionHandler.php @@ -224,7 +224,8 @@ public function read($id) /** * Write the session data to Cloud Datastore. * - * @param string $id Identifier used to construct a {@see \Google\Cloud\Datastore\Key} for the {@see \Google\Cloud\Datastore\Entity} to be written. + * @param string $id Identifier used to construct a {@see \Google\Cloud\Datastore\Key} + * for the {@see \Google\Cloud\Datastore\Entity} to be written. * @param string $data The session data to write to the {@see \Google\Cloud\Datastore\Entity}. * @return bool */ From e46797f5633ad6ac9fc90565b57bd2ccb05e350c Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Fri, 11 Aug 2017 12:35:31 -0700 Subject: [PATCH 6/7] Address PR comments --- src/Datastore/DatastoreSessionHandler.php | 9 ++++++- .../Datastore/DatastoreSessionHandlerTest.php | 25 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/Datastore/DatastoreSessionHandler.php b/src/Datastore/DatastoreSessionHandler.php index 0ac739741917..5e4845c03b37 100644 --- a/src/Datastore/DatastoreSessionHandler.php +++ b/src/Datastore/DatastoreSessionHandler.php @@ -145,7 +145,7 @@ class DatastoreSessionHandler implements SessionHandlerInterface * * @type array $entityOptions Default options to be passed to the * {@see \Google\Cloud\Datastore\DatastoreClient::entity()} method when writing session data to Datastore. - * If not specified, defaults to ['excludeFromIndexes' => ['data']]. + * If not specified, defaults to `['excludeFromIndexes' => ['data']]`. * } */ public function __construct( @@ -162,6 +162,13 @@ public function __construct( 'excludeFromIndexes' => ['data'] ]; } + if (!is_array($options['entityOptions'])) { + throw new InvalidArgumentException( + 'Optional argument `entityOptions` must be an array, got ' . + (is_object($options['entityOptions']) + ? get_class($options['entityOptions']) + : gettype($options['entityOptions']))); + } $this->options = $options; } diff --git a/tests/unit/Datastore/DatastoreSessionHandlerTest.php b/tests/unit/Datastore/DatastoreSessionHandlerTest.php index b46fdf386ac2..fa994a39e66c 100644 --- a/tests/unit/Datastore/DatastoreSessionHandlerTest.php +++ b/tests/unit/Datastore/DatastoreSessionHandlerTest.php @@ -348,6 +348,31 @@ public function testWriteWithEmptyEntityOptions() $this->assertEquals(true, $ret); } + /** + * @dataProvider invalidEntityOptions + * @expectedException InvalidArgumentException + */ + public function testInvalidEntityOptions($datastoreSessionHandlerOptions) + { + new DatastoreSessionHandler( + $this->datastore->reveal(), + DatastoreSessionHandler::DEFAULT_GC_LIMIT, + $datastoreSessionHandlerOptions + ); + } + + public function invalidEntityOptions() + { + return [ + [ + ['entityOptions' => 1] + ], + [ + ['entityOptions' => new \stdClass()] + ], + ]; + } + public function testDestroy() { $key = new Key('projectid'); From 56f6ee5f0a9ff1648e6149e565f1d42238601229 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Mon, 14 Aug 2017 08:17:20 -0700 Subject: [PATCH 7/7] Fix format violation --- src/Datastore/DatastoreSessionHandler.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Datastore/DatastoreSessionHandler.php b/src/Datastore/DatastoreSessionHandler.php index 5e4845c03b37..423845921f6a 100644 --- a/src/Datastore/DatastoreSessionHandler.php +++ b/src/Datastore/DatastoreSessionHandler.php @@ -167,7 +167,8 @@ public function __construct( 'Optional argument `entityOptions` must be an array, got ' . (is_object($options['entityOptions']) ? get_class($options['entityOptions']) - : gettype($options['entityOptions']))); + : gettype($options['entityOptions'])) + ); } $this->options = $options;