File sharing cleanup, works perfectly I think :)

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 66be5f2b155..ae6315600f8 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;
-				}
-				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'];
+				if ($item['mtime'] > $mtime) {
+					$mtime = $item['mtime'];
 				}
-				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 2f101d33c8a..b6db96614fd 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 fc2e6e32c7c..582c9c66172 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 70d822f8814..38c20a75709 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 fee71b777b3..07d432f8066 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 c420943da26..940bf2d48a0 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 4b73cc183a3..efff2c5522c 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
 	}
 
 }