[FileBackend] Simplified code for handling "overwrite" parameter.
authorAaron Schulz <aschulz@wikimedia.org>
Mon, 29 Oct 2012 23:56:43 +0000 (16:56 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Tue, 30 Oct 2012 00:13:02 +0000 (17:13 -0700)
* Since doQuickOperations() implicitely sets "overwrite", and doOperations() handles it
  via FileOp, there is no reason to also have each backend double checking this parameter
  to handle it. The parameter is no implicit for all the *Internal() functions. This does
  not affect callers. It does reduce the amount of HEAD requests since 404s are not cached.

Change-Id: I7d827e16bc55fe5c7b9aa51ec0c6b2f7c0bb629e

includes/filebackend/FSFileBackend.php
includes/filebackend/FileBackendStore.php
includes/filebackend/SwiftFileBackend.php

index dd43f82..97abc27 100644 (file)
@@ -191,14 +191,9 @@ class FSFileBackend extends FileBackendStore {
                }
 
                if ( file_exists( $dest ) ) {
-                       if ( !empty( $params['overwrite'] ) ) {
-                               $ok = unlink( $dest );
-                               if ( !$ok ) {
-                                       $status->fatal( 'backend-fail-delete', $params['dst'] );
-                                       return $status;
-                               }
-                       } else {
-                               $status->fatal( 'backend-fail-alreadyexists', $params['dst'] );
+                       $ok = unlink( $dest );
+                       if ( !$ok ) {
+                               $status->fatal( 'backend-fail-delete', $params['dst'] );
                                return $status;
                        }
                }
@@ -256,14 +251,9 @@ class FSFileBackend extends FileBackendStore {
                }
 
                if ( file_exists( $dest ) ) {
-                       if ( !empty( $params['overwrite'] ) ) {
-                               $ok = unlink( $dest );
-                               if ( !$ok ) {
-                                       $status->fatal( 'backend-fail-delete', $params['dst'] );
-                                       return $status;
-                               }
-                       } else {
-                               $status->fatal( 'backend-fail-alreadyexists', $params['dst'] );
+                       $ok = unlink( $dest );
+                       if ( !$ok ) {
+                               $status->fatal( 'backend-fail-delete', $params['dst'] );
                                return $status;
                        }
                }
@@ -321,18 +311,13 @@ class FSFileBackend extends FileBackendStore {
                }
 
                if ( file_exists( $dest ) ) {
-                       if ( !empty( $params['overwrite'] ) ) {
-                               // Windows does not support moving over existing files
-                               if ( wfIsWindows() ) {
-                                       $ok = unlink( $dest );
-                                       if ( !$ok ) {
-                                               $status->fatal( 'backend-fail-delete', $params['dst'] );
-                                               return $status;
-                                       }
+                       // Windows does not support moving over existing files
+                       if ( wfIsWindows() ) {
+                               $ok = unlink( $dest );
+                               if ( !$ok ) {
+                                       $status->fatal( 'backend-fail-delete', $params['dst'] );
+                                       return $status;
                                }
-                       } else {
-                               $status->fatal( 'backend-fail-alreadyexists', $params['dst'] );
-                               return $status;
                        }
                }
 
@@ -424,14 +409,9 @@ class FSFileBackend extends FileBackendStore {
                }
 
                if ( file_exists( $dest ) ) {
-                       if ( !empty( $params['overwrite'] ) ) {
-                               $ok = unlink( $dest );
-                               if ( !$ok ) {
-                                       $status->fatal( 'backend-fail-delete', $params['dst'] );
-                                       return $status;
-                               }
-                       } else {
-                               $status->fatal( 'backend-fail-alreadyexists', $params['dst'] );
+                       $ok = unlink( $dest );
+                       if ( !$ok ) {
+                               $status->fatal( 'backend-fail-delete', $params['dst'] );
                                return $status;
                        }
                }
index 5f562d2..dd0bec9 100644 (file)
@@ -83,12 +83,12 @@ abstract class FileBackendStore extends FileBackend {
 
        /**
         * Create a file in the backend with the given contents.
+        * This will overwrite any file that exists at the destination.
         * Do not call this function from places outside FileBackend and FileOp.
         *
         * $params include:
         *   - content       : the raw file contents
         *   - dst           : destination storage path
-        *   - overwrite     : overwrite any file that exists at the destination
         *   - disposition   : Content-Disposition header value for the destination
         *   - async         : Status will be returned immediately if supported.
         *                     If the status is OK, then its value field will be
@@ -106,9 +106,7 @@ abstract class FileBackendStore extends FileBackend {
                } else {
                        $status = $this->doCreateInternal( $params );
                        $this->clearCache( array( $params['dst'] ) );
-                       if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                               $this->deleteFileCache( $params['dst'] ); // persistent cache
-                       }
+                       $this->deleteFileCache( $params['dst'] ); // persistent cache
                }
                wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
@@ -122,12 +120,12 @@ abstract class FileBackendStore extends FileBackend {
 
        /**
         * Store a file into the backend from a file on disk.
+        * This will overwrite any file that exists at the destination.
         * Do not call this function from places outside FileBackend and FileOp.
         *
         * $params include:
         *   - src           : source path on disk
         *   - dst           : destination storage path
-        *   - overwrite     : overwrite any file that exists at the destination
         *   - disposition   : Content-Disposition header value for the destination
         *   - async         : Status will be returned immediately if supported.
         *                     If the status is OK, then its value field will be
@@ -145,9 +143,7 @@ abstract class FileBackendStore extends FileBackend {
                } else {
                        $status = $this->doStoreInternal( $params );
                        $this->clearCache( array( $params['dst'] ) );
-                       if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                               $this->deleteFileCache( $params['dst'] ); // persistent cache
-                       }
+                       $this->deleteFileCache( $params['dst'] ); // persistent cache
                }
                wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
@@ -161,12 +157,12 @@ abstract class FileBackendStore extends FileBackend {
 
        /**
         * Copy a file from one storage path to another in the backend.
+        * This will overwrite any file that exists at the destination.
         * Do not call this function from places outside FileBackend and FileOp.
         *
         * $params include:
         *   - src           : source storage path
         *   - dst           : destination storage path
-        *   - overwrite     : overwrite any file that exists at the destination
         *   - disposition   : Content-Disposition header value for the destination
         *   - async         : Status will be returned immediately if supported.
         *                     If the status is OK, then its value field will be
@@ -180,9 +176,7 @@ abstract class FileBackendStore extends FileBackend {
                wfProfileIn( __METHOD__ . '-' . $this->name );
                $status = $this->doCopyInternal( $params );
                $this->clearCache( array( $params['dst'] ) );
-               if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                       $this->deleteFileCache( $params['dst'] ); // persistent cache
-               }
+               $this->deleteFileCache( $params['dst'] ); // persistent cache
                wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
@@ -225,12 +219,12 @@ abstract class FileBackendStore extends FileBackend {
 
        /**
         * Move a file from one storage path to another in the backend.
+        * This will overwrite any file that exists at the destination.
         * Do not call this function from places outside FileBackend and FileOp.
         *
         * $params include:
         *   - src           : source storage path
         *   - dst           : destination storage path
-        *   - overwrite     : overwrite any file that exists at the destination
         *   - disposition   : Content-Disposition header value for the destination
         *   - async         : Status will be returned immediately if supported.
         *                     If the status is OK, then its value field will be
@@ -245,9 +239,7 @@ abstract class FileBackendStore extends FileBackend {
                $status = $this->doMoveInternal( $params );
                $this->clearCache( array( $params['src'], $params['dst'] ) );
                $this->deleteFileCache( $params['src'] ); // persistent cache
-               if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                       $this->deleteFileCache( $params['dst'] ); // persistent cache
-               }
+               $this->deleteFileCache( $params['dst'] ); // persistent cache
                wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
index fed496a..f6d81bb 100644 (file)
@@ -201,12 +201,6 @@ class SwiftFileBackend extends FileBackendStore {
                // (a) Check the destination container and object
                try {
                        $dContObj = $this->getContainer( $dstCont );
-                       if ( empty( $params['overwrite'] ) &&
-                               $this->fileExists( array( 'src' => $params['dst'], 'latest' => 1 ) ) )
-                       {
-                               $status->fatal( 'backend-fail-alreadyexists', $params['dst'] );
-                               return $status;
-                       }
                } catch ( NoSuchContainerException $e ) {
                        $status->fatal( 'backend-fail-create', $params['dst'] );
                        return $status;
@@ -240,14 +234,10 @@ class SwiftFileBackend extends FileBackendStore {
                        if ( !empty( $params['async'] ) ) { // deferred
                                $op = $obj->write_async( $params['content'] );
                                $status->value = new SwiftFileOpHandle( $this, $params, 'Create', $op );
-                               if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                                       $status->value->affectedObjects[] = $obj;
-                               }
+                               $status->value->affectedObjects[] = $obj;
                        } else { // actually write the object in Swift
                                $obj->write( $params['content'] );
-                               if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                                       $this->purgeCDNCache( array( $obj ) );
-                               }
+                               $this->purgeCDNCache( array( $obj ) );
                        }
                } catch ( CDNNotEnabledException $e ) {
                        // CDN not enabled; nothing to see here
@@ -287,12 +277,6 @@ class SwiftFileBackend extends FileBackendStore {
                // (a) Check the destination container and object
                try {
                        $dContObj = $this->getContainer( $dstCont );
-                       if ( empty( $params['overwrite'] ) &&
-                               $this->fileExists( array( 'src' => $params['dst'], 'latest' => 1 ) ) )
-                       {
-                               $status->fatal( 'backend-fail-alreadyexists', $params['dst'] );
-                               return $status;
-                       }
                } catch ( NoSuchContainerException $e ) {
                        $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] );
                        return $status;
@@ -337,15 +321,11 @@ class SwiftFileBackend extends FileBackendStore {
                                        $op = $obj->write_async( $fp, filesize( $params['src'] ), true );
                                        $status->value = new SwiftFileOpHandle( $this, $params, 'Store', $op );
                                        $status->value->resourcesToClose[] = $fp;
-                                       if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                                               $status->value->affectedObjects[] = $obj;
-                                       }
+                                       $status->value->affectedObjects[] = $obj;
                                }
                        } else { // actually write the object in Swift
                                $obj->load_from_filename( $params['src'], true ); // calls $obj->write()
-                               if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                                       $this->purgeCDNCache( array( $obj ) );
-                               }
+                               $this->purgeCDNCache( array( $obj ) );
                        }
                } catch ( CDNNotEnabledException $e ) {
                        // CDN not enabled; nothing to see here
@@ -396,12 +376,6 @@ class SwiftFileBackend extends FileBackendStore {
                try {
                        $sContObj = $this->getContainer( $srcCont );
                        $dContObj = $this->getContainer( $dstCont );
-                       if ( empty( $params['overwrite'] ) &&
-                               $this->fileExists( array( 'src' => $params['dst'], 'latest' => 1 ) ) )
-                       {
-                               $status->fatal( 'backend-fail-alreadyexists', $params['dst'] );
-                               return $status;
-                       }
                } catch ( NoSuchContainerException $e ) {
                        $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] );
                        return $status;
@@ -420,14 +394,10 @@ class SwiftFileBackend extends FileBackendStore {
                        if ( !empty( $params['async'] ) ) { // deferred
                                $op = $sContObj->copy_object_to_async( $srcRel, $dContObj, $dstRel, null, $hdrs );
                                $status->value = new SwiftFileOpHandle( $this, $params, 'Copy', $op );
-                               if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                                       $status->value->affectedObjects[] = $dstObj;
-                               }
+                               $status->value->affectedObjects[] = $dstObj;
                        } else { // actually write the object in Swift
                                $sContObj->copy_object_to( $srcRel, $dContObj, $dstRel, null, $hdrs );
-                               if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                                       $this->purgeCDNCache( array( $dstObj ) );
-                               }
+                               $this->purgeCDNCache( array( $dstObj ) );
                        }
                } catch ( CDNNotEnabledException $e ) {
                        // CDN not enabled; nothing to see here
@@ -474,12 +444,6 @@ class SwiftFileBackend extends FileBackendStore {
                try {
                        $sContObj = $this->getContainer( $srcCont );
                        $dContObj = $this->getContainer( $dstCont );
-                       if ( empty( $params['overwrite'] ) &&
-                               $this->fileExists( array( 'src' => $params['dst'], 'latest' => 1 ) ) )
-                       {
-                               $status->fatal( 'backend-fail-alreadyexists', $params['dst'] );
-                               return $status;
-                       }
                } catch ( NoSuchContainerException $e ) {
                        $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] );
                        return $status;
@@ -500,15 +464,11 @@ class SwiftFileBackend extends FileBackendStore {
                                $op = $sContObj->move_object_to_async( $srcRel, $dContObj, $dstRel, null, $hdrs );
                                $status->value = new SwiftFileOpHandle( $this, $params, 'Move', $op );
                                $status->value->affectedObjects[] = $srcObj;
-                               if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                                       $status->value->affectedObjects[] = $dstObj;
-                               }
+                               $status->value->affectedObjects[] = $dstObj;
                        } else { // actually write the object in Swift
                                $sContObj->move_object_to( $srcRel, $dContObj, $dstRel, null, $hdrs );
                                $this->purgeCDNCache( array( $srcObj ) );
-                               if ( !empty( $params['overwrite'] ) ) { // file possibly mutated
-                                       $this->purgeCDNCache( array( $dstObj ) );
-                               }
+                               $this->purgeCDNCache( array( $dstObj ) );
                        }
                } catch ( CDNNotEnabledException $e ) {
                        // CDN not enabled; nothing to see here