From 9fd48446a13dd3e23f360dad3838269ffa7f73a4 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 29 Oct 2012 16:56:43 -0700 Subject: [PATCH] [FileBackend] Simplified code for handling "overwrite" parameter. * 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 | 50 ++++++-------------- includes/filebackend/FileBackendStore.php | 24 ++++------ includes/filebackend/SwiftFileBackend.php | 56 ++++------------------- 3 files changed, 31 insertions(+), 99 deletions(-) diff --git a/includes/filebackend/FSFileBackend.php b/includes/filebackend/FSFileBackend.php index dd43f82921..97abc2720c 100644 --- a/includes/filebackend/FSFileBackend.php +++ b/includes/filebackend/FSFileBackend.php @@ -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; } } diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php index 5f562d2d8c..dd0bec950b 100644 --- a/includes/filebackend/FileBackendStore.php +++ b/includes/filebackend/FileBackendStore.php @@ -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; diff --git a/includes/filebackend/SwiftFileBackend.php b/includes/filebackend/SwiftFileBackend.php index fed496afcf..f6d81bbd71 100644 --- a/includes/filebackend/SwiftFileBackend.php +++ b/includes/filebackend/SwiftFileBackend.php @@ -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 -- 2.20.1