Skip to content

Commit

Permalink
OXDEV-665 fix __isset function in BaseModel
Browse files Browse the repository at this point in the history
  • Loading branch information
godefroy-le-hardi committed Jan 15, 2018
1 parent 0bf1ecf commit 71e9310
Show file tree
Hide file tree
Showing 7 changed files with 294 additions and 63 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- `OxidEsales\EshopCommunity\Application\Controller\Admin\ActionsMain::checkAccessToEditAction()`
- `OxidEsales\EshopCommunity\Application\Controller\Admin\AdminController::isNewEditObject()`
- `OxidEsales\EshopCommunity\Application\Model\Actions::isDefault()`
- `OxidEsales\EshopCommunity\Core\Model\BaseModel::isPropertyLoaded()`

### Changed
- In voucher series generation, if Coupon Number radio button checked, the number is marked as Required now. [PR-676](https://github.com/OXID-eSales/oxideshop_ce/pull/676)
Expand All @@ -30,6 +31,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Delivery dates from past shouldn't be displayed. [PR-543](https://github.com/OXID-eSales/oxideshop_ce/pull/543)
- Readme.md and Contributing.md files are updated.
- CSS adapted in OXID eShop Setup to reflect new design, extracted styles to separate file `Setup/out/src/main.css`
- The function isset on a not loaded property of a model with lazy loading loads the property if it's possible and returns true. To check if property is loaded use BaseModel::isPropertyLoaded()

### Deprecated
- `OxidEsales\EshopCommunity\Application\Controller\Admin\ArticleSeo::_getSaveObjectId`
Expand Down
73 changes: 52 additions & 21 deletions source/Core/Model/BaseModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Exception;
use OxidEsales\EshopCommunity\Core\Exception\DatabaseException;
use oxObjectException;
use \OxidEsales\Eshop\Core\Field;

/**
* Class BaseModel
Expand Down Expand Up @@ -326,7 +327,7 @@ public function __get($variableName)
}

//returns stdClass implementing __toString() method due to uknown scenario where this var should be used.
if (!isset($this->$variableName)) {
if (!$this->isPropertyLoaded($variableName)) {
$this->$variableName = null;
}

Expand All @@ -352,7 +353,7 @@ protected function getGetterViewName()
*/
public function __isset($variableName)
{
return isset($this->$variableName);
return $this->isPropertyLoaded($variableName) || $this->isPropertyField($variableName);
}

/**
Expand Down Expand Up @@ -525,7 +526,7 @@ public function setId($oxid = null)
}

$idFieldName = $this->getCoreTableName() . '__oxid';
$this->$idFieldName = new \OxidEsales\Eshop\Core\Field($this->_sOXID, \OxidEsales\Eshop\Core\Field::T_RAW);
$this->$idFieldName = new Field($this->_sOXID, Field::T_RAW);

return $this->_sOXID;
}
Expand Down Expand Up @@ -1162,15 +1163,15 @@ protected function _addField($fieldName, $fieldStatus, $type = null, $length = n

//already set?
$fieldLongName = $this->_getFieldLongName($fieldName);
if (isset($this->$fieldLongName)) {
if ($this->isPropertyLoaded($fieldLongName)) {
return;
}

//defining the field
$field = false;

if (isset($type)) {
$field = new \OxidEsales\Eshop\Core\Field();
$field = new Field();
$field->fldtype = $type;
//T2008-01-29
//can't clone as the fields are objects and are not fully cloned
Expand All @@ -1179,7 +1180,7 @@ protected function _addField($fieldName, $fieldStatus, $type = null, $length = n

if (isset($length)) {
if (!$field) {
$field = new \OxidEsales\Eshop\Core\Field();
$field = new Field();
}
$field->fldmax_length = $length;
$this->_blIsSimplyClonable = false;
Expand Down Expand Up @@ -1213,7 +1214,7 @@ protected function _getFieldLongName($fieldName)
* @param string $fieldValue Value of data field
* @param int $dataType Field type
*/
protected function _setFieldData($fieldName, $fieldValue, $dataType = \OxidEsales\Eshop\Core\Field::T_TEXT)
protected function _setFieldData($fieldName, $fieldValue, $dataType = Field::T_TEXT)
{
$longFieldName = $this->_getFieldLongName($fieldName);
//$sLongFieldName = $this->_sCoreTable . "__" . strtolower($sFieldName);
Expand All @@ -1225,22 +1226,29 @@ protected function _setFieldData($fieldName, $fieldValue, $dataType = \OxidEsale


//in non lazy loading case we just add a field and do not care about it more
if (!$this->_blUseLazyLoading && !isset($this->$longFieldName)) {
if (!$this->_blUseLazyLoading
&& !$this->isPropertyLoaded($longFieldName)
) {
$fieldsList = $this->_getAllFields(true);
if (isset($fieldsList[strtolower($fieldName)])) {
$this->_addField($fieldName, $this->_getFieldStatus($fieldName));
}
}
// if we have a double field we replace "," with "." in case somebody enters it in european format
if (isset($this->$longFieldName) && isset($this->$longFieldName->fldtype) && $this->$longFieldName->fldtype == 'double') {
if ($this->isPropertyLoaded($longFieldName)
&& isset($this->$longFieldName->fldtype)
&& $this->$longFieldName->fldtype == 'double'
) {
$fieldValue = str_replace(',', '.', $fieldValue);
}

// isset is REQUIRED here not to use getter
if (isset($this->$longFieldName) && is_object($this->$longFieldName)) {
if ($this->isPropertyLoaded($longFieldName)
&& is_object($this->$longFieldName)
) {
$this->$longFieldName->setValue($fieldValue, $dataType);
} else {
$this->$longFieldName = new \OxidEsales\Eshop\Core\Field($fieldValue, $dataType);
$this->$longFieldName = new Field($fieldValue, $dataType);
}
}

Expand Down Expand Up @@ -1285,15 +1293,15 @@ protected function _getFieldDefaultValue($fieldName)
/**
* returns quoted field value for using in update statement
*
* @param string $fieldName name of field
* @param \OxidEsales\Eshop\Core\Field $field field object
* @param string $fieldName name of field
* @param Field $field field object
*
* @return string
*/
protected function _getUpdateFieldValue($fieldName, $field)
{
$fieldValue = null;
if ($field instanceof \OxidEsales\Eshop\Core\Field) {
if ($field instanceof Field) {
$fieldValue = $field->getRawValue();
} elseif (isset($field->value)) {
$fieldValue = $field->value;
Expand Down Expand Up @@ -1380,7 +1388,7 @@ protected function _update()
$coreTableName = $this->getCoreTableName();

$idKey = \OxidEsales\Eshop\Core\Registry::getUtils()->getArrFldName($coreTableName . '.oxid');
$this->$idKey = new \OxidEsales\Eshop\Core\Field($this->getId(), \OxidEsales\Eshop\Core\Field::T_RAW);
$this->$idKey = new Field($this->getId(), Field::T_RAW);
$database = \OxidEsales\Eshop\Core\DatabaseProvider::getDb();

$updateQuery = "update {$coreTableName} set " . $this->_getUpdateFields() .
Expand Down Expand Up @@ -1424,18 +1432,17 @@ protected function _insert()
}

$idKey = $myUtils->getArrFldName($this->getCoreTableName() . '.oxid');
$this->$idKey = new \OxidEsales\Eshop\Core\Field($this->getId(), \OxidEsales\Eshop\Core\Field::T_RAW);
$this->$idKey = new Field($this->getId(), Field::T_RAW);
$insertSql = "Insert into {$this->getCoreTableName()} set ";

$shopIdField = $myUtils->getArrFldName($this->getCoreTableName() . '.oxshopid');

if (isset($this->$shopIdField)
&& (!$this->$shopIdField instanceof \OxidEsales\Eshop\Core\Field
|| !$this->$shopIdField->value)
if ($this->isPropertyLoaded($shopIdField)
&& (!$this->isPropertyField($shopIdField) || !$this->$shopIdField->value)
) {
$this->$shopIdField = new \OxidEsales\Eshop\Core\Field(
$this->$shopIdField = new Field(
$myConfig->getShopId(),
\OxidEsales\Eshop\Core\Field::T_RAW
Field::T_RAW
);
}

Expand Down Expand Up @@ -1547,6 +1554,18 @@ public function getLanguage()
return -1;
}

/**
* Returns true if the property is loaded.
*
* @param string $name
*
* @return bool
*/
public function isPropertyLoaded($name)
{
return property_exists($this, $name) && $this->$name !== null;
}

/**
* adds and activefrom/activeto to the query
*
Expand Down Expand Up @@ -1579,4 +1598,16 @@ protected function getSecondsToRoundForQueryCache()
//because active from setting is based on minutes
return 60;
}

/**
* Returns true if the property is a Field.
*
* @param string $name
*
* @return bool
*/
private function isPropertyField($name)
{
return $this->$name instanceof Field;
}
}
99 changes: 99 additions & 0 deletions tests/Integration/Core/Model/BaseModelTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php
/**
* Copyright © OXID eSales AG. All rights reserved.
* See LICENSE file for license details.
*/

namespace OxidEsales\EshopCommunity\Tests\Integration\Core\Model;

use OxidEsales\TestingLibrary\UnitTestCase;
use OxidEsales\Eshop\Application\Model\Article;

/**
* Class OxidEsales\EshopCommunity\Core\Model\BaseModelTest
*/
class BaseModelTest extends UnitTestCase
{
public function testFunctionIsPropertyLoadedReturnsFalseWhenPropertyIsNotLoadedAndIsField()
{
$model = $this->getModelWithLazyLoading();
$fieldName = $this->getTestFieldNameOfModelWithLazyLoading();

$this->assertFalse($model->isPropertyLoaded($fieldName));
}

public function testFunctionIsPropertyLoadedReturnsTrueWhenPropertyIsLoadedAndIsField()
{
$model = $this->getModelWithLazyLoading();
$fieldName = $this->getTestFieldNameOfModelWithLazyLoading();

$model->$fieldName;

$this->assertTrue($model->isPropertyLoaded($fieldName));
}

public function testLazyLoadingMagicIssetReturnsTrueWhenPropertyIsNotLoadedAndIsField()
{
$model = $this->getModelWithLazyLoading();
$fieldName = $this->getTestFieldNameOfModelWithLazyLoading();

$this->assertTrue(isset($model->$fieldName));
}

public function testLazyLoadingMagicIssetLoadsPropertyWhenPropertyIsNotLoadedAndIsField()
{
$model = $this->getModelWithLazyLoading();
$fieldName = $this->getTestFieldNameOfModelWithLazyLoading();

$this->assertTrue(isset($model->$fieldName));
}

public function testLazyLoadingMagicIssetReturnsTrueWhenPropertyIsLoadedAndIsField()
{
$model = $this->getModelWithLazyLoading();
$fieldName = $this->getTestFieldNameOfModelWithLazyLoading();

$model->$fieldName;

$this->assertTrue(isset($model->$fieldName));
}

public function testLazyLoadingMagicIssetOnValueOfFieldReturnsTrueWhenFieldIsNotLoaded()
{
$model = $this->getModelWithLazyLoading();
$fieldName = $this->getTestFieldNameOfModelWithLazyLoading();

$this->assertTrue(isset($model->$fieldName->value));
}

public function testLazyLoadingMagicIssetOnValueOfFieldReturnsTrueWhenFieldIsLoaded()
{
$model = $this->getModelWithLazyLoading();
$fieldName = $this->getTestFieldNameOfModelWithLazyLoading();

$model->$fieldName;

$this->assertTrue(isset($model->$fieldName->value));
}

public function testLazyLoadingMagicIssetOnValueOfPropertyReturnsFalseWhenPropertyIsNotFieldAndNotLoaded()
{
$model = $this->getModelWithLazyLoading();

$this->assertFalse(isset($model->someProperty->value));
}

private function getModelWithLazyLoading()
{
$model = oxNew(Article::class);
$model->init("oxarticles");
$model->load(2000);

return $model;
}

private function getTestFieldNameOfModelWithLazyLoading()
{
return 'oxarticles__oxartnum';
}
}
26 changes: 13 additions & 13 deletions tests/Unit/Application/Model/ArticleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4753,14 +4753,14 @@ public function testLazyLoadPictures()
$oArticle->load("09646538b54bac72b4ccb92fb5e3649f");
$oArticle->zz = true;

$this->assertFalse(isset($oArticle->oxarticles__oxpic1));
$this->assertFalse(isset($oArticle->oxarticles__oxzoom1));
$this->assertFalse($oArticle->isPropertyLoaded('oxarticles__oxpic1'));
$this->assertFalse($oArticle->isPropertyLoaded('oxarticles__oxzoom1'));

//first time access
$sPic = $oArticle->oxarticles__oxpic1->value;
$sZoomPic = $oArticle->oxarticles__oxzoom1->value;
$picture = $oArticle->oxarticles__oxpic1->value;
$zoom = $oArticle->oxarticles__oxzoom1->value;

$this->assertTrue(isset($oArticle->oxarticles__oxpic1));
$this->assertTrue($oArticle->isPropertyLoaded('oxarticles__oxpic1'));
$this->assertEquals("front_z1.jpg", $oArticle->oxarticles__oxpic1->value);
}

Expand All @@ -4774,13 +4774,13 @@ public function testLazyLoadPictureThumb()
$oArticle = oxNew('oxArticleHelper');
$oArticle->load("2000");

$this->assertFalse(isset($oArticle->oxarticles__oxthumb));
$this->assertFalse($oArticle->isPropertyLoaded('oxarticles__oxthumb'));

//first time access
$sPic = $oArticle->oxarticles__oxthumb->value;
$thumb = $oArticle->oxarticles__oxthumb->value;

$this->assertTrue(isset($oArticle->oxarticles__oxthumb));
$this->assertEquals("2000_th.jpg", $oArticle->oxarticles__oxthumb->value);
$this->assertTrue($oArticle->isPropertyLoaded('oxarticles__oxthumb'));
$this->assertEquals("2000_th.jpg", $thumb);
}

/**
Expand All @@ -4793,13 +4793,13 @@ public function testLazyLoadPictureIcon()
$oArticle = oxNew('oxArticleHelper');
$oArticle->load("2000");

$this->assertFalse(isset($oArticle->oxarticles__oxicon));
$this->assertFalse($oArticle->isPropertyLoaded('oxarticles__oxicon'));

//first time access
$sPic = $oArticle->oxarticles__oxicon->value;
$icon = $oArticle->oxarticles__oxicon->value;

$this->assertTrue(isset($oArticle->oxarticles__oxicon));
$this->assertEquals("2000_ico.jpg", $oArticle->oxarticles__oxicon->value);
$this->assertTrue($oArticle->isPropertyLoaded('oxarticles__oxicon'));
$this->assertEquals("2000_ico.jpg", $icon);
}

/**
Expand Down
12 changes: 6 additions & 6 deletions tests/Unit/Application/Model/ArticlelistTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1740,8 +1740,8 @@ public function testLazyLoadFields1()
$oTest = $this->getProxyClass("oxArticleList");
$oTest->selectString("select oxid from oxarticles where oxid = '2000'");
$this->assertEquals('2000', $oTest['2000']->getId());
//this should be lazy loaded
$this->assertFalse(isset($oTest['2000']->oxarticles__oxinsert));

$this->assertFalse($oTest['2000']->isPropertyLoaded('oxarticles__oxinsert'));
$this->assertEquals($sDate, $oTest['2000']->oxarticles__oxinsert->value);
}

Expand All @@ -1757,11 +1757,11 @@ public function testLazyLoadAllObjects1()
$oTest = $this->getProxyClass("oxArticleList");
$oTest->selectString("select oxid from oxarticles where oxid = '2000' or oxid = '1354'");
$this->assertEquals('2000', $oTest['2000']->getId());
//this should be lazy loaded
$this->assertFalse(isset($oTest['2000']->oxarticles__oxinsert));

$this->assertFalse($oTest['2000']->isPropertyLoaded('oxarticles__oxinsert'));
$this->assertEquals($sDate, $oTest['2000']->oxarticles__oxinsert->value);
//article 2
$this->assertFalse(isset($oTest['1354']->oxarticles__oxinsert));

$this->assertFalse($oTest['1354']->isPropertyLoaded('oxarticles__oxinsert'));
$this->assertEquals($sDate, $oTest['1354']->oxarticles__oxinsert->value);
}

Expand Down

0 comments on commit 71e9310

Please sign in to comment.