Skip to content

Commit

Permalink
chore(filestore): removed deprecated APIs
Browse files Browse the repository at this point in the history 
Fixes #9355

BREAKING CHANGE:
Removes `ElggFile::setFilestore`, `ElggFile::size`, `get_default_filestore`,
`set_default_filestore`, `ElggDiskFilestore::makeFileMatrix`, and the global
var `$DEFAULT_FILE_STORE`.
  • Loading branch information
@mrclay
mrclay committed Feb 16, 2016
1 parent b501223 commit 618c79d
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 239 deletions.
14 changes: 14 additions & 0 deletions docs/guides/upgrading.rst
View file
Expand Up @@ -17,6 +17,20 @@ Removed views

* ``resources/file/download``

Removed functions/methods
-------------------------

* ``ElggFile::setFilestore``: ElggFile objects can no longer use custom filestores.
* ``ElggFile::size``: Use ``getSize``
* ``get_default_filestore``
* ``set_default_filestore``
* ``ElggDiskFilestore::makeFileMatrix``: Use ``Elgg\EntityDirLocator``

Removed global vars
-------------------

* ``$DEFAULT_FILE_STORE``

From 2.0 to 2.1
===============

Expand Down
5 changes: 5 additions & 0 deletions engine/classes/Elgg/Di/ServiceProvider.php
View file
Expand Up @@ -34,6 +34,7 @@
* @property-read \Elgg\Database\EntityTable $entityTable
* @property-read \Elgg\EventsService $events
* @property-read \Elgg\Assets\ExternalFiles $externalFiles
* @property-read \ElggDiskFilestore $filestore
* @property-read \Elgg\PluginHooksService $hooks
* @property-read \Elgg\Http\Input $input
* @property-read \Elgg\Logger $logger
Expand Down Expand Up @@ -163,6 +164,10 @@ public function __construct(\Elgg\Config $config) {
return new \Elgg\Assets\ExternalFiles($c->config->getStorageObject());
});

$this->setFactory('filestore', function(ServiceProvider $c) {
return new \ElggDiskFilestore($c->config->getDataPath());
});

$this->setFactory('hooks', function(ServiceProvider $c) {
return $this->resolveLoggerDependencies('hooks');
});
Expand Down
26 changes: 0 additions & 26 deletions engine/classes/ElggDiskFilestore.php
View file
Expand Up @@ -318,30 +318,4 @@ public function setParameters(array $parameters) {

return false;
}



/**
* Deprecated methods
*/

