Chad has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/94258


Change subject: Clean up CDB classes
......................................................................

Clean up CDB classes

- Inheritance was a little funny, reduced some duplication
- Abstracted MWException to encourage reuse
- Abstracted wfDebug() to encourage reuse
- Declare visibility on a couple of public methods

Change-Id: I2edfeabaf62e39927abe869764ff2bd676821cd0
---
M includes/utils/Cdb.php
M includes/utils/CdbPHP.php
2 files changed, 104 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/58/94258/1

diff --git a/includes/utils/Cdb.php b/includes/utils/Cdb.php
index c6de088..5b2bcb1 100644
--- a/includes/utils/Cdb.php
+++ b/includes/utils/Cdb.php
@@ -21,11 +21,61 @@
  */
 
 /**
+ * Base class for all CDB operations. Doesn't do much other
+ * than abstracting some error handling/debug stuff
+ */
+abstract class CdbHandle {
+       /**
+        * The file handle
+        */
+       protected $handle;
+
+       /**
+        * Exception to throw on failures
+        * @var Exception
+        */
+       protected $exceptionClass = 'MWException';
+
+       /**
+        * Something to call when we've got debug output
+        * @var string
+        */
+       protected $debugCallback = 'wfDebug';
+
+       /**
+        * Create the object and open the file
+        *
+        * @param $fileName string
+        */
+       abstract public function __construct( $fileName );
+
+       /**
+        * Throw an exception!
+        *
+        * @param string $msg Error message
+        * @throws Exception
+        */
+       protected function throwException( $msg ) {
+               $e = $this->exceptionClass;
+               throw new $e( $msg );
+       }
+
+       /**
+        * Log a debug message
+        *
+        * @param string $msg Debug message
+        */
+       protected function debug( $msg ) {
+               call_user_func_array( $this->debugCallback, array( $msg ) );
+       }
+}
+
+/**
  * Read from a CDB file.
  * Native and pure PHP implementations are provided.
  * http://cr.yp.to/cdb.html
  */
