Jack Phoenix has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/334016 )

Change subject: Refactor backend logic (DB + memcache) into one class
......................................................................

Refactor backend logic (DB + memcache) into one class

This will allow to write an API module without duplicating the internals
for adding an entry to SpamRegex or deleting from SpamRegex.
Also there was no point in having things like validateRegex() inside the
spamRegexList class since that function doesn't seem like something
specific to the *form* class, but rather appears to be a generic utility
function.

Change-Id: I57802716596a7f8a03e02a808fd8e91ab047eb3d
---
A backend/SpamRegex.php
M backend/SpamRegexHooks.php
M backend/spamRegexForm.php
M backend/spamRegexList.php
M extension.json
5 files changed, 199 insertions(+), 140 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/SpamRegex 
refs/changes/16/334016/1

diff --git a/backend/SpamRegex.php b/backend/SpamRegex.php
new file mode 100644
index 0000000..c69da9b
--- /dev/null
+++ b/backend/SpamRegex.php
@@ -0,0 +1,171 @@
+<?php
+/**
+ * Utility class for managing the blocked phrases.
+ *
+ * @file
+ */
+class SpamRegex {
+
+       /**
+        * Add an entry to SpamRegex.
+        *
+        * @param string $phrase Phrase to block
+        * @param array $modes Block modes (textbox, summary)
+        * @param string $reason Reason for blocking the given phrase
+        * @param User $blocker User who is adding the phrase to SpamRegex
+        * @return Status
+        */
+       public static function add( $phrase, $modes, $reason, $blocker ) {
+               /* empty name */
+               if ( strlen( $phrase ) == 0 ) {
+                       return Status::newFatal( wfMessage( 
'spamregex-warning-1' )->escaped() );
+               }
+
+               /* validate expression */
+               $simple_regex = self::validateRegex( $phrase );
+               if ( !$simple_regex ) {
+                       return Status::newFatal( wfMessage( 'spamregex-error-1' 
)->escaped() );
+               }
+
+               /* we need at least one block mode specified... we can have 
them both, of course */
+               $textbox = isset( $modes['text'] ) && $modes['text'];
+               $summary = isset( $modes['summary'] ) && $modes['summary'];
+               if ( !$textbox && !$summary ) {
+                       return Status::newFatal( wfMessage( 
'spamregex-warning-2' )->escaped() );
+               }
+
+               /* make sure that we have a good reason for doing all this... */
+               if ( !$reason ) {
+                       return Status::newFatal( wfMessage( 
'spamregex-error-no-reason' )->escaped() );
+               }
+
+               /* insert to memc */
+               if ( !empty( $textbox ) ) {
+                       self::updateMemcKeys( 'add', $phrase, 0 );
+               }
+               if ( !empty( $summary ) ) {
+                       self::updateMemcKeys( 'add', $phrase, 1 );
+               }
+
+               /* make insert to DB */
+               $dbw = wfGetDB( DB_MASTER );
+               $dbw->insert(
+                       'spam_regex',
+                       array(
+                               'spam_text' => $phrase,
+                               'spam_timestamp' => wfTimestampNow(),
+                               'spam_user' => $blocker->getName(),
+                               'spam_textbox' => $textbox,
+                               'spam_summary' => $summary,
+                               'spam_reason' => $reason
+                       ),
+                       __METHOD__,
+                       array( 'IGNORE' )
+               );
+
+               /* duplicate entry */
+               if ( !$dbw->affectedRows() ) {
+                       return Status::newFatal( wfMessage( 
'spamregex-already-blocked', $phrase )->escaped() );
+               } else {
+                       return Status::newGood();
+               }
+       }
+
+       /**
+        * Delete a phrase from SpamRegex and update caches accordingly.
+        *
+        * @param string $phrase Phrase to delete from SpamRegex
+        * @return bool Operation status; true on success, false on failure
+        */
+       public static function delete( $phrase ) {
+               $text = urldecode( $phrase );
+
+               /* delete in memc */
+               self::updateMemcKeys( 'delete', $text );
+
+               /* delete in DB */
+               $dbw = wfGetDB( DB_MASTER );
+               $dbw->delete(
+                       'spam_regex',
+                       array( 'spam_text' => $text ),
+                       __METHOD__
+               );
+
+               return (bool)$dbw->affectedRows();
+       }
+
+       /**
+        * Get the correct cache key, depending on if we're on a wiki farm like
+        * setup where the spam_regex DB table is shared, or if we're on a
+        * single-wiki setup.
+        *
+        * @return string The proper memcached key, depending on whether 
spamRegex's DB table is shared or not
+        */
+       public static function getCacheKey( /*...*/ ) {
+               global $wgSharedDB, $wgSharedTables, $wgSharedPrefix;
+               $args = func_get_args();
+               if ( in_array( 'spam_regex', $wgSharedTables ) ) {
+                       $args = array_merge( array( $wgSharedDB, 
$wgSharedPrefix ), $args );
+                       return call_user_func_array( 'wfForeignMemcKey', $args 
);
+               } else {
+                       return call_user_func_array( 'wfMemcKey', $args );
+               }
+       }
+
+       /**
+        * Useful for cleaning the memcached keys
+        *
+        * @param string $action 'add' or 'delete', depending on what we're 
doing
+        * @param string $text String to be added or removed
+        * @param bool $mode 0 for textbox (=content), 1 for summary
+        */
+       public static function updateMemcKeys( $action, $text, $mode = false ) {
+               global $wgMemc;
+
+               if ( $mode === false ) {
+                       self::updateMemcKeys( $action, $text, 1 );
+                       self::updateMemcKeys( $action, $text, 0 );
+               }
+
+               $key_clause = ( $mode == 1 ) ? 'Summary' : 'Textbox';
+               $key = self::getCacheKey( 'spamRegexCore', 'spamRegex', 
$key_clause );
+               $phrases = $wgMemc->get( $key );
+
+               if ( $phrases ) {
+                       $spam_text = '/' . $text . '/i';
+
+                       switch ( $action ) {
+                               case 'add':
+                                       if ( !in_array( $spam_text, $phrases ) 
) {
+                                               $phrases[] = $spam_text;
+                                       }
+                                       break;
+                               case 'delete':
+                                       $phrases = array_diff( $phrases, array( 
$spam_text ) );
+                                       break;
+                       }
+
+                       $wgMemc->set( $key, $phrases, 0 );
+               }
+       }
+
+       /**
+        * Validate the given regex
+        *
+        * @throws Exception when supplied an invalid regular expression
+        * @param string $text Regex to be validated
+        * @return bool False if exceptions were found, otherwise true
+        */
+       public static function validateRegex( $text ) {
+               try {
+                       $test = @preg_match( "/{$text}/", 'Whatever' );
+                       if ( !is_int( $test ) ) {
+                               throw new Exception( 'error!' );
+                       }
+               } catch ( Exception $e ) {
+                       return false;
+               }
+               return true;
+       }
+
+}
\ No newline at end of file
diff --git a/backend/SpamRegexHooks.php b/backend/SpamRegexHooks.php
index 4dac7ad..c2e910b 100644
--- a/backend/SpamRegexHooks.php
+++ b/backend/SpamRegexHooks.php
@@ -1,8 +1,9 @@
 <?php