/**
* Construct a file path matrix for an entity.
*
* @param int $guid The guid of the entity to store the data under.
*
* @return string The path where the entity's data will be stored relative to the data dir.
* @deprecated 1.9 Use \Elgg\EntityDirLocator()
*/
protected function makeFileMatrix($guid) {
elgg_deprecated_notice('\ElggDiskFilestore::makeFileMatrix() is deprecated by \Elgg\EntityDirLocator', 1.9);
$entity = get_entity($guid);

if (!$entity instanceof \ElggEntity) {
return false;
}

$dir = new \Elgg\EntityDirLocator($guid);
return $dir->getPath();
}
}
191 changes: 16 additions & 175 deletions engine/classes/ElggFile.php
View file
Expand Up @@ -3,8 +3,7 @@
/**
* This class represents a physical file.
*
* Create a new \ElggFile object and specify a filename, and optionally a
* FileStore (if one isn't specified then the default is assumed.)
* Create a new \ElggFile object and specify a filename
*
* Open the file using the appropriate mode, and you will be able to
* read and write to the file.
Expand All @@ -21,11 +20,6 @@
*/
class ElggFile extends \ElggObject {

/**
* @var ElggFilestore|null Cache for getFilestore(). Do not use. Use getFilestore().
*/
private $filestore;

/**
* @var resource|null File handle used to identify this file in a filestore. Created by open.
*/
Expand All @@ -42,26 +36,6 @@ protected function initializeAttributes() {
$this->attributes['subtype'] = "file";
}

/**
* {@inheritdoc}
*/
public function getMetadata($name) {
if (0 === strpos($name, 'filestore::')) {
elgg_deprecated_notice("Do not access the ElggFile filestore metadata directly. Use setFilestore().", '2.0');
}
return parent::getMetadata($name);
}

/**
* {@inheritdoc}
*/
public function setMetadata($name, $value, $value_type = '', $multiple = false, $owner_guid = 0, $access_id = null) {
if (0 === strpos($name, 'filestore::')) {
elgg_deprecated_notice("Do not access the ElggFile filestore metadata directly. Use setFilestore().", '2.0');
}
return parent::setMetadata($name, $value, $value_type, $multiple, $owner_guid, $access_id);
}

/**
* Set the filename of this file.
*
Expand Down Expand Up @@ -104,9 +78,8 @@ public function getFilestoreSize($prefix = '', $container_guid = 0) {
if (!$container_guid) {
$container_guid = $this->container_guid;
}
$fs = $this->getFilestore();
// @todo add getSize() to \ElggFilestore
return $fs->getSize($prefix, $container_guid);
return $this->getFilestore()->getSize($prefix, $container_guid);
}

/**
Expand Down Expand Up @@ -217,14 +190,8 @@ public function open($mode) {
throw new \InvalidParameterException($msg);
}

// Get the filestore
$fs = $this->getFilestore();

// Ensure that we save the file details to object store
//$this->save();

// Open the file handle
$this->handle = $fs->open($this, $mode);
$this->handle = $this->getFilestore()->open($this, $mode);

return $this->handle;
}
Expand All @@ -237,9 +204,7 @@ public function open($mode) {
* @return bool
*/
public function write($data) {
$fs = $this->getFilestore();

return $fs->write($this->handle, $data);
return $this->getFilestore()->write($this->handle, $data);
}

/**
Expand All @@ -251,9 +216,7 @@ public function write($data) {
* @return mixed Data or false
*/
public function read($length, $offset = 0) {
$fs = $this->getFilestore();

return $fs->read($this->handle, $length, $offset);
return $this->getFilestore()->read($this->handle, $length, $offset);
}

/**
Expand All @@ -262,8 +225,7 @@ public function read($length, $offset = 0) {
* @return mixed The file contents.
*/
public function grabFile() {
$fs = $this->getFilestore();
return $fs->grabFile($this);
return $this->getFilestore()->grabFile($this);
}

/**
Expand All @@ -272,9 +234,7 @@ public function grabFile() {
* @return bool
*/
public function close() {
$fs = $this->getFilestore();

if ($fs->close($this->handle)) {
if ($this->getFilestore()->close($this->handle)) {
$this->handle = null;

return true;
Expand All @@ -289,9 +249,7 @@ public function close() {
* @return bool
*/
public function delete() {
$fs = $this->getFilestore();

$result = $fs->delete($this);
$result = $this->getFilestore()->delete($this);

if ($this->getGUID() && $result) {
$result = parent::delete();
Expand All @@ -305,13 +263,11 @@ public function delete() {
*
* @param int $position Position in bytes
*
* @return bool
* @return void
*/
public function seek($position) {
$fs = $this->getFilestore();

// @todo add seek() to \ElggFilestore
return $fs->seek($this->handle, $position);
$this->getFilestore()->seek($this->handle, $position);
}

/**
Expand All @@ -320,9 +276,7 @@ public function seek($position) {
* @return int The file position
*/
public function tell() {
$fs = $this->getFilestore();

return $fs->tell($this->handle);
return $this->getFilestore()->tell($this->handle);
}

/**
Expand All @@ -335,26 +289,13 @@ public function getSize() {
return $this->getFilestore()->getFileSize($this);
}

/**
* Return the size of the file in bytes.
*
* @return int
* @deprecated 1.8 Use getSize()
*/
public function size() {
elgg_deprecated_notice("Use \ElggFile::getSize() instead of \ElggFile::size()", 1.9);
return $this->getSize();
}

/**
* Return a boolean value whether the file handle is at the end of the file
*
* @return bool
*/
public function eof() {
$fs = $this->getFilestore();

return $fs->eof($this->handle);
return $this->getFilestore()->eof($this->handle);
}

/**
Expand All @@ -363,112 +304,16 @@ public function eof() {
* @return bool
*/
public function exists() {
$fs = $this->getFilestore();

return $fs->exists($this);
}

/**
* Set a filestore.
*
* @param \ElggFilestore $filestore The file store.
*
* @return void
* @deprecated Will be removed in 3.0
*/
public function setFilestore(\ElggFilestore $filestore) {
elgg_deprecated_notice(__METHOD__ . ' is deprecated.', '2.1');
$this->filestore = $filestore;
return $this->getFilestore()->exists($this);
}

/**
* Return a filestore suitable for saving this file.
* This filestore is either a pre-registered filestore,
* a filestore as recorded in metadata or the system default.
* Return the system filestore based on dataroot.
*
* @return \ElggFilestore
*
* @throws ClassNotFoundException
* @return \ElggDiskFilestore
*/
protected function getFilestore() {
if ($this->filestore) {
// already set
return $this->filestore;
}

// such a common case we just assume for now
$this->filestore = $GLOBALS['DEFAULT_FILE_STORE'];

if (!$this->guid) {
return $this->filestore;
}

// Note we use parent::getMetadata() below to avoid showing the warnings added in #9193

$class = parent::getMetadata('filestore::filestore');
if (!$class) {
return $this->filestore;
}

// common case
if ($class === ElggDiskFilestore::class
&& parent::getMetadata('filestore::dir_root') === _elgg_services()->config->getDataPath()) {
return $this->filestore;
}

if (!class_exists($class)) {
$this->filestore = null;
throw new \ClassNotFoundException("Unable to load filestore class $class for file {$this->guid}");
}

// 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;
}

/**
* Save the file
*
* Write the file's data to the filestore and save
* the corresponding entity.
*
* @see \ElggObject::save()
*
* @return bool
*/
public function save() {
if (!parent::save()) {
return false;
}

$filestore = $this->getFilestore();

// Note we use parent::getMetadata() below to avoid showing the warnings added in #9193

// Save datastore metadata
$params = $filestore->getParameters();
foreach ($params as $k => $v) {
parent::setMetadata("filestore::$k", $v);
}

// Now make a note of the filestore class
parent::setMetadata("filestore::filestore", get_class($filestore));

return true;
return _elgg_services()->filestore;
}

/**
Expand All @@ -478,10 +323,6 @@ public function save() {
*/
public function __sleep() {
return array_diff(array_keys(get_object_vars($this)), array(
// Don't persist filestore, which contains CONFIG
// https://github.com/Elgg/Elgg/issues/9081#issuecomment-152859856
'filestore',

// a resource
'handle',
));
Expand Down

0 comments on commit 618c79d

Please sign in to comment.