-abstract class CdbReader {
+abstract class CdbReader extends CdbHandle {
        /**
         * Open a file and return a subclass instance
         *
@@ -37,8 +87,7 @@
                if ( self::haveExtension() ) {
                        return new CdbReaderDBA( $fileName );
                } else {
-                       wfDebug( "Warning: no dba extension found, using 
emulation.\n" );
-
+                       $this->debug( "Warning: no dba extension found, using 
emulation.\n" );
                        return new CdbReaderPHP( $fileName );
                }
        }
@@ -61,14 +110,9 @@
        }
 
        /**
-        * Construct the object and open the file
-        */
-       abstract function __construct( $fileName );
-
-       /**
         * Close the file. Optional, you can just let the variable go out of 
scope.
         */
-       abstract function close();
+       abstract public function close();
 
        /**
         * Get a value with a given key. Only string values are supported.
@@ -82,7 +126,19 @@
  * Write to a CDB file.
  * Native and pure PHP implementations are provided.
  */
-abstract class CdbWriter {
+abstract class CdbWriter extends CdbHandle {
+       /**
+        * File we'll be writing to when we're done
+        * @var string
+        */
+       protected $realFileName;
+
+       /**
+        * File we write to temporarily until we're done
+        * @var string
+        */
+       protected $tmpFileName;
+
        /**
         * Open a writer and return a subclass instance.
         * The user must have write access to the directory, for temporary file 
creation.
@@ -95,18 +151,10 @@
                if ( CdbReader::haveExtension() ) {
                        return new CdbWriterDBA( $fileName );
                } else {
-                       wfDebug( "Warning: no dba extension found, using 
emulation.\n" );
-
+                       $this->debug( "Warning: no dba extension found, using 
emulation.\n" );
                        return new CdbWriterPHP( $fileName );
                }
        }
-
-       /**
-        * Create the object and open the file
-        *
-        * @param $fileName string
-        */
-       abstract function __construct( $fileName );
 
        /**
         * Set a key to a given value. The value will be converted to string.
@@ -120,29 +168,36 @@
         * goes out of scope, to write out the final hashtables.
         */
        abstract public function close();
+
+       /**
+        * If the object goes out of scope, close it for sanity
+        */
+       public function __destruct() {
+               if ( isset( $this->handle ) ) {
+                       $this->close();
+               }
+       }
 }
 
 /**
  * Reader class which uses the DBA extension
  */
-class CdbReaderDBA {
-       var $handle;
-
-       function __construct( $fileName ) {
+class CdbReaderDBA extends CdbReader {
+       public function __construct( $fileName ) {
                $this->handle = dba_open( $fileName, 'r-', 'cdb' );
                if ( !$this->handle ) {
-                       throw new MWException( 'Unable to open CDB file "' . 
$fileName . '"' );
+                       $this->throwException( 'Unable to open CDB file "' . 
$fileName . '"' );
                }
        }
 
-       function close() {
+       public function close() {
                if ( isset( $this->handle ) ) {
                        dba_close( $this->handle );
                }
                unset( $this->handle );
        }
 
-       function get( $key ) {
+       public function get( $key ) {
                return dba_fetch( $key, $this->handle );
        }
 }
@@ -150,23 +205,21 @@
 /**
  * Writer class which uses the DBA extension
  */
-class CdbWriterDBA {
-       var $handle, $realFileName, $tmpFileName;
-
-       function __construct( $fileName ) {
+class CdbWriterDBA extends CdbWriter {
+       public function __construct( $fileName ) {
                $this->realFileName = $fileName;
                $this->tmpFileName = $fileName . '.tmp.' . mt_rand( 0, 
0x7fffffff );
                $this->handle = dba_open( $this->tmpFileName, 'n', 'cdb_make' );
                if ( !$this->handle ) {
-                       throw new MWException( 'Unable to open CDB file for 
write "' . $fileName . '"' );
+                       $this->throwException( 'Unable to open CDB file for 
write "' . $fileName . '"' );
                }
        }
 
-       function set( $key, $value ) {
+       public function set( $key, $value ) {
                return dba_insert( $key, $value, $this->handle );
        }
 
-       function close() {
+       public function close() {
                if ( isset( $this->handle ) ) {
                        dba_close( $this->handle );
                }
@@ -174,14 +227,8 @@
                        unlink( $this->realFileName );
                }
                if ( !rename( $this->tmpFileName, $this->realFileName ) ) {
-                       throw new MWException( 'Unable to move the new CDB file 
into place.' );
+                       $this->throwException( 'Unable to move the new CDB file 
into place.' );
                }
                unset( $this->handle );
-       }
-
-       function __destruct() {
-               if ( isset( $this->handle ) ) {
-                       $this->close();
-               }
        }
 }
diff --git a/includes/utils/CdbPHP.php b/includes/utils/CdbPHP.php
index e7bb4bc..8dee671 100644
--- a/includes/utils/CdbPHP.php
+++ b/includes/utils/CdbPHP.php
@@ -103,9 +103,6 @@
        /** The filename */
        var $fileName;
 
-       /** The file handle */
-       var $handle;
-
        /* number of hash slots searched under this key */
        var $loop;
 
@@ -129,18 +126,18 @@
 
        /**
         * @param $fileName string
-        * @throws MWException
+        * @throws Exception
         */
-       function __construct( $fileName ) {
+       public function __construct( $fileName ) {
                $this->fileName = $fileName;
                $this->handle = fopen( $fileName, 'rb' );
                if ( !$this->handle ) {
-                       throw new MWException( 'Unable to open CDB file "' . 
$this->fileName . '".' );
+                       $this->throwException( 'Unable to open CDB file "' . 
$this->fileName . '".' );
                }
                $this->findStart();
        }
 
-       function close() {
+       public function close() {
                if ( isset( $this->handle ) ) {
                        fclose( $this->handle );
                }
@@ -176,7 +173,7 @@
        }
 
        /**
-        * @throws MWException
+        * @throws Exception
         * @param $length
         * @param $pos
         * @return string
@@ -184,7 +181,7 @@
        protected function read( $length, $pos ) {
                if ( fseek( $this->handle, $pos ) == -1 ) {
                        // This can easily happen if the internal pointers are 
incorrect
-                       throw new MWException(
+                       $this->throwException(
                                'Seek failed, file "' . $this->fileName . '" 
may be corrupted.' );
                }
 
@@ -194,7 +191,7 @@
 
                $buf = fread( $this->handle, $length );
                if ( $buf === false || strlen( $buf ) !== $length ) {
-                       throw new MWException(
+                       $this->throwException(
                                'Read from CDB file failed, file "' . 
$this->fileName . '" may be corrupted.' );
                }
 
@@ -204,13 +201,13 @@
        /**
         * Unpack an unsigned integer and throw an exception if it needs more 
than 31 bits
         * @param $s
-        * @throws MWException
+        * @throws Exception
         * @return mixed
         */
        protected function unpack31( $s ) {
                $data = unpack( 'V', $s );
                if ( $data[1] > 0x7fffffff ) {
-                       throw new MWException(
+                       $this->throwException(
                                'Error in CDB file "' . $this->fileName . '", 
integer too big.' );
                }
 
@@ -291,15 +288,13 @@
  * CDB writer class
  */
 class CdbWriterPHP extends CdbWriter {
-       var $handle, $realFileName, $tmpFileName;
-
        var $hplist;
        var $numentries, $pos;
 
        /**
         * @param $fileName string
         */
-       function __construct( $fileName ) {
+       public function __construct( $fileName ) {
                $this->realFileName = $fileName;
                $this->tmpFileName = $fileName . '.tmp.' . mt_rand( 0, 
0x7fffffff );
                $this->handle = fopen( $this->tmpFileName, 'wb' );
@@ -312,12 +307,6 @@
                $this->pos = 2048; // leaving space for the pointer array, 256 
* 8
                if ( fseek( $this->handle, $this->pos ) == -1 ) {
                        $this->throwException( 'fseek failed in file "' . 
$this->tmpFileName . '".' );
-               }
-       }
-
-       function __destruct() {
-               if ( isset( $this->handle ) ) {
-                       $this->close();
                }
        }
 
@@ -338,7 +327,7 @@
        }
 
        /**
-        * @throws MWException
+        * @throws Exception
         */
        public function close() {
                $this->finish();
@@ -355,7 +344,7 @@
        }
 
        /**
-        * @throws MWException
+        * @throws Exception
         * @param $buf
         */
        protected function write( $buf ) {
@@ -366,7 +355,7 @@
        }
 
        /**
-        * @throws MWException
+        * @throws Exception
         * @param $len
         */
        protected function posplus( $len ) {
@@ -396,7 +385,7 @@
        }
 
        /**
-        * @throws MWException
+        * @throws Exception
         * @param $keylen
         * @param $datalen
         */
@@ -412,7 +401,7 @@
        }
 
        /**
-        * @throws MWException
+        * @throws Exception
         */
        protected function finish() {
                // Hack for DBA cross-check
@@ -491,13 +480,13 @@
         * Clean up the temp file and throw an exception
         *
         * @param $msg string
-        * @throws MWException
+        * @throws Exception
         */
        protected function throwException( $msg ) {
                if ( $this->handle ) {
                        fclose( $this->handle );
                        unlink( $this->tmpFileName );
                }
-               throw new MWException( $msg );
+               parent::throwException( $msg );
        }
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/94258
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2edfeabaf62e39927abe869764ff2bd676821cd0
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Chad <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to