Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor prepopulation #8395

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions docs/en/04_Changelogs/4.3.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- `DataList::column()` now returns all values and not just "distinct" values from a column as per the API docs
- `DataList`, `ArrayList` and `UnsavedRalationList` all have `columnUnique()` method for fetching distinct column values
- Take care with `stageChildren()` overrides. `Hierarchy::numChildren() ` results will only make use of `stageChildren()` customisations that are applied to the base class and don't include record-specific behaviour.

## Upgrading {#upgrading}

Expand Down
63 changes: 63 additions & 0 deletions src/ORM/DB.php
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,69 @@ public static function inline_parameters($sql, $parameters)
return $joined;
}


/**
* Replace 1 parameter with the given value
* For example, you can use this to replace a parameter with a column name instead of a literal
* @param string $sql The parameterised query
* @param int $paramIdx The 0-based position of the parameter
* @param array $replacement The value to insert into the queyr
* @param bool $skipEscaping Set to true to insert the value as-is, with no escaping. Use with caution!
*
* @return string
*/
public static function replace_parameter($sql, $paramIdx, $replacement, $skipEscaping = false)
{
$segments = preg_split('/\?/', $sql);
Copy link
Contributor

Choose a reason for hiding this comment

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

explode would be faster here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Addressed on parent PR by 07f9cdc

$joined = '';
$inString = false;
$numSegments = count($segments);
$currentParamIdx = 0;

for ($i = 0; $i < $numSegments; $i++) {
$input = $segments[$i];
// Append next segment
$joined .= $segments[$i];
// Don't add placeholder after last segment
if ($i === $numSegments - 1) {
maxime-rainville marked this conversation as resolved.
Show resolved Hide resolved
break;
}
// check string escape on previous fragment
// Remove escaped backslashes, count them!
$input = preg_replace('/\\\\\\\\/', '', $input);
// Count quotes
$totalQuotes = substr_count($input, "'"); // Includes double quote escaped quotes
$escapedQuotes = substr_count($input, "\\'");
if ((($totalQuotes - $escapedQuotes) % 2) !== 0) {
$inString = !$inString;
}

// Append placeholder replacement
if ($inString) {
// Literal question mark
$joined .= '?';
continue;
}

// If we've found the right parameter, replace it
if ($currentParamIdx == $paramIdx) {
if ($skipEscaping) {
$value = $replacement;
} elseif (is_bool($replacement)) {
$value = $replacement ? '1' : '0';
} elseif (is_int($replacement)) {
$value = $replacement;
} else {
$value = (DB::get_conn() !== null) ? Convert::raw2sql($replacement, true) : $replacement;
}
$joined .= $value;
}

$currentParamIdx++;
}
return $joined;
}

/**
* Execute the given SQL parameterised query with the specified arguments
*
Expand Down
147 changes: 140 additions & 7 deletions src/ORM/Hierarchy/Hierarchy.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
use SilverStripe\ORM\ArrayList;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\DataExtension;
use SilverStripe\ORM\DB;
use SilverStripe\Versioned\Versioned;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Convert;
use Exception;

/**
Expand Down Expand Up @@ -71,6 +74,15 @@ class Hierarchy extends DataExtension
*/
private static $hide_from_cms_tree = array();

/**
* Used to enable or disable the prepopulation of the numchildren cache.
* Defaults to true.
*
* @config
* @var boolean
*/
private static $prepopulate_numchildren_cache = true;

/**
* Prevent virtual page virtualising these fields
*
Expand All @@ -79,9 +91,17 @@ class Hierarchy extends DataExtension
*/
private static $non_virtual_fields = [
'_cache_children',
'_cache_numChildren',
];

/**
* A cache used by numChildren().
* Clear through {@link flushCache()}.
* version (int)0 means not on this stage.
*
* @var array
*/
protected static $cache_numChildren = [];

