Addshore has uploaded a new change for review.

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


Change subject: Make sure ApiResult is ready before using it
......................................................................

Make sure ApiResult is ready before using it

In core the ApiResult is not actually ready to
use until just before execute() is called.

For this reason we can no construct the resultBuilder
in the constructor but instead use method which is
in turn called from within execute()

Bug: 57531

Change-Id: Iec91a8b471738f0958d9e1432fcb9bbf228cf096
---
M repo/includes/api/ApiWikibase.php
M repo/includes/api/CreateClaim.php
M repo/includes/api/EditEntity.php
M repo/includes/api/GetClaims.php
M repo/includes/api/GetEntities.php
M repo/includes/api/LinkTitles.php
M repo/includes/api/MergeItems.php
M repo/includes/api/ModifyEntity.php
M repo/includes/api/RemoveClaims.php
M repo/includes/api/RemoveQualifiers.php
M repo/includes/api/RemoveReferences.php
M repo/includes/api/SetAliases.php
M repo/includes/api/SetClaim.php
M repo/includes/api/SetClaimValue.php
M repo/includes/api/SetDescription.php
M repo/includes/api/SetLabel.php
M repo/includes/api/SetQualifier.php
M repo/includes/api/SetReference.php
M repo/includes/api/SetSiteLink.php
19 files changed, 46 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase 
refs/changes/36/97736/1

diff --git a/repo/includes/api/ApiWikibase.php 
b/repo/includes/api/ApiWikibase.php
index a88cb9c..557ab43 100644
--- a/repo/includes/api/ApiWikibase.php
+++ b/repo/includes/api/ApiWikibase.php
@@ -26,17 +26,11 @@
  * @licence GNU GPL v2+
  * @author John Erling Blad < jeb...@gmail.com >
  * @author Tobias Gritschacher < tobias.gritschac...@wikimedia.de >
