Skip to content
Browse files

Modifying user file locations to be based on guid.

Previous implementations utilized the owner's username to determine a file path matrix based on (up to) the first five letters. To eliminate language and filesystem inconsistencies, the matrix is now created from the entity's creation date and guid. This has the added benefit of (potentially) allowing users to update their usernames.


git-svn-id: http://code.elgg.org/elgg/trunk@3590 36083f99-b078-4883-b0ff-0f9b5a30f544
  • Loading branch information...
1 parent afa0511 commit bd2d492f2616489a40a0f3ed90ea5e6558117fc8 nickw committed Oct 27, 2009
Showing with 193 additions and 6 deletions.
  1. +61 −6 engine/lib/filestore.php
  2. +16 −0 engine/tests/elgg_unit_test.php
  3. +106 −0 engine/tests/objects/filestore.php
  4. +10 −0 engine/tests/objects/users.php
View
67 engine/lib/filestore.php
@@ -250,7 +250,7 @@ public function getFilenameOnFilestore(ElggFile $file) {
throw new InvalidParameterException(sprintf(elgg_echo('InvalidParameterException:MissingOwner'), $file->getFilename(), $file->guid));
}
- return $this->dir_root . $this->make_file_matrix($owner->username) . $file->getFilename();
+ return $this->dir_root . $this->make_file_matrix($owner->guid) . $file->getFilename();
}
public function grabFile(ElggFile $file) {
@@ -262,8 +262,8 @@ public function exists(ElggFile $file) {
}
public function getSize($prefix,$container_guid) {
- if ($container_guid && ($container=get_entity($container_guid)) && ($username = $container->username)) {
- return get_dir_size($this->dir_root.$this->make_file_matrix($username).$prefix);
+ if ($container_guid) {
+ return get_dir_size($this->dir_root.$this->make_file_matrix($container_guid).$prefix);
} else {
return false;
}
@@ -317,9 +317,56 @@ private function mb_str_split($string, $charset = 'UTF8') {
/**
* Construct the filename matrix.
*
- * @param string $filename
+ * @param int | string $identifier
+ * @return str
*/
- protected function make_file_matrix($filename) {
+ protected function make_file_matrix($identifier) {
+ if (is_numeric($identifier)) {
+ return $this->user_file_matrix($identifier);
+ }
+
+ return $this->deprecated_file_matrix($identifier);
+ }
+
+ /**
+ * Construct the filename matrix with user info
+ *
+ * This method will generate a matrix using the entity's creation time and
+ * unique guid. This is intended only to determine a user's data directory.
+ *
+ * @param int $guid
+ * @return str
+ */
+ protected function user_file_matrix($guid) {
+ // lookup the entity
+ $user = get_entity($guid);
+ if ($user->type != 'user')
+ {
+ // only to be used for user directories
+ return FALSE;
+ }
+
+ if (!$user->time_created) {
+ // fall back to deprecated method
+ return $this->deprecated_file_matrix($user->username);
+ }
+
+ $time_created = date('Y/m/d', $user->time_created);
+ return "$time_created/$user->guid/";
+ }
+
+ /**
+ * Construct the filename matrix using a string
+ *
+ * Particularly, this is used with a username to generate the file storage
+ * location.
+ *
+ * @deprecated for user directories: use user_file_matrix() instead.
+ *
+ * @param str $filename
+ * @return str
+ */
+ protected function deprecated_file_matrix($filename) {
$invalid_fs_chars = '*\'\\/"!$%^&*.%(){}[]#~?<>;|¬`@-+=';
$matrix = "";
@@ -1316,4 +1363,12 @@ function filestore_init() {
}
// Register a startup event
-register_elgg_event_handler('init', 'system', 'filestore_init', 100);
+register_elgg_event_handler('init', 'system', 'filestore_init', 100);
+
+// Unit testing
+register_plugin_hook('unit_test', 'system', 'filestore_test');
+function filestore_test($hook, $type, $value, $params) {
+ global $CONFIG;
+ $value[] = "{$CONFIG->path}engine/tests/objects/filestore.php";
+ return $value;
+}
View
16 engine/tests/elgg_unit_test.php
@@ -1,12 +1,28 @@
<?php
+/**
+ * Elgg Core Unit Tester
+ *
+ * This class is to be extended by all Elgg unit tests. As such, any method here
+ * will be available to the tests.
+ */
abstract class ElggCoreUnitTest extends UnitTestCase
{
+ /**
+ * Class constructor.
+ *
+ * A simple wrapper to call the parent constructor.
+ */
public function __construct()
{
parent::__construct();
}
+ /**
+ * Class destructor.
+ *
+ * The parent does not provide a destructor, so including an explicit one here.
+ */
public function __destruct()
{
}
View
106 engine/tests/objects/filestore.php
@@ -0,0 +1,106 @@
+<?php
+/**
+ * Elgg Test Skeleton
+ *
+ * @package Elgg
+ * @subpackage Test
+ * @author Curverider Ltd
+ * @link http://elgg.org/
+ */
+class ElggCoreFilestoreTest extends ElggCoreUnitTest {
+
+ /**
+ * Called before each test object.
+ */
+ public function __construct() {
+ parent::__construct();
+
+ // all code should come after here
+ }
+
+ /**
+ * Called before each test method.
+ */
+ public function setUp() {
+ $this->filestore = new ElggDiskFilestoreTest();
+ }
+
+ /**
+ * Called after each test method.
+ */
+ public function tearDown() {
+ // do not allow SimpleTest to interpret Elgg notices as exceptions
+ $this->swallowErrors();
+
+ unset($this->filestore);
+ }
+
+ /**
+ * Called after each test object.
+ */
+ public function __destruct() {
+ // all code should go above here
+ parent::__destruct();
+ }
+
+ public function testFileMatrix() {
+ global $CONFIG;
+
+ // create a test user
+ $user = $this->createTestUser();
+ $created = date('Y/m/d', $user->time_created);
+
+ // check matrix with username
+ $user_dir = $this->filestore->make_file_matrix($user->username);
+ $this->assertIdentical($user_dir, "f/i/l/e/T/fileTest/");
+
+ // check matrix with guid
+ $guid_dir = $this->filestore->make_file_matrix($user->guid);
+ $this->assertIdentical($guid_dir, "$created/$user->guid/");
+ $this->dump($user_dir);$this->dump($guid_dir);
+
+ // clean up user
+ $user->delete();
+ }
+
+ public function testFilenameOnFilestore() {
+ global $CONFIG;
+
+ // create a user to own the file
+ $user = $this->createTestUser();
+ $created = date('Y/m/d', $user->time_created);
+
+ // setup a test file; no need to save
+ $file = new ElggFile();
+ $file->owner_guid = $user->guid;
+ $file->setFilename('testing/filestore.txt');
+
+ // ensure filename and path is expected
+ $filename = $file->getFilenameOnFilestore($file);
+ $filepath = "$CONFIG->dataroot$created/$user->guid/testing/filestore.txt";
+ $this->assertIdentical($filename, $filepath);
+
+ // clean up user
+ $user->delete();
+ }
+
+
+ protected function createTestUser($username = 'fileTest') {
+ $user = new ElggUser();
+ $user->username = $username;
+ $guid = $user->save();
+
+ // load user to have access to creation time
+ return get_entity($guid);
+ }
+}
+
+class ElggDiskFilestoreTest extends ElggDiskFilestore {
+ public function make_file_matrix($filename) {
+ return parent::make_file_matrix($filename);
+ }
+
+ public function user_file_matrix($guid) {
+ return parent::user_file_matrix($guid);
+ }
+}
View
10 engine/tests/objects/users.php
@@ -142,6 +142,16 @@ public function testElggUserConstructorByObject() {
$object->delete();
}
+ public function testElggUserSave() {
+ // new object
+ $this->AssertEqual($this->user->getGUID(), 0);
+ $guid = $this->user->save();
+ $this->AssertNotEqual($guid, 0);
+
+ // clean up
+ $this->user->delete();
+ }
+
protected function fetchUser($guid) {
global $CONFIG;

0 comments on commit bd2d492

Please sign in to comment.
Something went wrong with that request. Please try again.