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