doctrine / KeyValueStore

Abstraction for Key-Value to Plain Old PHP Object mapping
http://www.doctrine-project.org
MIT License
200 stars 59 forks source link

Fix DynamoDbStorage::find #87

Closed foaly-nr1 closed 4 years ago

foaly-nr1 commented 7 years ago

Duplicate of (abandoned) https://github.com/doctrine/KeyValueStore/pull/84.

As per method signature:

/**
 * @method \Aws\Result getItem(array $args = [])
 */
foaly-nr1 commented 7 years ago

Broken build is definitely unrelated to my PR:

There were 4 errors:
1) Doctrine\Tests\KeyValueStore\Functional\Storage\CassandraTest::testInsert
Function Cassandra\ExecutionOptions::__construct() is deprecated
/home/travis/build/doctrine/KeyValueStore/lib/Doctrine/KeyValueStore/Storage/CassandraStorage.php:91
/home/travis/build/doctrine/KeyValueStore/tests/Doctrine/Tests/KeyValueStore/Functional/Storage/CassandraTest.php:64
2) Doctrine\Tests\KeyValueStore\Functional\Storage\CassandraTest::testFind
Function Cassandra\ExecutionOptions::__construct() is deprecated
/home/travis/build/doctrine/KeyValueStore/lib/Doctrine/KeyValueStore/Storage/CassandraStorage.php:91
/home/travis/build/doctrine/KeyValueStore/tests/Doctrine/Tests/KeyValueStore/Functional/Storage/CassandraTest.php:82
3) Doctrine\Tests\KeyValueStore\Functional\Storage\CassandraTest::testUpdate
Function Cassandra\ExecutionOptions::__construct() is deprecated
/home/travis/build/doctrine/KeyValueStore/lib/Doctrine/KeyValueStore/Storage/CassandraStorage.php:91
/home/travis/build/doctrine/KeyValueStore/tests/Doctrine/Tests/KeyValueStore/Functional/Storage/CassandraTest.php:94
4) Doctrine\Tests\KeyValueStore\Functional\Storage\CassandraTest::testDelete
Function Cassandra\ExecutionOptions::__construct() is deprecated
/home/travis/build/doctrine/KeyValueStore/lib/Doctrine/KeyValueStore/Storage/CassandraStorage.php:91
/home/travis/build/doctrine/KeyValueStore/tests/Doctrine/Tests/KeyValueStore/Functional/Storage/CassandraTest.php:110
jarenal commented 7 years ago

Hi @foaly-nr1 Could you tell me if your PR fix this issue, I think that I have the same problem. When I execute a find() query like this and the record doesn't exist:

$results = $entityManager->find('FooApp\Entity\DynamoDB\Click', array('click_id'=>'123'));

I get the next message:

Fatal error: Uncaught TypeError: Argument 1 passed to Aws\DynamoDb\Marshaler::unmarshalItem() must be of the type array, null given

Thanks in advance!

foaly-nr1 commented 7 years ago

I'm not sure @jarenal, but you can easily test it. Just change your composer.json to use the branch from this PR.

{
    "repositories": [
      {
        "type": "vcs",
        "url": "https://github.com/foaly-nr1/KeyValueStore"
      }
    ],
    "minimum-stability": "dev",
    "prefer-stable": true,
    "require": {
        "doctrine/key-value-store": "dev-dynamo-find"
    },
    "require-dev": {},
}
jarenal commented 7 years ago

Thanks for your quick response.

Now it works but throws me an exception with this message:

"Could not find an item with key: 123"

I think that it's a wrong approach, it should return null, or an empty array or an empty object. When I use Doctrine + MySQL if there are not records no throw exceptions.

Regards!

foaly-nr1 commented 7 years ago

Regardless of whether you think that's the right approach, that's the expected behaviour. Just catch the exception:

try {
    $click = $entityManager->find(Click::class, array('click_id'=>'123'));
} catch (NotFoundException $e) {
    $click = new Click();
}
jarenal commented 7 years ago

Hi @foaly-nr1

It's a curious approach, never I could imagine that an empty result from a database will be managed as an exception. Every day I learn something new.

PS: Thanks for the example.

Regards.