Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Commit

Permalink
Merge pull request #2409 from alexispeter/model-create
Browse files Browse the repository at this point in the history
Model: don't call "_onChange" when creating a model
  • Loading branch information
alexispeter authored Dec 1, 2016
2 parents 173dcc5 + 1983eb9 commit a2bab53
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 17 deletions.
5 changes: 3 additions & 2 deletions library/CM/Model/Abstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
if ($useReplace) {
$this->_onChange();
}
}
$this->_autoCommit = true;
}
Expand Down Expand Up @@ -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;
Expand Down
49 changes: 34 additions & 15 deletions tests/library/CM/Model/AbstractTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -194,7 +195,6 @@ public function testCommit() {
$model->commit();

$this->assertSame($idRaw, $model->getIdRaw());
$model->_set($data);
}

public function testCommitMultipleSaves() {
Expand Down Expand Up @@ -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 */

Expand Down Expand Up @@ -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 */

Expand Down Expand Up @@ -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());
}
}

Expand Down

0 comments on commit a2bab53

Please sign in to comment.