+ * @author Adam Shorland
  */
 abstract class ApiWikibase extends \ApiBase {
 
-       protected $resultBuilder;
-
-       public function __construct( $mainModule, $moduleName, $modulePrefix = 
'' ) {
-               parent::__construct( $mainModule, $moduleName, $modulePrefix );
-
-               // todo inject serialization factory to result builder
-               $this->resultBuilder = new ResultBuilder( $this->getResult() );
-       }
+       private $resultBuilder;
 
        /**
         * Wrapper message for single errors
@@ -53,6 +47,16 @@
        protected static $longErrorContextMessage = false;
 
        /**
+        * @return ResultBuilder
+        */
+       public function getResultBuilder() {
+               if( !isset( $this->resultBuilder ) ) {
+                       $this->resultBuilder = new ResultBuilder( 
$this->getResult() );
+               }
+               return $this->resultBuilder;
+       }
+
+       /**
         * @see ApiBase::getPossibleErrors()
         */
        public function getPossibleErrors() {
diff --git a/repo/includes/api/CreateClaim.php 
b/repo/includes/api/CreateClaim.php
index 3bfabf8..ce4dbde 100644
--- a/repo/includes/api/CreateClaim.php
+++ b/repo/includes/api/CreateClaim.php
@@ -61,8 +61,8 @@
                $claim = $claims->getClaimWithGuid( $changeOp->getClaimGuid() );
 
                $this->saveChanges( $entityContent, $summary );
-               $this->resultBuilder->markSuccess();
-               $this->resultBuilder->addClaim( $claim );
+               $this->getResultBuilder()->markSuccess();
+               $this->getResultBuilder()->addClaim( $claim );
 
                wfProfileOut( __METHOD__ );
        }
diff --git a/repo/includes/api/EditEntity.php b/repo/includes/api/EditEntity.php
index 61f8a98..5b456e7 100644
--- a/repo/includes/api/EditEntity.php
+++ b/repo/includes/api/EditEntity.php
@@ -464,15 +464,15 @@
         * @param Entity $entity
         */
        protected function buildResult( Entity $entity ) {
-               $this->resultBuilder->addLabels( $entity->getLabels(), 'entity' 
);
-               $this->resultBuilder->addDescriptions( 
$entity->getDescriptions(), 'entity' );
-               $this->resultBuilder->addAliases( $entity->getAllAliases(), 
'entity' );
+               $this->getResultBuilder()->addLabels( $entity->getLabels(), 
'entity' );
+               $this->getResultBuilder()->addDescriptions( 
$entity->getDescriptions(), 'entity' );
+               $this->getResultBuilder()->addAliases( 
$entity->getAllAliases(), 'entity' );
 
                if ( $entity instanceof Item ) {
-                       $this->resultBuilder->addSiteLinks( 
$entity->getSimpleSiteLinks(), 'entity' );
+                       $this->getResultBuilder()->addSiteLinks( 
$entity->getSimpleSiteLinks(), 'entity' );
                }
 
-               $this->resultBuilder->addClaims( $entity->getClaims(), 'entity' 
);
+               $this->getResultBuilder()->addClaims( $entity->getClaims(), 
'entity' );
        }
 
        /**
diff --git a/repo/includes/api/GetClaims.php b/repo/includes/api/GetClaims.php
index 48b0307..bc7b9f3 100644
--- a/repo/includes/api/GetClaims.php
+++ b/repo/includes/api/GetClaims.php
@@ -55,7 +55,7 @@
                }
 
                $claims = $this->getClaims( $entity, $claimGuid );
-               $this->resultBuilder->addClaims( $claims, null );
+               $this->getResultBuilder()->addClaims( $claims, null );
 
                wfProfileOut( __METHOD__ );
        }
diff --git a/repo/includes/api/GetEntities.php 
b/repo/includes/api/GetEntities.php
index 16fbbb0..5268a52 100644
--- a/repo/includes/api/GetEntities.php
+++ b/repo/includes/api/GetEntities.php
@@ -71,7 +71,7 @@
                        $this->getResult()->setIndexedTagName_internal( array( 
'entities' ), 'entity' );
                }
 
-               $this->resultBuilder->markSuccess( 1 );
+               $this->getResultBuilder()->markSuccess( 1 );
 
                wfProfileOut( __METHOD__ );
        }
@@ -128,7 +128,7 @@
                $siteLinkCache = StoreFactory::getStore()->newSiteLinkCache();
                $siteStore = SiteSQLStore::newInstance();
                return new ItemByTitleHelper(
-                       $this->resultBuilder,
+                       $this->getResultBuilder(),
                        $siteLinkCache,
                        $siteStore,
                        $this->stringNormalizer
@@ -140,7 +140,7 @@
         */
        private function addMissingItemstoResult( $missingItems ){
                foreach( $missingItems as $missingItem ) {
-                       $this->resultBuilder->addMissingEntity( $missingItem );
+                       $this->getResultBuilder()->addMissingEntity( 
$missingItem );
                }
        }
 
@@ -180,7 +180,7 @@
 
                $entityContent = $entityContentFactory->getFromId( $entityId );
                if ( is_null( $entityContent ) ) {
-                       $this->resultBuilder->addMissingEntity( array( 'id' => 
$entityId->getSerialization() ) );
+                       $this->getResultBuilder()->addMissingEntity( array( 
'id' => $entityId->getSerialization() ) );
                        return;
                }
 
diff --git a/repo/includes/api/LinkTitles.php b/repo/includes/api/LinkTitles.php
index e32a760..63ce064 100644
--- a/repo/includes/api/LinkTitles.php
+++ b/repo/includes/api/LinkTitles.php
@@ -109,7 +109,7 @@
                        $this->dieUsage( 'No common item detected, unable to 
link titles' , 'no-common-item' );
                }
 
-               $this->resultBuilder->addSiteLinks( $return, 'entity' );
+               $this->getResultBuilder()->addSiteLinks( $return, 'entity' );
                $status = $this->getAttemptSaveStatus( $itemContent, $summary, 
$flags );
                $this->buildResult( $itemContent, $status );
                wfProfileOut( __METHOD__ );
@@ -160,7 +160,7 @@
                        );
                }
 
