From e0fc836e817649be58245cc3fb0243c39df49579 Mon Sep 17 00:00:00 2001 From: Alexis Peter Date: Tue, 8 Nov 2016 18:35:10 +0100 Subject: [PATCH 1/2] Don't call _onChange() when creating models --- library/CM/Model/Abstract.php | 7 ++-- tests/library/CM/Model/AbstractTest.php | 49 +++++++++++++++++-------- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/library/CM/Model/Abstract.php b/library/CM/Model/Abstract.php index 5be371401..f7aca2c7c 100644 --- a/library/CM/Model/Abstract.php +++ b/library/CM/Model/Abstract.php @@ -94,9 +94,11 @@ public function commit($useReplace = null) { $this->_loadAssets(true); $cache->save($type, $this->getIdRaw(), $this->_getData()); } - $this->_onChange(); - $this->_changeContainingCacheables(); $this->_onCreate(); + $this->_changeContainingCacheables(); + if ($useReplace) { + $this->_onChange(); + } } $this->_autoCommit = true; } @@ -499,7 +501,6 @@ final public static function createStatic(array $data = null) { $data = array(); } $model = static::_createStatic($data); - $model->_onChange(); $model->_changeContainingCacheables(); $model->_onCreate(); return $model; diff --git a/tests/library/CM/Model/AbstractTest.php b/tests/library/CM/Model/AbstractTest.php index 7dcea9d6b..478c55026 100644 --- a/tests/library/CM/Model/AbstractTest.php +++ b/tests/library/CM/Model/AbstractTest.php @@ -175,14 +175,15 @@ public function testCommit() { $persistence = $this->getMockBuilder('CM_Model_StorageAdapter_AbstractAdapter')->setMethods(array('create', 'save')) ->getMockForAbstractClass(); $persistence->expects($this->once())->method('create')->with($type, $data)->will($this->returnValue($idRaw)); - $persistence->expects($this->once())->method('save')->with($type, $idRaw, $data); /** @var CM_Model_StorageAdapter_AbstractAdapter $persistence */ - $model = $this->getMockBuilder('CM_Model_Abstract')->setMethods(array('getType', '_getSchema', '_getPersistence')) + $model = $this->getMockBuilder('CM_Model_Abstract')->setMethods(array('getType', '_getSchema', '_getPersistence', '_onCreate', '_onChange')) ->disableOriginalConstructor()->getMockForAbstractClass(); $model->expects($this->any())->method('getType')->will($this->returnValue($type)); $model->expects($this->any())->method('_getSchema')->will($this->returnValue($schema)); $model->expects($this->any())->method('_getPersistence')->will($this->returnValue($persistence)); + $model->expects($this->once())->method('_onCreate'); + $model->expects($this->never())->method('_onChange'); /** @var CM_Model_Abstract $model */ $model->__construct(null, null); @@ -194,7 +195,6 @@ public function testCommit() { $model->commit(); $this->assertSame($idRaw, $model->getIdRaw()); - $model->_set($data); } public function testCommitMultipleSaves() { @@ -281,7 +281,7 @@ public function testCreate() { $model->expects($this->any())->method('_getSchema')->will($this->returnValue($schema)); $model->expects($this->once())->method('_getContainingCacheables')->will($this->returnValue(array($cacheableMock))); $model->expects($this->once())->method('_getAssets')->will($this->returnValue(array($asset))); - $model->expects($this->once())->method('_onChange'); + $model->expects($this->never())->method('_onChange'); $model->expects($this->once())->method('_onCreate'); /** @var CM_Model_Abstract $model */ @@ -326,7 +326,7 @@ public function testCreateMultiple() { $model->expects($this->any())->method('getType')->will($this->returnValue($type)); $model->expects($this->any())->method('_getPersistence')->will($this->returnValue($persistence)); $model->expects($this->any())->method('_getSchema')->will($this->returnValue($schema)); - $model->expects($this->exactly(2))->method('_onChange'); + $model->expects($this->never())->method('_onChange'); $model->expects($this->exactly(2))->method('_onCreate'); /** @var CM_Model_Abstract $model */ @@ -1151,20 +1151,39 @@ public function testDebugInfoWithoutId() { } public function testCommitWithReplace() { - CM_Config::get()->CM_Model_Abstract->types[CM_ModelMock3::getTypeStatic()] = 'CM_ModelMock3'; - $dbModel = new CM_ModelMock3(); - $dbModel->_set('foo', 'bar1'); - $dbModel->commit(true); - - $cacheModel = new CM_ModelMock4(); - $cacheModel->_set('bar', 5); - $exception = $this->catchException(function () use ($cacheModel) { - $cacheModel->commit(true); + $schema = new CM_Model_Schema_Definition(array('foo' => array())); + $idRaw = array('id' => '909'); + $type = 12; + $data = array('foo' => 12); + $persistence = $this->mockClass(CM_Model_StorageAdapter_AbstractAdapter::class, [CM_Model_StorageAdapter_ReplaceableInterface::class])->newInstance(); + $persistence->mockMethod('replace')->set($idRaw); + $modelClass = $this->mockClass(CM_Model_Abstract::class); + $modelClass->mockMethod('_getPersistence')->set($persistence); + $modelClass->mockMethod('_getSchema')->set($schema); + $modelClass->mockMethod('getType')->set($type); + $mockOnCreate = $modelClass->mockMethod('_onCreate'); + $mockOnChange = $modelClass->mockMethod('_onChange'); + + /** @var CM_Model_Abstract $model */ + $model = $modelClass->newInstance(); + $model->_set($data); + $model->commit(true); + $this->assertSame(1, $mockOnCreate->getCallCount()); + $this->assertSame(1, $mockOnChange->getCallCount()); + + $persistence = $this->mockClass(CM_Model_StorageAdapter_AbstractAdapter::class)->newInstance(); + $modelClass->mockMethod('_getPersistence')->set($persistence); + + /** @var CM_Model_Abstract $model */ + $model = $modelClass->newInstance(); + $model->_set($data); + $exception = $this->catchException(function () use ($model) { + $model->commit(true); }); $this->assertInstanceOf('CM_Exception_NotImplemented', $exception); /** @var CM_Exception_NotImplemented $exception */ $this->assertSame('Param `useReplace` is not allowed with adapter', $exception->getMessage()); - $this->assertSame(['adapterName' => CM_ModelMock4::getPersistenceClass()], $exception->getMetaInfo()); + $this->assertSame(['adapterName' => get_class($persistence)], $exception->getMetaInfo()); } } From 8862f80a728006f151377fb81d7be3c8c4d06ecd Mon Sep 17 00:00:00 2001 From: Alexis Peter Date: Wed, 23 Nov 2016 12:17:35 +0100 Subject: [PATCH 2/2] move _changeContainingCacheables()-call back before _onCreate()-call --- library/CM/Model/Abstract.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/CM/Model/Abstract.php b/library/CM/Model/Abstract.php index f7aca2c7c..f9b44b34c 100644 --- a/library/CM/Model/Abstract.php +++ b/library/CM/Model/Abstract.php @@ -94,8 +94,8 @@ public function commit($useReplace = null) { $this->_loadAssets(true); $cache->save($type, $this->getIdRaw(), $this->_getData()); } - $this->_onCreate(); $this->_changeContainingCacheables(); + $this->_onCreate(); if ($useReplace) { $this->_onChange(); }