From 3b2680c20272bee4458cdd564b7196f61769a05f Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 19 Feb 2015 10:48:26 -0500 Subject: [PATCH] Guard against missing new entities. If a find operation has been modified via an event listener or overloaded find method we should avoid trying to marshal data that doesn't exist. Refs #5922 --- src/ORM/Marshaller.php | 8 ++-- tests/TestCase/ORM/MarshallerTest.php | 65 +++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/ORM/Marshaller.php b/src/ORM/Marshaller.php index 2e1b5ccf432..28d829bd89b 100644 --- a/src/ORM/Marshaller.php +++ b/src/ORM/Marshaller.php @@ -471,11 +471,13 @@ public function mergeMany($entities, array $data, array $options = []) return $query->orWhere($query->newExpr()->and_(array_combine($primary, $keys))); }, $this->_table->find()); - if (count($maybeExistentQuery->clause('where'))) { + if (!empty($indexed) && count($maybeExistentQuery->clause('where'))) { foreach ($maybeExistentQuery as $entity) { $key = implode(';', $entity->extract($primary)); - $output[] = $this->merge($entity, $indexed[$key], $options); - unset($indexed[$key]); + if (isset($indexed[$key])) { + $output[] = $this->merge($entity, $indexed[$key], $options); + unset($indexed[$key]); + } } } diff --git a/tests/TestCase/ORM/MarshallerTest.php b/tests/TestCase/ORM/MarshallerTest.php index 4419a480b04..cff4caab8c8 100644 --- a/tests/TestCase/ORM/MarshallerTest.php +++ b/tests/TestCase/ORM/MarshallerTest.php @@ -46,6 +46,40 @@ class ProtectedArticle extends Entity ]; } +/** + * Test stub for greedy find operations. + */ +class GreedyCommentsTable extends Table +{ + /** + * initialize hook + * + * @param $config Config data. + * @return void + */ + public function initialize(array $config) + { + $this->table('comments'); + $this->alias('Comments'); + } + + /** + * Overload find to cause issues. + * + * @param string $type Find type + * @param array $options find options + * @return object + */ + public function find($type = 'all', $options = []) + { + if (empty($options['conditions'])) { + $options['conditions'] = []; + } + $options['conditions'] = array_merge($options['conditions'], ['Comments.published' => 'Y']); + return parent::find($type, $options); + } +} + /** * Marshaller test case */ @@ -1211,6 +1245,37 @@ public function testMergeManyCompositeKey() $this->assertSame($entities[1], $result[1], 'Should retain object'); } + /** + * Test mergeMany() when the exist check returns nothing. + * + * @return void + */ + public function testMergeManyExistQueryFails() + { + $entities = [ + new Entity(['id' => 1, 'comment' => 'First post', 'user_id' => 2]), + new Entity(['id' => 2, 'comment' => 'Second post', 'user_id' => 2]) + ]; + $entities[0]->clean(); + $entities[1]->clean(); + + $data = [ + ['id' => 2, 'comment' => 'Changed 2', 'user_id' => 2], + ['id' => 1, 'comment' => 'Changed 1', 'user_id' => 1], + ['id' => 3, 'comment' => 'New 1'], + ]; + $comments = TableRegistry::get('GreedyComments', [ + 'className' => __NAMESPACE__ . '\\GreedyCommentsTable' + ]); + $marshall = new Marshaller($comments); + $result = $marshall->mergeMany($entities, $data); + + $this->assertEquals('Changed 1', $result[0]->comment); + $this->assertEquals(1, $result[0]->user_id); + $this->assertEquals('Changed 2', $result[1]->comment); + $this->assertEquals('New 1', $result[2]->comment); + } + /** * Tests merge with data types that need to be marshalled *