public static function get_extra_config($class, $extension, $args)
{
return array(
Expand Down Expand Up @@ -271,11 +291,18 @@ public function numHistoricalChildren()
*/
public function numChildren($cache = true)
{
// Load if caching

$baseClass = $this->owner->baseClass();
$cacheType = 'numChildren';
$id = $this->owner->ID;

// cached call
if ($cache) {
$numChildren = $this->owner->_cache_numChildren;
if (isset($numChildren)) {
return $numChildren;
if (isset(self::$cache_numChildren[$baseClass][$cacheType][$id])) {
return self::$cache_numChildren[$baseClass][$cacheType][$id];
} elseif (isset(self::$cache_numChildren[$baseClass][$cacheType]['_complete'])) {
// If the cache is complete and we didn't find our ID in the cache, it means this object is childless.
return 0;
}
}

Expand All @@ -284,11 +311,117 @@ public function numChildren($cache = true)

// Save if caching
if ($cache) {
$this->owner->_cache_numChildren = $numChildren;
self::$cache_numChildren[$baseClass][$cacheType][$id] = $numChildren;
}

return $numChildren;
}

/**
* Pre-populate any appropriate caches prior to rendering a tree.
* This is used to allow for the efficient rendering of tree views, notably in the CMS.
* In the cace of Hierarchy, it caches numChildren values. Other extensions can provide an
* onPrepopulateTreeDataCache(DataList $recordList = null, array $options) methods to hook
* into this event as well.
*
* @param DataList $recordList The list of records to prepopulate caches for. Null for all records.
* @param array $options A map of hints about what should be cached. "numChildrenMethod" and
* "childrenMethod" are allowed keys.
*/
public function prepopulateTreeDataCache(DataList $recordList = null, array $options = [])
{
if (empty($options['numChildrenMethod']) || $options['numChildrenMethod'] === 'numChildren') {
$idList = $recordList ? $recordList->column('ID') : null;
self::prepopulate_numchildren_cache($this->owner->baseClass(), $idList);
}

$this->owner->extend('onPrepopulateTreeDataCache', $recordList, $options);
}

/**
* Pre-populate the cache for Versioned::get_versionnumber_by_stage() for
* a list of record IDs, for more efficient database querying. If $idList
* is null, then every record will be pre-cached.
*
* @param string $class
* @param string $stage
* @param array $idList
*/
public static function prepopulate_numchildren_cache($baseClass, $idList = null)
{
if (!Config::inst()->get(static::class, 'prepopulate_numchildren_cache')) {
return;
}

/** @var Versioned|DataObject $singleton */
$dummyObject = new $baseClass();
$dummyObject->ID = -23478234; // going to use this for search & replace
$baseTable = $dummyObject->baseTable();

$idColumn = Convert::symbol2sql("{$baseTable}.ID");
$parentIdColumn = Convert::symbol2sql("ParentID");


// Get the stageChildren() result of a dummy object and break down into a generic query
$query = $dummyObject->stageChildren(true)->dataQuery()->query();
$where = $query->getWhere();

$newWhere = [];

foreach ($where as $i => $compoundClause) {
foreach ($compoundClause as $clause => $params) {
// Skip any "WHERE ParentID = [marker]" clauses as this will be replaced with a GROUP BY
if (!(preg_match('/^"[^"]+"\."ParentID" = \?$/', $clause) && $clause[1] == $dummyObject->ID)) {
// Replace any marker IDs with "<basetable>"."ID"
$paramNum = 0;
foreach ($params as $j => $param) {
if ($param == $dummyObject->ID) {
unset($params[$j]);
$clause = DB::replace_parameter($clause, $paramNum, $parentIdColumn, true);
} else {
$paramNum++;
}
}

$newWhere[] = [ $clause => $params ];
}
}
}

// optional ID-list filter
if ($idList) {
// Validate the ID list
foreach ($idList as $id) {
if (!is_numeric($id)) {
user_error(
"Bad ID passed to Versioned::prepopulate_numchildren_cache() in \$idList: " . $id,
E_USER_ERROR
);
}
}
$newWhere[] = ['"ParentID" IN (' . DB::placeholders($idList) . ')' => $idList];
}

$query->setWhere($newWhere);
$query->setOrderBy(null);

$query->setSelect([
'"ParentID"',
"COUNT(DISTINCT $idColumn) AS \"NumChildren\"",
]);
$query->setGroupBy([
"ParentID
"]);

$numChildren = $query->execute()->map();
self::$cache_numChildren[$baseClass]['numChildren'] = $numChildren;
if (!$idList) {
// If all objects are being cached, mark this cache as complete
// to avoid counting children of childless object.
self::$cache_numChildren[$baseClass]['numChildren']['_complete'] = true;
}
}

/**
* Checks if we're on a controller where we should filter. ie. Are we loading the SiteTree?
*
Expand Down Expand Up @@ -439,6 +572,6 @@ public function getBreadcrumbs($separator = ' &raquo; ')
public function flushCache()
{
$this->owner->_cache_children = null;
$this->owner->_cache_numChildren = null;
self::$cache_numChildren = [];
}
}