Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

[RTM] Improve model registry #7725

Merged
merged 16 commits into from
Apr 7, 2015
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion system/modules/core/dca/tl_files.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@
),
'path' => array
(
'sql' => "varchar(1022) NOT NULL default ''"
'eval' => array('unique'=>true),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong, the path must not be unique.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??? That's what the whole PR and discussion today was about?? 20x FilesModel::findByPath('path.jpg') should return the same instance because it's unique? If not, what combination of columns does make it unique then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the UUID is unique. If you e.g. use SyncCto it may happen that you have two or more entries of the same file with different UUIDs, therefore we decided not to make the path unique.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'sql' => "varchar(1022) NOT NULL default ''",
),
'extension' => array
(
Expand Down
39 changes: 39 additions & 0 deletions system/modules/core/library/Contao/DcaExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ class DcaExtractor extends \Controller
*/
protected $arrOrderFields = array();

/**
* Unique fields
* @var array
*/
protected $arrUniqueFields = array();

/**
* Keys
* @var array
Expand Down Expand Up @@ -208,6 +214,28 @@ public function hasOrderFields()
}


/**
* Return an array of unique columns
*
* @return array
*/
public function getUniqueFields()
{
return $this->arrUniqueFields;
}


/**
* Return true if there are unique fields
*
* @return boolean True if there are unique fields
*/
public function hasUniqueFields()
{
return !empty($this->arrUniqueFields);
}


/**
* Return the keys as array
*
Expand Down Expand Up @@ -468,6 +496,11 @@ protected function createExtract()
{
$this->arrOrderFields[] = $config['eval']['orderField'];
}

if (isset($config['eval']['unique']) && $config['eval']['unique'] === true)
{
$this->arrUniqueFields[] = $field;
}
}
}

Expand All @@ -479,6 +512,11 @@ protected function createExtract()
foreach ($sql['keys'] as $field=>$type)
{
$this->arrKeys[$field] = $type;

if ($type == 'unique')
{
$this->arrUniqueFields[] = $field;
}
}
}

Expand All @@ -498,6 +536,7 @@ protected function createExtract()
}
}

$this->arrUniqueFields = array_unique($this->arrUniqueFields);
$this->blnIsDbTable = true;
}
}
121 changes: 101 additions & 20 deletions system/modules/core/library/Contao/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ abstract class Model
*/
protected static $strPk = 'id';

/**
* Class name cache
* @var array
*/
protected static $arrClassNames = array();

/**
* Data
* @var array
Expand Down Expand Up @@ -257,6 +263,25 @@ public static function getPk()
}


/**
* Return an array of unique field/column names
* Do not include the PK here as this is handled separately
*
* @return array
*/
public static function getUniqueFields()
{
$objDca = \DcaExtractor::getInstance(static::getTable());

if ($objDca->hasUniqueFields())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be omitted as \DcaExtractor is initializing to empty array.
Simply always return the value of $objDca->getUniqueFields()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, „fixed“ in 2337fa6.

{
return $objDca->getUniqueFields();
}

return array();
}


/**
* Return the name of the related table
*
Expand Down Expand Up @@ -677,6 +702,44 @@ public function attach()
}


/**
* Called when the model is attached/registered to the model registry
*
* @param \Model\Registry
*/
public function onRegister(\Model\Registry $registry)
{
// Register aliases to unique fields
foreach (static::getUniqueFields() as $strColumn)
{
$varAliasValue = $this->{$strColumn};
if (!$registry->isRegisteredAlias($this, $strColumn, $varAliasValue))
{
$registry->registerAlias($this, $strColumn, $varAliasValue);
}
}
}


/**
* Called when the model is detached/unregistered from the model registry
*
* @param \Model\Registry
*/
public function onUnregister(\Model\Registry $registry)
{
// Unregister aliases to unique fields
foreach (static::getUniqueFields() as $strColumn)
{
$varAliasValue = $this->{$strColumn};
if ($registry->isRegisteredAlias($this, $strColumn, $varAliasValue))
{
$registry->unregisterAlias($this, $strColumn, $varAliasValue);
}
}
}


/**
* Prevent saving the model
*
Expand Down Expand Up @@ -846,23 +909,6 @@ public static function findMultipleByIds($arrIds, array $arrOptions=array())
*/
public static function findOneBy($strColumn, $varValue, array $arrOptions=array())
{
// Try to load from the registry
if (empty($arrOptions))
{
$arrColumn = (array) $strColumn;

if (count($arrColumn) == 1 && $arrColumn[0] == static::$strPk)
{
$intId = is_array($varValue) ? $varValue[0] : $varValue;
$objModel = \Model\Registry::getInstance()->fetch(static::$strTable, $intId);

if ($objModel !== null)
{
return $objModel;
}
}
}

$arrOptions = array_merge
(
array
Expand Down Expand Up @@ -891,13 +937,22 @@ public static function findOneBy($strColumn, $varValue, array $arrOptions=array(
*/
public static function findBy($strColumn, $varValue, array $arrOptions=array())
{
$blnModel = false;

$arrColumn = (array) $strColumn;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming of $strColumn is now misleading, so is the phpDoc.
This should be changed to reflect the new behaviour.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I did not change this afaik?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be a string or an array in your implementation but was only a string before.
Otherwise the check on count($arrColumn) == 1 is not needed as it will always be 1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it could always be an array.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then the description was misleading from the beginning. :)


if (count($arrColumn) == 1 && ($arrColumn[0] === static::getPk() || in_array($arrColumn[0], static::getUniqueFields())))
{
$blnModel = true;
}

$arrOptions = array_merge
(
array
(
'column' => $strColumn,
'value' => $varValue,
'return' => 'Collection'
'return' => $blnModel ? 'Model' : 'Collection'
),

$arrOptions
Expand Down Expand Up @@ -988,6 +1043,25 @@ protected static function find(array $arrOptions)
return null;
}

if ($arrOptions['return'] === 'Model')
{
$arrColumn = (array) $arrOptions['column'];

if (count($arrColumn) == 1)
{
if ($arrColumn[0] == static::$strPk || in_array($arrColumn[0], static::getUniqueFields()))
{
$intId = is_array($arrOptions['value']) ? $arrOptions['value'][0] : $arrOptions['value'];
$objModel = \Model\Registry::getInstance()->fetch(static::$strTable, $intId, $arrColumn[0]);

if ($objModel !== null)
{
return $objModel;
}
}
}
}

$arrOptions['table'] = static::$strTable;
$strQuery = static::buildFindQuery($arrOptions);

Expand Down Expand Up @@ -1121,9 +1195,15 @@ public static function countAll()
*/
public static function getClassFromTable($strTable)
{
if (isset(static::$arrClassNames[$strTable]))
{
return static::$arrClassNames[$strTable];
}

if (isset($GLOBALS['TL_MODELS'][$strTable]))
{
return $GLOBALS['TL_MODELS'][$strTable]; // see 4796
static::$arrClassNames[$strTable] = $GLOBALS['TL_MODELS'][$strTable]; // see 4796
return static::$arrClassNames[$strTable];
}
else
{
Expand All @@ -1134,7 +1214,8 @@ public static function getClassFromTable($strTable)
array_shift($arrChunks);
}

return implode('', array_map('ucfirst', $arrChunks)) . 'Model';
static::$arrClassNames[$strTable] = implode('', array_map('ucfirst', $arrChunks)) . 'Model';
return static::$arrClassNames[$strTable];
}
}

Expand Down
Loading