-
 /**
- * Fetch phrases to block, and fill $wgSpamRegex with them, rather than
- * scribble that into the variable at startup
+ * Hooked functions used by SpamRegex to add our magic to things like the edit
+ * page and whatnot.
+ *
+ * @file
  */
 class SpamRegexHooks {
 
@@ -104,6 +105,9 @@
         * Fetch regex data for the given mode, either from memcached, or 
failing
         * that, then from the database.
         *
+        * @todo Maybe consider moving to the SpamRegex class? Then again there 
are
+        * no outside callers for this method...
+        *
         * @param int $mode 0 = summary, 1 = textbox
         * @param int $db_master Use master database for fetching data?
         * @return array Array of spamRegexed phrases
@@ -114,7 +118,7 @@
                $phrases = array();
                /* first, check if regex string is already stored in memcache */
                $key_clause = ( $mode == 1 ) ? 'Textbox' : 'Summary';
-               $key = self::getCacheKey( 'spamRegexCore', 'spamRegex', 
$key_clause );
+               $key = SpamRegex::getCacheKey( 'spamRegexCore', 'spamRegex', 
$key_clause );
                $cached = $wgMemc->get( $key );
 
                if ( !$cached ) {
@@ -187,21 +191,5 @@
                );
 
                return true;
-       }
-
-       /**
-        * @note This is not a hooked function, but an utility function used all
-        *       over the place.
-        * @return string The proper memcached key, depending on whether 
spamRegex's DB table is shared or not
-        */
-       public static function getCacheKey( /*...*/ ) {
-               global $wgSharedDB, $wgSharedTables, $wgSharedPrefix;
-               $args = func_get_args();
-               if ( in_array( 'spam_regex', $wgSharedTables ) ) {
-                       $args = array_merge( array( $wgSharedDB, 
$wgSharedPrefix ), $args );
-                       return call_user_func_array( 'wfForeignMemcKey', $args 
);
-               } else {
-                       return call_user_func_array( 'wfMemcKey', $args );
-               }
        }
 }
