From 1ade959c37f4ba66f102e50ae395c03b1b673ecb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Cansado=20Sol=C3=A0?= Date: Sun, 24 Jul 2016 11:53:32 +0200 Subject: [PATCH] update MarshallerTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - separate method (_hasTranslations) - delete all “pass by reference” - simplify tests and remove all mocks --- src/ORM/Marshaller.php | 46 ++- tests/TestCase/ORM/MarshallerTest.php | 535 +++++++++++--------------- 2 files changed, 262 insertions(+), 319 deletions(-) diff --git a/src/ORM/Marshaller.php b/src/ORM/Marshaller.php index 7105bca1f13..0c9d6238cb5 100644 --- a/src/ORM/Marshaller.php +++ b/src/ORM/Marshaller.php @@ -148,6 +148,9 @@ public function one(array $data, array $options = []) } $hasTranslations = $this->_hasTranslations($options); + if ($hasTranslations) { + $options = $this->_addTranslationsToFieldList($options); + } $errors = $this->_validate($data, $options, true); $properties = []; @@ -169,7 +172,10 @@ public function one(array $data, array $options = []) $converter = Type::build($columnType); $value = $converter->marshal($value); } elseif ($key === '_translations' && $hasTranslations) { - $value = $this->_mergeTranslations(null, $value, $errors, $options); + list($value, $translationsErrors) = $this->_mergeTranslations(null, $value, $options); + if (!empty($translationsErrors)) { + $errors += $translationsErrors; + } } $properties[$key] = $value; } @@ -454,18 +460,18 @@ protected function _loadBelongsToMany($assoc, $ids) } /** - * Call translation merge. Validations errors during merge will be added to `$errors` param + * Call translation merge * * @param \Cake\Datasource\EntityInterface $original The original entity * @param array $data key value list of languages with fields to be merged into the translate entity - * @param array $errors array with entity errors * @param array $options list of options - * @return array|null + * @return array */ - protected function _mergeTranslations($original, array $data, array &$errors, array $options = []) + protected function _mergeTranslations($original, array $data, array $options = []) { $result = $this->_table->mergeTranslations($original, $data, $this, $options); + $errors = []; if (is_array($result)) { if ((bool)$result[1]) { $errors['_translations'] = $result[1]; @@ -473,14 +479,12 @@ protected function _mergeTranslations($original, array $data, array &$errors, ar $result = $result[0]; } - return $result; + return [$result, $errors]; } /** * Return if table contains translate behavior or we specificate to use via `translations` options. * - * In case that $options has `fieldList` option and `_translations` field is not present inside it, it will include - * * ### Options: * * - translations: Set to false to disable translations @@ -488,14 +492,24 @@ protected function _mergeTranslations($original, array $data, array &$errors, ar * @param array $options List of options * @return bool */ - protected function _hasTranslations(array &$options = []) + protected function _hasTranslations(array $options = []) + { + return ($this->_table->behaviors()->hasMethod('mergeTranslations') && (bool)$options['translations']); + } + + /** + * Add `_translations` field to `fieldList` $options if it's not present inside + * + * @param array $options List of options + * @return array + */ + protected function _addTranslationsToFieldList(array $options = []) { - $hasTranslations = ($this->_table->behaviors()->hasMethod('mergeTranslations') && (bool)$options['translations']); - if ($hasTranslations && !empty($options['fieldList']) && !in_array('_translations', $options['fieldList'])) { + if (!empty($options['fieldList']) && !in_array('_translations', $options['fieldList'])) { array_push($options['fieldList'], '_translations'); } - return $hasTranslations; + return $options; } /** @@ -553,6 +567,9 @@ public function merge(EntityInterface $entity, array $data, array $options = []) } $hasTranslations = $this->_hasTranslations($options); + if ($hasTranslations) { + $options = $this->_addTranslationsToFieldList($options); + } $errors = $this->_validate($data + $keys, $options, $isNew); $schema = $this->_table->schema(); @@ -582,7 +599,10 @@ public function merge(EntityInterface $entity, array $data, array $options = []) continue; } } elseif ($key === '_translations' && $hasTranslations) { - $value = $this->_mergeTranslations($original, $value, $errors, $options); + list($value, $translationsErrors) = $this->_mergeTranslations($original, $value, $options); + if (!empty($translationsErrors)) { + $errors += $translationsErrors; + } } $properties[$key] = $value; diff --git a/tests/TestCase/ORM/MarshallerTest.php b/tests/TestCase/ORM/MarshallerTest.php index edc565ea4f8..7954281970b 100644 --- a/tests/TestCase/ORM/MarshallerTest.php +++ b/tests/TestCase/ORM/MarshallerTest.php @@ -3067,372 +3067,295 @@ public function testEnsurePrimaryKeyBeingReadFromTableWhenLoadingBelongsToManyRe */ public function testHasTranslations() { - // Mocks - $mockBehavior = $this->_getMockBehaviors(); - $mockBehavior->expects($this->at(1)) - ->method('hasMethod') - ->with('mergeTranslations') - ->will($this->returnValue(true)); - $mockBehavior->expects($this->at(2)) - ->method('hasMethod') - ->with('mergeTranslations') - ->will($this->returnValue(true)); - $mockBehavior->expects($this->at(3)) - ->method('hasMethod') - ->with('mergeTranslations') - ->will($this->returnValue(true)); - $mockBehavior->expects($this->at(4)) - ->method('hasMethod') - ->with('mergeTranslations') - ->will($this->returnValue(true)); - $mockBehavior->expects($this->at(5)) - ->method('hasMethod') - ->with('mergeTranslations') - ->will($this->returnValue(false)); - - $mockTable = $this->_getMockTable(['mergeTranslations'], ['behaviors' => $mockBehavior]); - - $marshall = new Marshaller($mockTable); - $method = $this->_getProtectedMethod($marshall, '_hasTranslations'); + // Translate Behavior not attached + $table = TableRegistry::get('Articles'); - $options = ['translations' => true]; - $args = [&$options]; + $marshall = new Marshaller($table); + $method = $this->_getProtectedMethod($marshall, '_hasTranslations'); + $marshallOptions = [ + 'validate' => true, + 'translations' => true + ]; - // Must return true - $this->assertTrue($method->invokeArgs($marshall, $args)); + $result = $method->invokeArgs($marshall, [$marshallOptions]); + $this->assertFalse($result, 'should return false'); + unset($marshall); - // Add _translations to fieldList - $options['translations'] = true; - $options['fieldList'] = ['title']; - $this->assertTrue($method->invokeArgs($marshall, $args)); - $this->assertContains('_translations', $options['fieldList']); + // Translate Behavior is attached + $table->behaviors()->load('Translate'); - // Call again, now _translations is already inside fieldList - $this->assertTrue($method->invokeArgs($marshall, $args)); - $this->assertEquals(['title', '_translations'], $options['fieldList']); + $marshall = new Marshaller($table); + $method = $this->_getProtectedMethod($marshall, '_hasTranslations'); - // Return false - $options['translations'] = false; - $this->assertFalse($method->invokeArgs($marshall, $args)); + $result = $method->invokeArgs($marshall, [$marshallOptions]); + $this->assertTrue($result, 'should return true'); - // Table not has behavior - $options['translations'] = true; - $this->assertFalse($method->invokeArgs($marshall, $args)); + // + $marshallOptions['translations'] = false; + $result = $method->invokeArgs($marshall, [$marshallOptions]); + $this->assertFalse($result, 'should return false'); } - /** - * Test Merge translation without errors - * - * @return void - */ - public function testMergeTranslations() + public function testAddTranslationToFieldList() { - // Mocks - $mockBehavior = $this->_getMockBehaviors(); - $mockTable = $this->_getMockTable(['mergeTranslations'], ['behaviors' => $mockBehavior]); - - $marshall = new Marshaller($mockTable); - $method = $this->_getProtectedMethod($marshall, '_mergeTranslations'); + $table = TableRegistry::get('Articles'); - $errors = []; - $data = ['en' => ['title' => 'Hi'], 'es' => ['title' => 'hola']]; - $args = [ - null, - $data, - &$errors + $marshall = new Marshaller($table); + $method = $this->_getProtectedMethod($marshall, '_addTranslationsToFieldList'); + $marshallOptions = [ + 'validate' => true, + 'translations' => true ]; - $returnValue = [ - [ - 'en' => null, - 'es' => null - ], - [] - ]; - $mockTable->expects($this->once()) - ->method('mergeTranslations') - ->with( - $this->isNull(), - $this->equalTo($data), - $this->isInstanceOf('Cake\ORM\Marshaller'), - $this->equalTo([]) - ) - ->will($this->returnValue($returnValue)); + // FieldList not present + $result = $method->invokeArgs($marshall, [$marshallOptions]); + $this->assertArrayNotHasKey('fieldList', $result); + + // Empty fieldlist + $marshallOptions['fieldList'] = []; + $result = $method->invokeArgs($marshall, [$marshallOptions]); + $this->assertEmpty($result['fieldList']); - $result = $method->invokeArgs($marshall, $args); + // FieldList not has _translations + $marshallOptions['fieldList'] = ['name']; + $result = $method->invokeArgs($marshall, [$marshallOptions]); + $expected = ['name', '_translations']; + $this->assertEquals($expected, $result['fieldList']); - $this->assertEquals($returnValue[0], $result); - $this->assertEmpty($errors); + // FieldList has _translations + $result = $method->invokeArgs($marshall, [$marshallOptions]); + $expected = ['name', '_translations']; + $this->assertEquals($expected, $result['fieldList']); } - /** - * Test Merge translation with errors, add to `$errors` parameter - * - * @return void - */ - public function testMergeTranslationsWithErrors() + public function testMergeTransaltionsWithOneMethod() { - // Mocks - $mockBehavior = $this->_getMockBehaviors(); - $mockTable = $this->_getMockTable(['mergeTranslations'], ['behaviors' => $mockBehavior]); - - $marshall = new Marshaller($mockTable); - $method = $this->_getProtectedMethod($marshall, '_mergeTranslations'); - - $errors = []; - $data = ['en' => ['title' => 'Hi'], 'es' => ['title' => 'hola']]; - $args = [ - null, - $data, - &$errors - ]; + $this->articles->behaviors()->load('Translate', [ + 'fields' => ['title', 'body'] + ]); - $returnValue = [ - [ - 'en' => null, - 'es' => null - ], - [ - 'en' => [] + $data = [ + 'author_id' => 1, + '_translations' => [ + 'en' => [ + 'title' => 'English Title', + 'body' => 'English Content' + ], + 'es' => [ + 'title' => 'Titulo Español', + 'body' => 'Contenido Español' + ] ] ]; - $mockTable->expects($this->once()) - ->method('mergeTranslations') - ->with( - $this->isNull(), - $this->equalTo($data), - $this->isInstanceOf('Cake\ORM\Marshaller'), - $this->equalTo([]) - ) - ->will($this->returnValue($returnValue)); - $result = $method->invokeArgs($marshall, $args); + $marshall = new Marshaller($this->articles); + $result = $marshall->one($data, []); + + $translations = $result->get('_translations'); - $this->assertEquals($returnValue[0], $result); - $this->assertArrayHasKey('_translations', $errors); + $this->assertEmpty($result->errors()); + $this->assertCount(2, $translations); + $this->assertInstanceOf(__NAMESPACE__ . '\OpenEntity', $translations['en']); + $this->assertInstanceOf(__NAMESPACE__ . '\OpenEntity', $translations['es']); + $this->assertEquals($data['_translations']['en'], $translations['en']->toArray()); } - /** - * Test Merge translation when return null value - * - * @return void - */ - public function testMergeTranslationsReturnNull() + public function testMergeTranslationsWithOneMethodAndReturnErrors() { - // Mocks - $mockBehavior = $this->_getMockBehaviors(); - $mockTable = $this->_getMockTable(['mergeTranslations'], ['behaviors' => $mockBehavior]); + $this->articles->behaviors()->load('Translate', [ + 'fields' => ['title', 'body'], + 'validator' => 'custom' + ]); - $marshall = new Marshaller($mockTable); - $method = $this->_getProtectedMethod($marshall, '_mergeTranslations'); + $validator = (new Validator)->add('title', 'notBlank', ['rule' => 'notBlank']); + $this->articles->validator('custom', $validator); - $errors = []; - $data = ['en' => ['title' => 'Hi'], 'es' => ['title' => 'hola']]; - $args = [ - null, - $data, - &$errors + $data = [ + 'author_id' => 1, + '_translations' => [ + 'en' => [ + 'title' => 'English Title', + 'body' => 'English Content' + ], + 'es' => [ + 'title' => '', + 'body' => '' + ] + ] ]; - $mockTable->expects($this->once()) - ->method('mergeTranslations') - ->with( - $this->isNull(), - $this->equalTo($data), - $this->isInstanceOf('Cake\ORM\Marshaller'), - $this->equalTo([]) - ) - ->will($this->returnValue(null)); + $marshall = new Marshaller($this->articles); + $result = $marshall->one($data, []); - $this->assertNull($method->invokeArgs($marshall, $args)); - $this->assertEmpty($errors); + $expected = [ + 'es' => [ + 'title' => [ + '_empty' => 'This field cannot be left empty' + ] + ] + ]; + $errors = $result->errors(); + $this->assertNotEmpty($errors); + $this->assertArrayHasKey('_translations', $errors); + $this->assertEquals($expected, $errors['_translations']); } - /** - * Test calling One method from Marshaller - * - * @return void - */ - public function testMergeTranslationsViaOneMethod() + public function testMergeTransaltionsWithMergeMethod() { - $data = [ - 'author_id' => 1, - '_translations' => [ - 'en' => [ - 'title' => 'Title EN' - ], - 'es' => [ - 'title' => 'Title ES' - ] - ] - ]; + $this->articles->behaviors()->load('Translate', [ + 'fields' => ['title', 'body'] + ]); - $expectedMarshallerOptions = [ - 'validate' => true, - 'translations' => true + $data = [ + 'author_id' => 1, + '_translations' => [ + 'en' => [ + 'title' => 'English Title', + 'body' => 'English Content' + ], + 'es' => [ + 'title' => 'Titulo Español', + 'body' => 'Contenido Español' + ] + ] ]; - // Mocks - $mockBehavior = $this->_getMockBehaviors(); - $mockBehavior->expects($this->any()) - ->method('hasMethod') - ->with('mergeTranslations') - ->will($this->returnValue(true)); - - $mockTable = $this->_getMockTable(['mergeTranslations'], ['behaviors' => $mockBehavior]); - $mockTable->expects($this->at(0)) - ->method('mergeTranslations') - ->with( - $this->isNull(), - $this->equalTo($data['_translations']), - $this->isInstanceOf('Cake\ORM\Marshaller'), - $this->equalTo($expectedMarshallerOptions) - ) - ->will($this->returnValue(true)); - - // Second Call - $returnValue = [ - [ - 'en' => null, - 'es' => null - ], - [ - 'en' => [] - ] - ]; - $mockTable->expects($this->at(1)) - ->method('mergeTranslations') - ->with( - $this->isNull(), - $this->equalTo($data['_translations']), - $this->isInstanceOf('Cake\ORM\Marshaller'), - $this->equalTo($expectedMarshallerOptions) - ) - ->will($this->returnValue($returnValue)); + $marshall = new Marshaller($this->articles); + $entity = $this->articles->newEntity(); + $result = $marshall->merge($entity, $data, []); - $marshall = new Marshaller($mockTable); - $result = $marshall->one($data); + $translations = $result->get('_translations'); - $this->assertTrue($result->get('_translations')); + $this->assertSame($entity, $result); $this->assertEmpty($result->errors()); - - // With errors - $result = $marshall->one($data); - $this->assertEquals($returnValue[0], $result->get('_translations')); - $this->assertNotEmpty($result->errors()); - $this->assertArrayHasKey('_translations', $result->errors()); + $this->assertTrue($result->dirty('_translations')); + $this->assertCount(2, $translations); + $this->assertInstanceOf(__NAMESPACE__ . '\OpenEntity', $translations['en']); + $this->assertInstanceOf(__NAMESPACE__ . '\OpenEntity', $translations['es']); + $this->assertEquals($data['_translations']['en'], $translations['en']->toArray()); } - /** - * Test calling merge method from Marshaller - * - * @return void - */ - public function testMergeTranslationsViaMergeMethod() + public function testMergeTranslationsWithOneMethodWithFieldList() { + $this->articles->behaviors()->load('Translate', [ + 'fields' => ['title', 'body'], + 'validator' => 'custom' + ]); + + $validator = (new Validator)->add('title', 'notBlank', ['rule' => 'notBlank']); + $this->articles->validator('custom', $validator); + $data = [ - 'author_id' => 1, - '_translations' => [ - 'en' => [ - 'title' => 'Title EN' - ], - 'es' => [ - 'title' => 'Title ES' - ] - ] + 'author_id' => 1, + '_translations' => [ + 'en' => [ + 'title' => 'English Title', + 'body' => 'English Content' + ], + 'es' => [ + 'title' => 'Titulo Español', + 'body' => '' + ] + ] ]; - $expectedMarshallerOptions = [ - 'validate' => true, - 'translations' => true + $marshall = new Marshaller($this->articles); + $result = $marshall->one($data, ['fieldList' => ['author_id', 'title']]); + + $expected = [ + 'en' => [ + 'title' => 'English Title' + ], + 'es' => [ + 'title' => 'Titulo Español' + ] ]; + $translations = $result->get('_translations'); + + $this->assertTrue($result->has('author_id')); + $this->assertEmpty($result->errors()); + $this->assertEquals($expected['en'], $translations['en']->toArray()); + $this->assertEquals($expected['es'], $translations['es']->toArray()); + } - // Mocks - $mockBehavior = $this->_getMockBehaviors(); - $mockBehavior->expects($this->any()) - ->method('hasMethod') - ->with('mergeTranslations') - ->will($this->returnValue(true)); + public function testMergeTranslationsWithMergeMethodAndReturnErrors() + { + $this->articles->behaviors()->load('Translate', [ + 'fields' => ['title', 'body'], + 'validator' => 'custom' + ]); - $mockTable = $this->_getMockTable(['mergeTranslations'], ['behaviors' => $mockBehavior]); - $mockTable->expects($this->at(0)) - ->method('mergeTranslations') - ->with( - $this->isNull(), - $this->equalTo($data['_translations']), - $this->isInstanceOf('Cake\ORM\Marshaller'), - $this->equalTo($expectedMarshallerOptions) - ) - ->will($this->returnValue(true)); + $validator = (new Validator)->add('title', 'notBlank', ['rule' => 'notBlank']); + $this->articles->validator('custom', $validator); - // Second Call - $returnValue = [ - [ - 'en' => null, - 'es' => null - ], - [ - 'en' => [] - ] + $data = [ + 'author_id' => 1, + '_translations' => [ + 'en' => [ + 'title' => 'English Title', + 'body' => 'English Content' + ], + 'es' => [ + 'title' => '', + 'body' => '' + ] + ] ]; - $mockTable->expects($this->at(1)) - ->method('mergeTranslations') - ->with( - $this->isTrue(), // _translation value inside entity is true (see last expect call) - $this->equalTo($data['_translations']), - $this->isInstanceOf('Cake\ORM\Marshaller'), - $this->equalTo($expectedMarshallerOptions) - ) - ->will($this->returnValue($returnValue)); + $marshall = new Marshaller($this->articles); $entity = $this->articles->newEntity(); + $result = $marshall->merge($entity, $data, []); - $marshall = new Marshaller($mockTable); - $marshall->merge($entity, $data); - - $this->assertTrue($entity->get('_translations')); - $this->assertEmpty($entity->errors()); - - // Now with errors - $marshall->merge($entity, $data); - $this->assertEquals($returnValue[0], $entity->get('_translations')); - $this->assertNotEmpty($entity->errors()); - $this->assertArrayHasKey('_translations', $entity->errors()); + $expected = [ + 'es' => [ + 'title' => [ + '_empty' => 'This field cannot be left empty' + ] + ] + ]; + $errors = $result->errors(); + $this->assertArrayHasKey('_translations', $errors); + $this->assertEquals($expected, $errors['_translations']); } - /** - * Helper method for making mocks. - * - * @param array $methods - * @return \Cake\ORM\Table (Mock) - */ - protected function _getMockTable(array $methods = [], array $config = []) + public function testMergeTranslationsWithMergeMethodUpdateFields() { - $config += [ - 'alias' => 'Articles', - 'schema' => [ - 'id' => ['type' => 'integer'] + $this->articles->behaviors()->load('Translate', [ + 'fields' => ['title', 'body'], + 'validator' => 'custom' + ]); + + $validator = (new Validator)->add('title', 'notBlank', ['rule' => 'notBlank']); + $this->articles->validator('custom', $validator); + + $data = [ + 'author_id' => 1, + '_translations' => [ + 'en' => [ + 'title' => 'English Title', + 'body' => 'English Content' + ], + 'es' => [ + 'title' => 'Titulo Español', + 'body' => '' + ] ] ]; + $marshall = new Marshaller($this->articles); + $entity = $this->articles->newEntity($data); - return $this->getMockBuilder('Cake\ORM\Table') - ->setMethods($methods) - ->setConstructorArgs([$config]) - ->getMock(); - } + $updateData = [ + '_translations' => [ + 'es' => [ + 'body' => 'Contenido Español' + ] + ] + ]; + $result = $marshall->merge($entity, $updateData, []); - /** - * Helper method for making mocks. - * - * @param array $methods - * @return Cake\ORM\BehaviorRegistry (Mock) - */ - protected function _getMockBehaviors(array $methods = []) - { - return $this->getMockBuilder('Cake\ORM\BehaviorRegistry') - ->disableOriginalConstructor() - ->setMethods($methods) - ->getMock(); + $entityData = $result->toArray(); + $this->assertEquals('Contenido Español', $entityData['_translations']['es']['body']); + $this->assertEmpty($result->errors()); } protected function _getProtectedMethod($obj, $name)