-               $this->resultBuilder->markSuccess( $status->isOK() );
+               $this->getResultBuilder()->markSuccess( $status->isOK() );
        }
 
        /**
diff --git a/repo/includes/api/MergeItems.php b/repo/includes/api/MergeItems.php
index 14dfd0c..e382a8f 100644
--- a/repo/includes/api/MergeItems.php
+++ b/repo/includes/api/MergeItems.php
@@ -175,10 +175,10 @@
                        $this->handleSaveStatus( $toStatus );
                        $this->addEntityToOutput( $toItemContent, $toStatus, 
'to' );
 
-                       $this->resultBuilder->markSuccess( 1 );
+                       $this->getResultBuilder()->markSuccess( 1 );
                } else {
                        //todo if the second result is not a success we should 
probably undo the first change
-                       $this->resultBuilder->markSuccess( 0 );
+                       $this->getResultBuilder()->markSuccess( 0 );
                }
        }
 
diff --git a/repo/includes/api/ModifyEntity.php 
b/repo/includes/api/ModifyEntity.php
index 32f9638..ee07dac 100644
--- a/repo/includes/api/ModifyEntity.php
+++ b/repo/includes/api/ModifyEntity.php
@@ -280,7 +280,7 @@
                        $this->addNormalizationInfoToOutput( $params['title'] );
                }
 
-               $this->resultBuilder->markSuccess( 1 );
+               $this->getResultBuilder()->markSuccess( 1 );
        }
 
        protected function addNormalizationInfoToOutput( $title ) {
diff --git a/repo/includes/api/RemoveClaims.php 
b/repo/includes/api/RemoveClaims.php
index a9f57c9..bb83141 100644
--- a/repo/includes/api/RemoveClaims.php
+++ b/repo/includes/api/RemoveClaims.php
@@ -69,8 +69,8 @@
                }
 
                $this->saveChanges( $entityContent, $summary );
-               $this->resultBuilder->markSuccess();
-               $this->resultBuilder->addValue( null, $params['claim'], 
'claims', 'claim' );
+               $this->getResultBuilder()->markSuccess();
+               $this->getResultBuilder()->addValue( null, $params['claim'], 
'claims', 'claim' );
 
                wfProfileOut( __METHOD__ );
        }
diff --git a/repo/includes/api/RemoveQualifiers.php 
b/repo/includes/api/RemoveQualifiers.php
index cd2789a..c79f8f8 100644
--- a/repo/includes/api/RemoveQualifiers.php
+++ b/repo/includes/api/RemoveQualifiers.php
@@ -54,7 +54,7 @@
                }
 
                $this->saveChanges( $entityContent, $summary );
-               $this->resultBuilder->markSuccess();
+               $this->getResultBuilder()->markSuccess();
 
                wfProfileOut( __METHOD__ );
        }
diff --git a/repo/includes/api/RemoveReferences.php 
b/repo/includes/api/RemoveReferences.php
index 2ab9bb3..7600a69 100644
--- a/repo/includes/api/RemoveReferences.php
+++ b/repo/includes/api/RemoveReferences.php
@@ -58,7 +58,7 @@
                }
 
                $this->saveChanges( $entityContent, $summary );
-               $this->resultBuilder->markSuccess();
+               $this->getResultBuilder()->markSuccess();
 
                wfProfileOut( __METHOD__ );
        }
diff --git a/repo/includes/api/SetAliases.php b/repo/includes/api/SetAliases.php
index c933963..58f3bd7 100644
--- a/repo/includes/api/SetAliases.php
+++ b/repo/includes/api/SetAliases.php
@@ -92,7 +92,7 @@
 
                $aliases = $entity->getAliases( $language );
                if ( count( $aliases ) ) {
-                       $this->resultBuilder->addAliases( array( $language => 
$aliases ), 'entity' );
+                       $this->getResultBuilder()->addAliases( array( $language 
=> $aliases ), 'entity' );
                }
 
                wfProfileOut( __METHOD__ );
diff --git a/repo/includes/api/SetClaim.php b/repo/includes/api/SetClaim.php
index 551d236..5cb7b88 100644
--- a/repo/includes/api/SetClaim.php
+++ b/repo/includes/api/SetClaim.php
@@ -67,8 +67,8 @@
                }
 
                $this->saveChanges( $entityContent, $summary );
-               $this->resultBuilder->markSuccess();
-               $this->resultBuilder->addClaim( $claim );
+               $this->getResultBuilder()->markSuccess();
+               $this->getResultBuilder()->addClaim( $claim );
        }
 
        /**
diff --git a/repo/includes/api/SetClaimValue.php 
b/repo/includes/api/SetClaimValue.php
index 737f844..fc9a9a7 100644
--- a/repo/includes/api/SetClaimValue.php
+++ b/repo/includes/api/SetClaimValue.php
@@ -58,8 +58,8 @@
                }
 
                $this->saveChanges( $entityContent, $summary );
-               $this->resultBuilder->markSuccess();
-               $this->resultBuilder->addClaim( $claim );
+               $this->getResultBuilder()->markSuccess();
+               $this->getResultBuilder()->addClaim( $claim );
 
                wfProfileOut( __METHOD__ );
        }
diff --git a/repo/includes/api/SetDescription.php 
b/repo/includes/api/SetDescription.php
index 19c2381..baac2e9 100644
--- a/repo/includes/api/SetDescription.php
+++ b/repo/includes/api/SetDescription.php
@@ -42,7 +42,7 @@
                $this->getChangeOp( $params )->apply( $entity, $summary );
                $descriptions = array( $language => ( $entity->getDescription( 
$language ) !== false ) ? $entity->getDescription( $language ) : "" );
 
-               $this->resultBuilder->addDescriptions( $descriptions, 'entity' 
);
+               $this->getResultBuilder()->addDescriptions( $descriptions, 
'entity' );
 
                wfProfileOut( __METHOD__ );
                return $summary;
diff --git a/repo/includes/api/SetLabel.php b/repo/includes/api/SetLabel.php
index 03ae6a0..554e87f 100644
--- a/repo/includes/api/SetLabel.php
+++ b/repo/includes/api/SetLabel.php
@@ -42,7 +42,7 @@
                $this->getChangeOp( $params )->apply( $entity, $summary );
                $labels = array( $language => ( $entity->getLabel( $language ) 
!== false ) ? $entity->getLabel( $language ) : "" );
 
-               $this->resultBuilder->addLabels( $labels, 'entity' );
+               $this->getResultBuilder()->addLabels( $labels, 'entity' );
 
                wfProfileOut( __METHOD__ );
                return $summary;
diff --git a/repo/includes/api/SetQualifier.php 
b/repo/includes/api/SetQualifier.php
index eb51208..9543720 100644
--- a/repo/includes/api/SetQualifier.php
+++ b/repo/includes/api/SetQualifier.php
@@ -57,8 +57,8 @@
                }
 
                $this->saveChanges( $entityContent, $summary );
-               $this->resultBuilder->markSuccess();
-               $this->resultBuilder->addClaim( $claim );
+               $this->getResultBuilder()->markSuccess();
+               $this->getResultBuilder()->addClaim( $claim );
 
                wfProfileOut( __METHOD__ );
        }
diff --git a/repo/includes/api/SetReference.php 
b/repo/includes/api/SetReference.php
index 66fec4a..b13c9f5 100644
--- a/repo/includes/api/SetReference.php
+++ b/repo/includes/api/SetReference.php
@@ -74,8 +74,8 @@
                }
 
                $this->saveChanges( $entityContent, $summary );
-               $this->resultBuilder->markSuccess();
-               $this->resultBuilder->addReference( $newReference );
+               $this->getResultBuilder()->markSuccess();
+               $this->getResultBuilder()->addReference( $newReference );
 
                wfProfileOut( __METHOD__ );
        }
diff --git a/repo/includes/api/SetSiteLink.php 
b/repo/includes/api/SetSiteLink.php
index 3a3cb37..a7b1107 100644
--- a/repo/includes/api/SetSiteLink.php
+++ b/repo/includes/api/SetSiteLink.php
@@ -67,12 +67,12 @@
                        if ( $item->hasLinkToSite( $linksite ) ) {
                                $link = $item->getSimpleSiteLink( $linksite );
                                $this->getChangeOp( $params )->apply( $item, 
$summary );
-                               $this->resultBuilder->addSiteLinks( array( 
$link ), 'entity', array( 'removed' ) );
+                               $this->getResultBuilder()->addSiteLinks( array( 
$link ), 'entity', array( 'removed' ) );
                        }
                } else {
                        $this->getChangeOp( $params )->apply( $item, $summary );
                        $link = $item->getSimpleSiteLink( $linksite );
-                       $this->resultBuilder->addSiteLinks( array( $link ), 
'entity', array( 'url' ) );
+                       $this->getResultBuilder()->addSiteLinks( array( $link 
), 'entity', array( 'url' ) );
                }
 
                wfProfileOut( __METHOD__ );

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iec91a8b471738f0958d9e1432fcb9bbf228cf096
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <addshorew...@gmail.com>

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

Reply via email to