Supress ChangeTags::addTags() exceptions for ManualLogEntry objects
authorPiotr Miazga <piotr@polishdeveloper.pl>
Tue, 26 Mar 2019 15:17:05 +0000 (16:17 +0100)
committerPiotr Miazga <piotr@polishdeveloper.pl>
Tue, 26 Mar 2019 17:00:12 +0000 (18:00 +0100)
Looks like some parts of the code try to publish log event when
$newId is 0 or null. This cause ChangeTags::addTags to throw
an exception as at least one of rc_id, rev_id, or log_id must be
specified.

When ChangeTags::addTags() fails (because both $rev_id and $log_id
are not present), just ignore the exception and continue with
the execution.

Also, if one of those is set to 0, we need to pass null instead
(do not insert 0's in to DB as both log_id and rev_id are foreign
keys).

Additionally log all places where ManualLogEntry::publish() is
called with incorrect arguments so later we can fix all occurencie
and remove that try{}catch around ChangeTags::addTags() call.

Bug: T218940
Change-Id: I495f79f2b7a7ef1503d229a689babdc12deb353c

includes/logging/LogEntry.php

index 33dd69b..5cad31f 100644 (file)
@@ -793,8 +793,24 @@ class ManualLogEntry extends LogEntryBase implements Taggable {
         * @param string $to One of: rcandudp (default), rc, udp
         */
        public function publish( $newId, $to = 'rcandudp' ) {
+               $canAddTags = true;
+               // FIXME: this code should be removed once all callers properly call publish()
+               if ( $to === 'udp' && !$newId && !$this->getAssociatedRevId() ) {
+                       \MediaWiki\Logger\LoggerFactory::getInstance( 'logging' )->warning(
+                               'newId and/or revId must be set when calling ManualLogEntry::publish()',
+                               [
+                                       'newId' => $newId,
+                                       'to' => $to,
+                                       'revId' => $this->getAssociatedRevId(),
+                                       // pass a new exception to register the stack trace
+                                       'exception' => new RuntimeException()
+                               ]
+                       );
+                       $canAddTags = false;
+               }
+
                DeferredUpdates::addCallableUpdate(
-                       function () use ( $newId, $to ) {
+                       function () use ( $newId, $to, $canAddTags ) {
                                $log = new LogPage( $this->getType() );
                                if ( !$log->isRestricted() ) {
                                        Hooks::runWithoutAbort( 'ManualLogEntryBeforePublish', [ $this ] );
@@ -806,9 +822,14 @@ class ManualLogEntry extends LogEntryBase implements Taggable {
                                                $rc->save( $rc::SEND_NONE );
                                        } else {
                                                $tags = $this->getTags();
-                                               if ( $tags ) {
-                                                       $revId = $this->getAssociatedRevId(); // Use null if $revId is 0
-                                                       ChangeTags::addTags( $tags, null, $revId > 0 ? $revId : null, $newId );
+                                               if ( $tags && $canAddTags ) {
+                                                       $revId = $this->getAssociatedRevId();
+                                                       ChangeTags::addTags(
+                                                               $tags,
+                                                               null,
+                                                               $revId > 0 ? $revId : null,
+                                                               $newId > 0 ? $newId : null
+                                                       );
                                                }
                                        }