* Cleanups to getFirstRevision and getEarliestTime

* Rewrote countRevisionsBetween/countAuthorsBetween to avoid assuming rev_id is in chronological order
* Made countAuthorsBetween use $limit+1 automatically for convenience; updated callers

diff --git a/includes/Title.php b/includes/Title.php
index 45a31a3..7a1919e 100644 (file)
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -3638,65 +3638,68 @@ class Title {
         * @return Revision|Null if page doesn't exist
         */
        public function getFirstRevision( $flags = 0 ) {
-               $db = ( $flags & self::GAID_FOR_UPDATE ) ? wfGetDB( DB_MASTER ) : wfGetDB( DB_SLAVE );
                $pageId = $this->getArticleId( $flags );
-               if ( !$pageId ) {
-                       return null;
-               }
-               $row = $db->selectRow( 'revision', '*',
-                       array( 'rev_page' => $pageId ),
-                       __METHOD__,
-                       array( 'ORDER BY' => 'rev_timestamp ASC', 'LIMIT' => 1 )
-               );
-               if ( !$row ) {
-                       return null;
-               } else {
-                       return new Revision( $row );
+               if ( $pageId ) {
+                       $db = ( $flags & self::GAID_FOR_UPDATE ) ? wfGetDB( DB_MASTER ) : wfGetDB( DB_SLAVE );
+                       $row = $db->selectRow( 'revision', '*',
+                               array( 'rev_page' => $pageId ),
+                               __METHOD__,
+                               array( 'ORDER BY' => 'rev_timestamp ASC', 'LIMIT' => 1 )
+                       );
+                       if ( $row ) {
+                               return new Revision( $row );
+                       }
                }
+               return null;
        }
 
        /**
-        * Check if this is a new page
+        * Get the oldest revision timestamp of this page
         *
-        * @return bool
+        * @param $flags Int Title::GAID_FOR_UPDATE
+        * @return String: MW timestamp
         */
-       public function isNewPage() {
-               $dbr = wfGetDB( DB_SLAVE );
-               return (bool)$dbr->selectField( 'page', 'page_is_new', $this->pageCond(), __METHOD__ );
+       public function getEarliestRevTime( $flags = 0 ) {
+               $rev = $this->getFirstRevision( $flags );       
+               return $rev ? $rev->getTimestamp() : null;
        }
 
        /**
-        * Get the oldest revision timestamp of this page
+        * Check if this is a new page
         *
-        * @return String: MW timestamp
+        * @return bool
         */
-       public function getEarliestRevTime() {
+       public function isNewPage() {
                $dbr = wfGetDB( DB_SLAVE );
-               if ( $this->exists() ) {
-                       $min = $dbr->selectField( 'revision',
-                               'MIN(rev_timestamp)',
-                               array( 'rev_page' => $this->getArticleId() ),
-                               __METHOD__ );
-                       return wfTimestampOrNull( TS_MW, $min );
-               }
-               return null;
+               return (bool)$dbr->selectField( 'page', 'page_is_new', $this->pageCond(), __METHOD__ );
        }
 
        /**
-        * Get the number of revisions between the given revision IDs.
+        * Get the number of revisions between the given revision.
         * Used for diffs and other things that really need it.
         *
-        * @param $old Int Revision ID.
-        * @param $new Int Revision ID.
-        * @return Int Number of revisions between these IDs.
+        * @param $old int|Revision Old revision or rev ID (first before range)
+        * @param $new int|Revision New revision or rev ID (first after range)
+        * @return Int Number of revisions between these revisions.
         */
        public function countRevisionsBetween( $old, $new ) {
+               if ( !( $old instanceof Revision ) ) {
+                       $old = Revision::newFromTitle( $this, (int)$old );
+               }
+               if ( !( $new instanceof Revision ) ) {
+                       $new = Revision::newFromTitle( $this, (int)$new );
+               }
+               if ( !$old || !$new ) {
+                       return 0; // nothing to compare
+               }
                $dbr = wfGetDB( DB_SLAVE );
-               return (int)$dbr->selectField( 'revision', 'count(*)', array(
-                               'rev_page' => intval( $this->getArticleId() ),
-                               'rev_id > ' . intval( $old ),
-                               'rev_id < ' . intval( $new )
-                       ), __METHOD__
+               return (int)$dbr->selectField( 'revision', 'count(*)',
+                       array(
+                               'rev_page' => $this->getArticleId(),
+                               'rev_timestamp > ' . $dbr->addQuotes( $dbr->timestamp( $old->getTimestamp() ) ),
+                               'rev_timestamp < ' . $dbr->addQuotes( $dbr->timestamp( $new->getTimestamp() ) )
+                       ),
+                       __METHOD__
                );
        }
 
@@ -3704,23 +3707,31 @@ class Title {
         * Get the number of authors between the given revision IDs.
         * Used for diffs and other things that really need it.
         *
-        * @param $fromRevId Int Revision ID (first before range)
-        * @param $toRevId Int Revision ID (first after range)
+        * @param $old int|Revision Old revision or rev ID (first before range)
+        * @param $new int|Revision New revision or rev ID (first after range)
         * @param $limit Int Maximum number of authors
-        * @param $flags Int Title::GAID_FOR_UPDATE
-        * @return Int
+        * @return Int Number of revision authors between these revisions.
         */
-       public function countAuthorsBetween( $fromRevId, $toRevId, $limit, $flags = 0 ) {
-               $db = ( $flags & self::GAID_FOR_UPDATE ) ? wfGetDB( DB_MASTER ) : wfGetDB( DB_SLAVE );
-               $res = $db->select( 'revision', 'DISTINCT rev_user_text',
+       public function countAuthorsBetween( $old, $new, $limit ) {
+               if ( !( $old instanceof Revision ) ) {
+                       $old = Revision::newFromTitle( $this, (int)$old );
+               }
+               if ( !( $new instanceof Revision ) ) {
+                       $new = Revision::newFromTitle( $this, (int)$new );
+               }
+               if ( !$old || !$new ) {
+                       return 0; // nothing to compare
+               }
+               $dbr = wfGetDB( DB_SLAVE );
+               $res = $dbr->select( 'revision', 'DISTINCT rev_user_text',
                        array(
                                'rev_page' => $this->getArticleID(),
-                               'rev_id > ' . (int)$fromRevId,
-                               'rev_id < ' . (int)$toRevId
+                               'rev_timestamp > ' . $dbr->addQuotes( $dbr->timestamp( $old->getTimestamp() ) ),
+                               'rev_timestamp < ' . $dbr->addQuotes( $dbr->timestamp( $new->getTimestamp() ) )
                        ), __METHOD__,
-                       array( 'LIMIT' => $limit )
+                       array( 'LIMIT' => $limit + 1 ) // add one so caller knows it was truncated
                );
-               return (int)$db->numRows( $res );
+               return (int)$dbr->numRows( $res );
        }
 
        /**

diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php
index 9ec292f..e626335 100644 (file)
--- a/includes/diff/DifferenceEngine.php
+++ b/includes/diff/DifferenceEngine.php
@@ -844,7 +844,7 @@ CONTROL;
                        $limit = 100;
                        // We use ($limit + 1) so we can detect if there are > 100 authors
                        // in a given revision range. In that case, diff-multi-manyusers is used.
-                       $numUsers = $this->mTitle->countAuthorsBetween( $oldid, $newid, $limit + 1 );
+                       $numUsers = $this->mTitle->countAuthorsBetween( $oldid, $newid, $limit );
                        return self::intermediateEditsMsg( $nEdits, $numUsers, $limit );
                }
                return ''; // nothing