Skip to content

Commit

Permalink
Correctly marshall _joinData when it is not accessible
Browse files Browse the repository at this point in the history 
When _joinData is not an accessible field it should still be marshaled
into an entity, or have the existing entity patched correctly. The
previous code was incorrectly marshalling the _joinData property twice,
and the removed array cast was masking the source of the issue.

Also ensure that scalar _joinData properties are not marshaled as they
are always invalid.

Refs #5542
  • Loading branch information
@markstory
markstory committed Feb 18, 2015
1 parent d26d198 commit a01fe7d
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 5 deletions.
29 changes: 26 additions & 3 deletions src/ORM/Marshaller.php
View file
Expand Up @@ -451,7 +451,6 @@ public function mergeMany($entities, array $data, array $options = [])
}

$key = implode(';', $entity->extract($primary));

if ($key === null || !isset($indexed[$key])) {
continue;
}
Expand Down Expand Up @@ -521,7 +520,7 @@ protected function _mergeAssociation($original, $assoc, $value, $options)
* @param \Cake\ORM\Association $assoc The association to marshall
* @param array $value The data to hydrate
* @param array $options List of options.
* @return mixed
* @return array
*/
protected function _mergeBelongsToMany($original, $assoc, $value, $options)
{
Expand All @@ -538,8 +537,26 @@ protected function _mergeBelongsToMany($original, $assoc, $value, $options)
return $this->mergeMany($original, $value, $options);
}

return $this->_mergeJoinData($original, $assoc, $value, $options);
}

/**
* Merge the special _joinData property into the entity set.
*
* @param \Cake\Datasource\EntityInterface $original The original entity
* @param \Cake\ORM\Association $assoc The association to marshall
* @param array $value The data to hydrate
* @param array $options List of options.
* @return array An array of entities
*/
protected function _mergeJoinData($original, $assoc, $value, $options)
{
$associated = isset($options['associated']) ? $options['associated'] : [];
$extra = [];
foreach ($original as $entity) {
// Mark joinData as accessible so we can marshal it properly.
$entity->accessible('_joinData', true);

$joinData = $entity->get('_joinData');
if ($joinData && $joinData instanceof EntityInterface) {
$extra[spl_object_hash($entity)] = $joinData;
Expand All @@ -558,8 +575,14 @@ protected function _mergeBelongsToMany($original, $assoc, $value, $options)
foreach ($records as $record) {
$hash = spl_object_hash($record);
$value = $record->get('_joinData');

if (!is_array($value)) {
$record->unsetProperty('_joinData');
continue;
}

if (isset($extra[$hash])) {
$record->set('_joinData', $marshaller->merge($extra[$hash], (array)$value, $nested));
$record->set('_joinData', $marshaller->merge($extra[$hash], $value, $nested));
} else {
$joinData = $marshaller->one($value, $nested);
$record->set('_joinData', $joinData);
Expand Down
74 changes: 72 additions & 2 deletions tests/TestCase/ORM/MarshallerTest.php
View file
Expand Up @@ -52,7 +52,14 @@ class ProtectedArticle extends Entity
class MarshallerTest extends TestCase
{

public $fixtures = ['core.tags', 'core.articles_tags', 'core.articles', 'core.users', 'core.comments'];
public $fixtures = [
'core.tags',
'core.articles_tags',
'core.articles',
'core.users',
'core.comments',
'core.special_tags'
];

/**
* setup
Expand Down Expand Up @@ -988,6 +995,69 @@ public function testMergeBelongsToManyEntitiesFromIdsEmptyValue()
$this->assertCount(0, $result->tags);
}

/**
* Test that invalid _joinData (scalar data) is not marshalled.
*
* @return void
*/
public function testMergeBelongsToManyJoinDataScalar()
{
TableRegistry::clear();
$articles = TableRegistry::get('Articles');
$articles->belongsToMany('Tags', [
'through' => 'SpecialTags'
]);

$entity = $articles->get(1, ['contain' => 'Tags']);
$data = [
'title' => 'Haz data',
'tags' => [
['id' => 3, 'tag' => 'Cake', '_joinData' => 'Invalid'],
]
];
$marshall = new Marshaller($articles);
$result = $marshall->merge($entity, $data, ['associated' => 'Tags._joinData']);

$articles->save($entity, ['associated' => ['Tags._joinData']]);
$this->assertFalse($entity->tags[0]->dirty('_joinData'));
$this->assertEmpty($entity->tags[0]->_joinData);
}

/**
* Test merging the _joinData entity for belongstomany associations when * is not
* accessible.
*
* @return void
*/
public function testMergeBelongsToManyJoinDataNotAccessible()
{
TableRegistry::clear();
$articles = TableRegistry::get('Articles');
$articles->belongsToMany('Tags', [
'through' => 'SpecialTags'
]);

$entity = $articles->get(1, ['contain' => 'Tags']);
$data = [
'title' => 'Haz data',
'tags' => [
['id' => 3, 'tag' => 'Cake', '_joinData' => ['highlighted' => '1', 'author_id' => '99']],
]
];
// Make only specific fields accessible, but not _joinData.
$entity->tags[0]->accessible('*', false);
$entity->tags[0]->accessible(['article_id', 'tag_id'], true);

$marshall = new Marshaller($articles);
$result = $marshall->merge($entity, $data, ['associated' => 'Tags._joinData']);

$this->assertTrue($entity->tags[0]->dirty('_joinData'));
$this->assertTrue($result->tags[0]->_joinData->dirty('author_id'), 'Field not modified');
$this->assertTrue($result->tags[0]->_joinData->dirty('highlighted'), 'Field not modified');
$this->assertSame(99, $result->tags[0]->_joinData->author_id);
$this->assertTrue($result->tags[0]->_joinData->highlighted);
}

/**
* Test merging the _joinData entity for belongstomany associations.
*
Expand Down Expand Up @@ -1017,7 +1087,7 @@ public function testMergeBelongsToManyJoinData()
],
];

$options = ['associated' => ['Tags' => ['associated' => ['_joinData']]]];
$options = ['associated' => ['Tags._joinData']];
$marshall = new Marshaller($this->articles);
$entity = $marshall->one($data, $options);
$entity->accessible('*', true);
Expand Down

0 comments on commit a01fe7d

Please sign in to comment.