Permalink
Browse files

perf(files): ElggFile no longer queries metadata for filestore data

For each persisted `ElggFile` entity loaded, `getFilestore()` used to be called
on instantiation, which required a separate metadata query.

This refactors so `getFilestore()` is called only when needed, and typically when
Elgg's metadata cache has already been populated for the entity.

Fixes #9138
  • Loading branch information...
mrclay committed Dec 2, 2015
1 parent bb253ab commit d92430027393fb6d6657af72867f65e31e713ac0
Showing with 46 additions and 62 deletions.
  1. +46 −62 engine/classes/ElggFile.php
View
@@ -20,10 +20,15 @@
* @subpackage DataModel.File
*/
class ElggFile extends \ElggObject {
/** Filestore */
/**
* @var ElggFilestore|null Cache for getFilestore(). Do not use. Use getFilestore().
*/
private $filestore;
/** File handle used to identify this file in a filestore. Created by open. */
/**
* @var resource|null File handle used to identify this file in a filestore. Created by open.
*/
private $handle;
/**
@@ -37,18 +42,6 @@ protected function initializeAttributes() {
$this->attributes['subtype'] = "file";
}
/**
* Loads an \ElggFile entity.
*
* @param \stdClass $row Database result or null for new \ElggFile
*/
public function __construct($row = null) {
parent::__construct($row);
// Set default filestore
$this->filestore = $this->getFilestore();
}
/**
* Set the filename of this file.
*
@@ -76,7 +69,7 @@ public function getFilename() {
* @return string
*/
public function getFilenameOnFilestore() {
return $this->filestore->getFilenameOnFilestore($this);
return $this->getFilestore()->getFilenameOnFilestore($this);
}
/**
@@ -319,7 +312,7 @@ public function tell() {
* @since 1.9
*/
public function getSize() {
return $this->filestore->getFileSize($this);
return $this->getFilestore()->getFileSize($this);
}
/**
@@ -376,51 +369,50 @@ public function setFilestore(\ElggFilestore $filestore) {
* @throws ClassNotFoundException
*/
protected function getFilestore() {
// Short circuit if already set.
if ($this->filestore) {
// already set
return $this->filestore;
}
// ask for entity specific filestore
// saved as filestore::className in metadata.
// need to get all filestore::* metadata because the rest are "parameters" that
// get passed to filestore::setParameters()
if ($this->guid) {
$options = array(
'guid' => $this->guid,
'where' => array("n.string LIKE 'filestore::%'"),
);
$mds = elgg_get_metadata($options);
$parameters = array();
foreach ($mds as $md) {
list( , $name) = explode("::", $md->name);
if ($name == 'filestore') {
$filestore = $md->value;
}
$parameters[$name] = $md->value;
}
// such a common case we just assume for now
$this->filestore = get_default_filestore();
if (!$this->guid) {
return $this->filestore;
}
// need to check if filestore is set because this entity is loaded in save()
// before the filestore metadata is saved.
if (isset($filestore)) {
if (!class_exists($filestore)) {
$msg = "Unable to load filestore class " . $filestore . " for file " . $this->guid;
throw new \ClassNotFoundException($msg);
}
$class = $this->{'filestore::filestore'};
if (!$class) {
return $this->filestore;
}
// common case
if ($class === ElggDiskFilestore::class
&& $this->{'filestore::dir_root'} === _elgg_services()->config->getDataPath()) {
return $this->filestore;
}
$this->filestore = new $filestore();
$this->filestore->setParameters($parameters);
// @todo explain why $parameters will always be set here (PhpStorm complains)
if (!class_exists($class)) {
$this->filestore = null;
throw new \ClassNotFoundException("Unable to load filestore class $class for file {$this->guid}");
}
// this means the entity hasn't been saved so fallback to default
if (!$this->filestore) {
$this->filestore = get_default_filestore();
// need to get all filestore::* metadata because the rest are "parameters" that
// get passed to filestore::setParameters()
$mds = elgg_get_metadata([
'guid' => $this->guid,
'where' => array("n.string LIKE 'filestore::%'"),
]);
$parameters = [];
foreach ($mds as $md) {
list( , $name) = explode("::", $md->name);
if ($name !== 'filestore') {
$parameters[$name] = $md->value;
}
}
$this->filestore = new $class();
$this->filestore->setParameters($parameters);
return $this->filestore;
}
@@ -439,14 +431,16 @@ public function save() {
return false;
}
$filestore = $this->getFilestore();
// Save datastore metadata
$params = $this->filestore->getParameters();
$params = $filestore->getParameters();
foreach ($params as $k => $v) {
$this->setMetadata("filestore::$k", $v);
}
// Now make a note of the filestore class
$this->setMetadata("filestore::filestore", get_class($this->filestore));
$this->setMetadata("filestore::filestore", get_class($filestore));
return true;
}
@@ -466,14 +460,4 @@ public function __sleep() {
'handle',
));
}
/**
* Reestablish filestore property
*
* @return void
* @throws ClassNotFoundException
*/
public function __wakeup() {
$this->getFilestore();
}
}

0 comments on commit d924300

Please sign in to comment.