\ No newline at end of file
diff --git a/backend/spamRegexForm.php b/backend/spamRegexForm.php
index 3ea81dd..147b46b 100644
--- a/backend/spamRegexForm.php
+++ b/backend/spamRegexForm.php
@@ -1,6 +1,6 @@
 <?php
 /**
- * Backend logic for the form for blocking phrases
+ * Logic for the form for blocking phrases
  *
  * @file
  */
@@ -21,7 +21,7 @@
        public $mBlockedTextbox;
 
        /**
-        * @var bool $mBlockedTextbox Is the phrase to be blocked in edit 
summaries?
+        * @var bool $mBlockedSummary Is the phrase to be blocked in edit 
summaries?
         */
        public $mBlockedSummary;
 
@@ -92,58 +92,23 @@
 
        /* on submit */
        function doSubmit() {
-               /* empty name */
-               if ( strlen( $this->mBlockedPhrase ) == 0 ) {
-                       $this->showForm( $this->context->msg( 
'spamregex-warning-1' )->escaped() );
-                       return;
+               $modes = array();
+               if ( $this->mBlockedTextbox ) {
+                       $modes['text'] = true;
+               }
+               if ( $this->mBlockedSummary ) {
+                       $modes['summary'] = true;
                }
 
-               /* validate expression */
-               $simple_regex = spamRegexList::validateRegex( 
$this->mBlockedPhrase );
-               if ( !$simple_regex ) {
-                       $this->showForm( $this->context->msg( 
'spamregex-error-1' )->escaped() );
-                       return;
-               }
-
-               /* we need at least one block mode specified... we can have 
them both, of course */
-               if ( !$this->mBlockedTextbox && !$this->mBlockedSummary ) {
-                       $this->showForm( $this->context->msg( 
'spamregex-warning-2' )->escaped() );
-                       return;
-               }
-
-               /* make sure that we have a good reason for doing all this... */
-               if ( !$this->mBlockedReason ) {
-                       $this->showForm( $this->context->msg( 
'spamregex-error-no-reason' )->escaped() );
-                       return;
-               }
-
-               /* insert to memc */
-               if ( !empty( $this->mBlockedTextbox ) ) {
-                       spamRegexList::updateMemcKeys( 'add', 
$this->mBlockedPhrase, 0 );
-               }
-               if ( !empty( $this->mBlockedSummary ) ) {
-                       spamRegexList::updateMemcKeys( 'add', 
$this->mBlockedPhrase, 1 );
-               }
-
-               /* make insert to DB */
-               $dbw = wfGetDB( DB_MASTER );
-               $dbw->insert(
-                       'spam_regex',
-                       array(
-                               'spam_text' => $this->mBlockedPhrase,
-                               'spam_timestamp' => wfTimestampNow(),
-                               'spam_user' => 
$this->context->getUser()->getName(),
-                               'spam_textbox' => $this->mBlockedTextbox,
-                               'spam_summary' => $this->mBlockedSummary,
-                               'spam_reason' => $this->mBlockedReason
-                       ),
-                       __METHOD__,
-                       array( 'IGNORE' )
+               $status = SpamRegex::add(
+                       $this->mBlockedPhrase,
+                       $modes,
+                       $this->mBlockedReason,
+                       $this->context->getUser()
                );
 
-               /* duplicate entry */
-               if ( !$dbw->affectedRows() ) {
-                       $this->showForm( $this->context->msg( 
'spamregex-already-blocked', $this->mBlockedPhrase )->escaped() );
+               if ( !$status->isGood() ) {
+                       $this->showForm( $status->getErrorMessage() );
                        return;
                }
 
diff --git a/backend/spamRegexList.php b/backend/spamRegexList.php
index 0f13c0f..21b5a81 100644
--- a/backend/spamRegexList.php
+++ b/backend/spamRegexList.php
@@ -40,43 +40,6 @@
        }
 
        /**
-        * Useful for cleaning the memcached keys
-        *
-        * @param string $action 'add' or 'delete', depending on what we're 
doing
-        * @param string $text String to be added or removed
-        * @param bool $mode 0 for textbox (=content), 1 for summary
-        */
-       public static function updateMemcKeys( $action, $text, $mode = false ) {
-               global $wgMemc;
-
-               if ( $mode === false ) {
-                       self::updateMemcKeys( $action, $text, 1 );
-                       self::updateMemcKeys( $action, $text, 0 );
-               }
-
-               $key_clause = ( $mode == 1 ) ? 'Summary' : 'Textbox';
-               $key = SpamRegexHooks::getCacheKey( 'spamRegexCore', 
'spamRegex', $key_clause );
-               $phrases = $wgMemc->get( $key );
-
-               if ( $phrases ) {
-                       $spam_text = '/' . $text . '/i';
-
-                       switch ( $action ) {
-                               case 'add':
-                                       if ( !in_array( $spam_text, $phrases ) 
) {
-                                               $phrases[] = $spam_text;
-                                       }
-                                       break;
-                               case 'delete':
-                                       $phrases = array_diff( $phrases, array( 
$spam_text ) );
-                                       break;
-                       }
-
-                       $wgMemc->set( $key, $phrases, 0 );
-               }
-       }
-
-       /**
         * Output list of blocked expressions
         *
         * @param string $err Error message, if any
@@ -159,21 +122,11 @@
 
        /* remove from list - without confirmation */
        function deleteFromList() {
-               $text = urldecode( $this->context->getRequest()->getVal( 'text' 
) );
+               $text = $this->context->getRequest()->getVal( 'text' );
 
-               /* delete in memc */
-               self::updateMemcKeys( 'delete', $text );
-
-               /* delete in DB */
-               $dbw = wfGetDB( DB_MASTER );
-               $dbw->delete(
-                       'spam_regex',
-                       array( 'spam_text' => $text ),
-                       __METHOD__
-               );
                $titleObj = SpecialPage::getTitleFor( 'SpamRegex' );
 
-               if ( $dbw->affectedRows() ) {
+               if ( SpamRegex::delete( $text ) ) {
                        /* success */
                        $action = 'success_unblock';
                } else {
@@ -196,7 +149,7 @@
                global $wgMemc;
 
                /* we use memcached here */
-               $key = SpamRegexHooks::getCacheKey( 'spamRegexCore', 
'numResults' );
+               $key = SpamRegex::getCacheKey( 'spamRegexCore', 'numResults' );
                $cached = $wgMemc->get( $key );
                $results = 0;
 
@@ -211,25 +164,6 @@
                $this->numResults = $results;
 
                return $results;
-       }
-
-       /**
-        * Validate the given regex
-        *
-        * @throws Exception when supplied an invalid regular expression
-        * @param string $text Regex to be validated
-        * @return bool False if exceptions were found, otherwise true
-        */
-       public static function validateRegex( $text ) {
-               try {
-                       $test = @preg_match( "/{$text}/", 'Whatever' );
-                       if ( !is_int( $test ) ) {
-                               throw new Exception( 'error!' );
-                       }
-               } catch ( Exception $e ) {
-                       return false;
-               }
-               return true;
        }
 
        /* on success */
diff --git a/extension.json b/extension.json
index a1c7abf..b252e3f 100644
--- a/extension.json
+++ b/extension.json
@@ -1,6 +1,6 @@
 {
        "name": "Regular Expression Spam Block",
-       "version": "1.4",
+       "version": "1.5",
        "author": [
                "Bartek Łapiński",
                "Alexandre Emsenhuber",
@@ -22,6 +22,7 @@
                "SpamRegexAliases": "SpamRegex.alias.php"
        },
        "AutoloadClasses": {
+               "SpamRegex": "/backend/SpamRegex.php",
                "spamRegexForm": "/backend/spamRegexForm.php",
                "spamRegexList": "/backend/spamRegexList.php",
                "SpamRegexUITemplate": "/templates/ui.tmpl.php",

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I57802716596a7f8a03e02a808fd8e91ab047eb3d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/SpamRegex
Gerrit-Branch: master
Gerrit-Owner: Jack Phoenix <j...@countervandalism.net>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to