[ticket/15692] Move checks if file exist from adapter to storage

PHPBB3-15692

phpBB/phpbb/storage/adapter/adapter_interface.php

@@ -28,8 +28,7 @@ interface adapter_interface
 	 * @param string	path		The file to be written to.
 	 * @param string	content		The data to write into the file.
 	 *
-	 * @throws \phpbb\storage\exception\exception		When the file already exists
+	 * @throws \phpbb\storage\exception\exception		When the file cannot be written
-	 * 													When the file cannot be written
 	 */
 	public function put_contents($path, $content);
 
@@ -38,8 +37,7 @@ interface adapter_interface
 	 *
 	 * @param string	$path	The file to read
 	 *
-	 * @throws \phpbb\storage\exception\exception	When the file doesn't exist
+	 * @throws \phpbb\storage\exception\exception	When cannot read file contents
-	 * 													When cannot read file contents
 	 *
 	 * @return string	Returns file contents
 	 *
@@ -70,8 +68,7 @@ interface adapter_interface
 	 * @param string	$path_orig	The original file/direcotry
 	 * @param string	$path_dest	The target file/directory
 	 *
-	 * @throws \phpbb\storage\exception\exception		When target exists
+	 * @throws \phpbb\storage\exception\exception		When file/directory cannot be renamed
-	 * 													When file/directory cannot be renamed
 	 */
 	public function rename($path_orig, $path_dest);
 
@@ -81,8 +78,7 @@ interface adapter_interface
 	 * @param string	$path_orig	The original filename
 	 * @param string	$path_dest	The target filename
 	 *
-	 * @throws \phpbb\storage\exception\exception		When target exists
+	 * @throws \phpbb\storage\exception\exception		When the file cannot be copied
-	 * 													When the file cannot be copied
 	 */
 	public function copy($path_orig, $path_dest);
 

phpBB/phpbb/storage/adapter/local.php

@@ -118,11 +118,6 @@ class local implements adapter_interface, stream_interface
 	{
 		$this->ensure_directory_exists($path);
 
-		if ($this->exists($path))
-		{
-			throw new exception('STORAGE_FILE_EXISTS', $path);
-		}
-
 		try
 		{
 			$this->filesystem->dump_file($this->root_path . $this->get_path($path) . $this->get_filename($path), $content);

phpBB/phpbb/storage/storage.php

@@ -105,10 +105,24 @@ class storage
 	 * 													When the file cannot be written
 	 */
 	public function put_contents($path, $content)
+	{
+		if ($this->exists($path))
+		{
+			throw new exception('STORAGE_FILE_EXISTS', $path);
+		}
+
+		try
 		{
 			$this->get_adapter()->put_contents($path, $content);
 			$this->track_file($path);
 		}
+		catch (\Exception $e)
+		{
+			$this->get_adapter()->delete($path);
+			$this->untrack_file($path);
+			throw $e;
+		}
+	}
 
 	/**
 	 * Read the contents of a file
@@ -123,6 +137,11 @@ class storage
 	 */
 	public function get_contents($path)
 	{
+		if (!$this->exists($path))
+		{
+			throw new exception('STORAGE_FILE_NO_EXIST', $path);
+		}
+
 		return $this->get_adapter()->get_contents($path);
 	}
 
@@ -147,6 +166,11 @@ class storage
 	 */
 	public function delete($path)
 	{
+		if (!$this->exists($path))
+		{
+			throw new exception('STORAGE_FILE_NO_EXIST', $path);
+		}
+
 		$this->get_adapter()->delete($path);
 		$this->untrack_file($path);
 	}
@@ -157,14 +181,32 @@ class storage
 	 * @param string	$path_orig	The original file/direcotry
 	 * @param string	$path_dest	The target file/directory
 	 *
-	 * @throws \phpbb\storage\exception\exception		When target exists
+	 * @throws \phpbb\storage\exception\exception		When the file doesn't exist
+	 *													When target exists
 	 * 													When file/directory cannot be renamed
 	 */
 	public function rename($path_orig, $path_dest)
 	{
+		if (!$this->exists($path_orig))
+		{
+			throw new exception('STORAGE_FILE_NO_EXIST', $path_orig);
+		}
+
+		if ($this->exists($path_dest))
+		{
+			throw new exception('STORAGE_FILE_EXISTS', $path_dest);
+		}
+
+		try {
 			$this->get_adapter()->rename($path_orig, $path_dest);
 			$this->track_rename($path_orig, $path_dest);
 		}
+		catch (\Exception $e)
+		{
+			$this->untrack_file($path_dest);
+			throw $e;
+		}
+	}
 
 	/**
 	 * Copies a file
@@ -172,26 +214,51 @@ class storage
 	 * @param string	$path_orig	The original filename
 	 * @param string	$path_dest	The target filename
 	 *
-	 * @throws \phpbb\storage\exception\exception		When target exists
+	 * @throws \phpbb\storage\exception\exception		When the file doesn't exist
+	 *													When target exists
 	 * 													When the file cannot be copied
 	 */
 	public function copy($path_orig, $path_dest)
+	{
+		if (!$this->exists($path_orig))
+		{
+			throw new exception('STORAGE_FILE_NO_EXIST', $path_orig);
+		}
+
+		if ($this->exists($path_dest))
+		{
+			throw new exception('STORAGE_FILE_EXISTS', $path_dest);
+		}
+
+		try
 		{
 			$this->get_adapter()->copy($path_orig, $path_dest);
 			$this->track_file($path_dest);
 		}
+		catch (\Exception $e)
+		{
+			$this->untrack_file($path_dest);
+			throw $e;
+		}
+	}
 
 	/**
 	 * Reads a file as a stream
 	 *
 	 * @param string	$path	File to read
 	 *
-	 * @throws \phpbb\storage\exception\exception		When unable to open file
+	 * @throws \phpbb\storage\exception\exception		When the file doesn't exist
+	 *													When unable to open file
 	 *
 	 * @return resource	Returns a file pointer
 	 */
 	public function read_stream($path)
 	{
+		if (!$this->exists($path))
+		{
+			throw new exception('STORAGE_FILE_NO_EXIST', $path);
+		}
+
 		$stream = null;
 		$adapter = $this->get_adapter();
 
@@ -217,16 +284,30 @@ class storage
 	 * @param string	$path		The target file
 	 * @param resource	$resource	The resource
 	 *
-	 * @throws \phpbb\storage\exception\exception		When target file cannot be created
+	 * @throws \phpbb\storage\exception\exception		When the file exist
+	 *													When target file cannot be created
 	 */
 	public function write_stream($path, $resource)
 	{
+		if ($this->exists($path))
+		{
+			throw new exception('STORAGE_FILE_EXISTS', $path);
+		}
+
 		$adapter = $this->get_adapter();
 
 		if ($adapter instanceof stream_interface)
+		{
+			try
 			{
 				$adapter->write_stream($path, $resource);
 			}
+			catch (\Exception $e)
+			{
+				$this->get_adapter()->delete($path);
+				$this->untrack_file($path);
+			}
+		}
 		else
 		{
 			// Simulate the stream