From df8a2e5361714838637457373f9957c76fbf449f Mon Sep 17 00:00:00 2001 From: Michael Gapczynski Date: Thu, 16 Aug 2012 12:20:14 -0400 Subject: [PATCH] File sharing cleanup, works perfectly I think :) --- apps/files_sharing/lib/share/file.php | 91 +++++++++++-------------- apps/files_sharing/lib/share/folder.php | 73 ++++++++------------ apps/files_sharing/sharedstorage.php | 26 +++---- core/js/share.js | 1 - lib/files.php | 4 +- lib/public/share.php | 29 +++++--- tests/lib/share/share.php | 8 +++ 7 files changed, 111 insertions(+), 121 deletions(-) diff --git a/apps/files_sharing/lib/share/file.php b/apps/files_sharing/lib/share/file.php index 66be5f2b15..ae6315600f 100644 --- a/apps/files_sharing/lib/share/file.php +++ b/apps/files_sharing/lib/share/file.php @@ -27,10 +27,10 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { const FORMAT_OPENDIR = 3; public function isValidSource($item, $uid) { -// if (OC_Filesystem::file_exists($item)) { + if (OC_Filesystem::file_exists($item)) { return true; -// } -// return false; + } + return false; } public function getFilePath($item, $uid) { @@ -43,61 +43,48 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { } public function formatItems($items, $format, $parameters = null) { - if ($format == self::FORMAT_OPENDIR) { + if ($format == self::FORMAT_SHARED_STORAGE) { + // Only 1 item should come through for this format call + return array('path' => $items[key($items)]['file_source'], 'permissions' => $items[key($items)]['permissions']); + } else if ($format == self::FORMAT_FILE_APP) { $files = array(); - foreach ($items as $file) { - $files[] = basename($file['file_target']); + foreach ($items as $item) { + $file = array(); + $file['path'] = $item['file_target']; + $file['name'] = basename($item['file_target']); + $file['ctime'] = $item['ctime']; + $file['mtime'] = $item['mtime']; + $file['mimetype'] = $item['mimetype']; + $file['size'] = $item['size']; + $file['encrypted'] = $item['encrypted']; + $file['versioned'] = $item['versioned']; + $file['directory'] = $parameters['folder']; + $file['type'] = ($item['mimetype'] == 'httpd/unix-directory') ? 'dir' : 'file'; + $file['permissions'] = $item['permissions']; + if ($file['type'] == 'file') { + // Remove Create permission if type is file + $file['permissions'] &= ~OCP\Share::PERMISSION_CREATE; + } + $files[] = $file; } return $files; - } else if ($format == self::FORMAT_SHARED_STORAGE) { - $id = $items[key($items)]['file_source']; - $query = OCP\DB::prepare('SELECT path FROM *PREFIX*fscache WHERE id = ?'); - $result = $query->execute(array($id))->fetchAll(); - if (isset($result[0]['path'])) { - return array('path' => $result[0]['path'], 'permissions' => $items[key($items)]['permissions']); - } - return false; - } else { - $shares = array(); - $ids = array(); + } else if ($format == self::FORMAT_FILE_APP_ROOT) { + $mtime = 0; + $size = 0; foreach ($items as $item) { - $shares[$item['file_source']] = $item; - $ids[] = $item['file_source']; - } - $ids = "'".implode("','", $ids)."'"; - if ($format == self::FORMAT_FILE_APP) { - $query = OCP\DB::prepare('SELECT id, path, name, ctime, mtime, mimetype, size, encrypted, versioned, writable FROM *PREFIX*fscache WHERE id IN ('.$ids.')'); - $result = $query->execute(); - $files = array(); - while ($file = $result->fetchRow()) { - // Set target path - $file['path'] = $shares[$file['id']]['file_target']; - $file['name'] = basename($file['path']); - $file['directory'] = $parameters['folder']; - $file['type'] = ($file['mimetype'] == 'httpd/unix-directory') ? 'dir' : 'file'; - $permissions = $shares[$file['id']]['permissions']; - if ($file['type'] == 'file') { - // Remove Create permission if type is file - $permissions &= ~OCP\Share::PERMISSION_CREATE; - } - $file['permissions'] = $permissions; - $files[] = $file; + if ($item['mtime'] > $mtime) { + $mtime = $item['mtime']; } - return $files; - } else if ($format == self::FORMAT_FILE_APP_ROOT) { - $query = OCP\DB::prepare('SELECT id, path, name, ctime, mtime, mimetype, size, encrypted, versioned, writable FROM *PREFIX*fscache WHERE id IN ('.$ids.')'); - $result = $query->execute(); - $mtime = 0; - $size = 0; - while ($file = $result->fetchRow()) { - if ($file['mtime'] > $mtime) { - $mtime = $file['mtime']; - } - $size += $file['size']; - } - return array(0 => array('name' => 'Shared', 'mtime' => $mtime, 'mimetype' => 'httpd/unix-directory', 'size' => $size, 'writable' => false, 'type' => 'dir', 'directory' => '', 'permissions' => OCP\Share::PERMISSION_READ)); + $size += $item['size']; } - } + return array(0 => array('name' => 'Shared', 'mtime' => $mtime, 'mimetype' => 'httpd/unix-directory', 'size' => $size, 'writable' => false, 'type' => 'dir', 'directory' => '', 'permissions' => OCP\Share::PERMISSION_READ)); + } else if ($format == self::FORMAT_OPENDIR) { + $files = array(); + foreach ($items as $item) { + $files[] = basename($item['file_target']); + } + return $files; + } return array(); } diff --git a/apps/files_sharing/lib/share/folder.php b/apps/files_sharing/lib/share/folder.php index 2f101d33c8..b6db96614f 100644 --- a/apps/files_sharing/lib/share/folder.php +++ b/apps/files_sharing/lib/share/folder.php @@ -21,8 +21,32 @@ class OC_Share_Backend_Folder extends OC_Share_Backend_File { - public function inCollection($collections, $item) { - // TODO + public function formatItems($items, $format, $parameters = null) { + if ($format == self::FORMAT_SHARED_STORAGE) { + // Only 1 item should come through for this format call + return array('path' => $items[key($items)]['file_source'], 'permissions' => $items[key($items)]['permissions']); + } else if ($format == self::FORMAT_FILE_APP && isset($parameters['folder'])) { + // Only 1 item should come through for this format call + $folder = $items[key($items)]; + if (isset($parameters['mimetype_filter'])) { + $mimetype_filter = $parameters['mimetype_filter']; + } else { + $mimetype_filter = ''; + } + $path = $folder['file_source'].substr($parameters['folder'], 7 + strlen($folder['file_target'])); + $files = OC_FileCache::getFolderContent($path, '', $mimetype_filter); + foreach ($files as &$file) { + $file['directory'] = $parameters['folder']; + $file['type'] = ($file['mimetype'] == 'httpd/unix-directory') ? 'dir' : 'file'; + $file['permissions'] = $folder['permissions']; + if ($file['type'] == 'file') { + // Remove Create permission if type is file + $file['permissions'] &= ~OCP\Share::PERMISSION_CREATE; + } + } + return $files; + } + return array(); } public function getChildren($itemSource) { @@ -34,47 +58,4 @@ class OC_Share_Backend_Folder extends OC_Share_Backend_File { return $sources; } - public function formatItems($items, $format, $parameters = null) { - if ($format == self::FORMAT_FILE_APP && isset($parameters['folder'])) { - $folder = $items[key($items)]; - $query = OCP\DB::prepare('SELECT path FROM *PREFIX*fscache WHERE id = ?'); - $result = $query->execute(array($folder['file_source']))->fetchRow(); - if (isset($result['path'])) { - if (isset($parameters['mimetype_filter'])) { - $mimetype_filter = $parameters['mimetype_filter']; - } else { - $mimetype_filter = ''; - } - $pos = strpos($result['path'], $folder['item']); - $path = substr($result['path'], $pos).substr($parameters['folder'], strlen($folder['file_target'])); - $root = substr($result['path'], 0, $pos); - $files = OC_FileCache::getFolderContent($path, $root, $mimetype_filter); - foreach ($files as &$file) { - $file['directory'] = $parameters['folder']; - $file['type'] = ($file['mimetype'] == 'httpd/unix-directory') ? 'dir' : 'file'; - $permissions = $folder['permissions']; - if ($file['type'] == 'file') { - // Remove Create permission if type is file - $permissions &= ~OCP\Share::PERMISSION_CREATE; - } - $file['permissions'] = $permissions; - } - return $files; - } - }/* else if ($format == self::FORMAT_OPENDIR_ROOT) { - $query = OCP\DB::prepare('SELECT name FROM *PREFIX*fscache WHERE id IN ('.$ids.')'); - $result = $query->execute(); - $files = array(); - while ($file = $result->fetchRow()) { - // Set target path - $files[] = basename($shares[$file['id']]['item_target']); - } - return $files; - }*/ - return array(); - } - -} - - -?> \ No newline at end of file +} \ No newline at end of file diff --git a/apps/files_sharing/sharedstorage.php b/apps/files_sharing/sharedstorage.php index fc2e6e32c7..582c9c6617 100644 --- a/apps/files_sharing/sharedstorage.php +++ b/apps/files_sharing/sharedstorage.php @@ -50,7 +50,7 @@ class OC_Filestorage_Shared extends OC_Filestorage_Common { if (isset($this->files[$folder])) { $file = $this->files[$folder]; } else { - $file = OCP\Share::getItemSharedWith('file', $folder, OC_Share_Backend_File::FORMAT_SHARED_STORAGE); + $file = OCP\Share::getItemSharedWith('folder', $folder, OC_Share_Backend_File::FORMAT_SHARED_STORAGE); } if ($file) { $this->files[$target]['path'] = $file['path'].substr($target, strlen($folder)); @@ -285,19 +285,21 @@ class OC_Filestorage_Shared extends OC_Filestorage_Common { } public function file_put_contents($path, $data) { - if ($this->is_writable($path)) { - $source = $this->getSourcePath($path); - if ($source) { - $info = array( - 'target' => $this->sharedFolder.$path, - 'source' => $source, - ); - OCP\Util::emitHook('OC_Filestorage_Shared', 'file_put_contents', $info); - $storage = OC_Filesystem::getStorage($source); - $result = $storage->file_put_contents($this->getInternalPath($source), $data); - return $result; + if ($source = $this->getSourcePath($path)) { + // Check if permission is granted + if (($this->file_exists($path) && !$this->isUpdatable($path)) || ($this->is_dir($path) && !$this->isCreatable($path))) { + return false; } + $info = array( + 'target' => $this->sharedFolder.$path, + 'source' => $source, + ); + OCP\Util::emitHook('OC_Filestorage_Shared', 'file_put_contents', $info); + $storage = OC_Filesystem::getStorage($source); + $result = $storage->file_put_contents($this->getInternalPath($source), $data); + return $result; } + return false; } public function unlink($path) { diff --git a/core/js/share.js b/core/js/share.js index 70d822f881..38c20a7570 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -267,7 +267,6 @@ $(document).ready(function() { if (typeof FileActions !== 'undefined') { OC.Share.loadIcons('file'); - OC.Share.loadIcons('folder'); FileActions.register('all', 'Share', FileActions.PERMISSION_SHARE, function(filename) { // Return the correct sharing icon if (scanFiles.scanning) { return; } // workaround to prevent additional http request block scanning feedback diff --git a/lib/files.php b/lib/files.php index fee71b777b..07d432f806 100644 --- a/lib/files.php +++ b/lib/files.php @@ -41,9 +41,9 @@ class OC_Files { $pos = strpos($directory, '/', 8); // Get shared folder name if ($pos !== false) { - $itemTarget = substr($directory, 0, $pos); + $itemTarget = substr($directory, 7, $pos - 7); } else { - $itemTarget = $directory; + $itemTarget = substr($directory, 7); } $files = OCP\Share::getItemSharedWith('folder', $itemTarget, OC_Share_Backend_File::FORMAT_FILE_APP, array('folder' => $directory, 'mimetype_filter' => $mimetype_filter)); } diff --git a/lib/public/share.php b/lib/public/share.php index c420943da2..940bf2d48a 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -399,23 +399,28 @@ class Share { */ private static function getItems($itemType, $item = null, $shareType = null, $shareWith = null, $uidOwner = null, $format = self::FORMAT_NONE, $parameters = null, $limit = -1, $includeCollections = false, $itemShareWithBySource = false) { $backend = self::getBackend($itemType); - // Get filesystem root to add it to the file target and remove from the file source + // Get filesystem root to add it to the file target and remove from the file source, match file_source with the file cache if ($backend instanceof Share_Backend_File_Dependent) { + $fileDependent = true; $root = \OC_Filesystem::getRoot(); + // TODO No need to do this for FORMAT_STATUSES and loading the item in the dropdown, it's a performance waste + $where = 'INNER JOIN *PREFIX*fscache ON file_source = *PREFIX*fscache.id '; } else { + $fileDependent = false; + $where = ''; $root = ''; } if ($itemType == 'file' && !isset($item)) { - $where = 'WHERE file_target IS NOT NULL'; + $where .= 'WHERE file_target IS NOT NULL'; $query_args = array(); } else if ($includeCollections && !isset($item) && $collectionTypes = self::getCollectionItemTypes($itemType)) { // If includeCollections is true, find collections of this item type, e.g. a music album contains songs $item_types = array_merge(array($itemType), $collectionTypes); $placeholders = join(',', array_fill(0, count($item_types), '?')); - $where = "WHERE item_type IN ('".$placeholders."')"; + $where .= "WHERE item_type IN ('".$placeholders."')"; $query_args = $item_types; - } else { - $where = "WHERE item_type = ?"; + } else if ($itemType != 'file' && $itemType != 'folder') { + $where .= "WHERE item_type = ?"; $query_args = array($itemType); } if (isset($shareType) && isset($shareWith)) { @@ -445,7 +450,6 @@ class Share { $query_args[] = self::$shareTypeGroupUserUnique; } if ($itemType == 'file' || $itemType == 'folder') { - $where = "INNER JOIN *PREFIX*fscache ON file_source = *PREFIX*fscache.id ".$where; $column = 'file_source'; } else { $column = 'item_source'; @@ -491,6 +495,7 @@ class Share { } $where .= ' LIMIT '.$limit; } + // TODO Optimize selects if ($format == self::FORMAT_STATUSES) { if ($itemType == 'file' || $itemType == 'folder') { $select = '*PREFIX*share.id, item_type, *PREFIX*share.parent, share_type, *PREFIX*fscache.path as file_source'; @@ -505,7 +510,15 @@ class Share { $select = 'id, item_type, item_source, parent, share_type, share_with, permissions, stime, file_source'; } } else { - $select = '*'; + if ($fileDependent) { + if (($itemType == 'file' || $itemType == 'folder') && $format == \OC_Share_Backend_File::FORMAT_FILE_APP || $format == \OC_Share_Backend_File::FORMAT_FILE_APP_ROOT) { + $select = '*PREFIX*share.id, item_type, *PREFIX*share.parent, share_type, share_with, permissions, file_target, *PREFIX*fscache.id, path as file_source, name, ctime, mtime, mimetype, size, encrypted, versioned, writable'; + } else { + $select = '*PREFIX*share.id, item_type, item_source, item_target, *PREFIX*share.parent, share_type, share_with, permissions, stime, path as file_source, file_target'; + } + } else { + $select = '*'; + } } } $root = strlen($root); @@ -534,7 +547,7 @@ class Share { // TODO Check this outside of the loop // Check if this is a collection of the requested item type if ($row['item_type'] != $itemType && $itemType != 'file' && !isset($item)) { - if ($collectionBackend = self::getBackend($row['item_type'])) { + if ($collectionBackend = self::getBackend($row['item_type'] && $collectionBackend instanceof Share_Backend_Collection)) { $row['collection'] = array('item_type' => $itemType, $column => $row[$column]); // Fetch all of the children sources $children = $collectionBackend->getChildren($row[$column]); diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php index 4b73cc183a..efff2c5522 100644 --- a/tests/lib/share/share.php +++ b/tests/lib/share/share.php @@ -233,6 +233,8 @@ class Test_Share extends UnitTestCase { $this->assertTrue(OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user2, OCP\Share::PERMISSION_READ)); OC_User::setUserId($this->user2); $this->assertEqual(OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET), array('test.txt', 'test1.txt')); + + // Remove user } public function testShareWithGroup() { @@ -280,6 +282,12 @@ class Test_Share extends UnitTestCase { $this->assertEqual(OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET), array('test.txt', 'test1.txt')); OC_User::setUserId($this->user3); $this->assertEqual(OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET), array('test.txt')); + + // Remove user from group + + // Add user to group + + // Remove group } }