jenkins-bot has submitted this change and it was merged.
Change subject: ApiResponse: return success status or die on EventLogging fail.
......................................................................
ApiResponse: return success status or die on EventLogging fail.
Added tests for invalid claims
Change-Id: Ia6fbb60d56ab9e50619e761b1210ba8933263642
---
M includes/api/ApiResponse.php
M tests/phpunit/api/ApiResponseTest.php
2 files changed, 105 insertions(+), 9 deletions(-)
Approvals:
Phuedx: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/api/ApiResponse.php b/includes/api/ApiResponse.php
index 875d134..6fb54e7 100644
--- a/includes/api/ApiResponse.php
+++ b/includes/api/ApiResponse.php
@@ -53,12 +53,20 @@
'value' => $claim['value'],
'response' => $claim['correct'],
);
- $eventLogger->logEvent(
+ $result = $eventLogger->logEvent(
self::SCHEMA,
self::SCHEMA_REV_ID,
$event
);
+ if ( !$result ) {
+ $this->dieUsage(
+ 'Logging claims failed. Be sure
Extension:EventLogging is setup correctly.',
+ 'loggingfailed'
+ );
+ }
}
+
+ $this->getResult()->addValue( 'query', $this->getModuleName(),
'success' );
}
/**
@@ -79,7 +87,7 @@
|| !isset( $claim['valueid'] )
|| !isset( $claim['value'] )
) {
- $this->dieUsage( 'Invalid claim submitted',
'invalidclaim' );
+ $this->dieUsage( 'Invalid claim: ' . json_encode(
$claim ), 'invalidclaim' );
}
}
diff --git a/tests/phpunit/api/ApiResponseTest.php
b/tests/phpunit/api/ApiResponseTest.php
index ca8d8ae..d209d50 100644
--- a/tests/phpunit/api/ApiResponseTest.php
+++ b/tests/phpunit/api/ApiResponseTest.php
@@ -59,7 +59,9 @@
protected function setUp() {
parent::setUp();
- $this->setMwGlobals( 'wgAPIModules', array( 'wikigrokresponse'
=> 'Tests\WikiGrok\Api\StubApiResponse' ) );
+ $this->setMwGlobals( 'wgAPIModules', array(
+ 'wikigrokresponse' =>
'Tests\WikiGrok\Api\StubApiResponse' )
+ );
$this->eventLogger = $this->getMock( 'WikiGrok\EventLogger' );
$this->user = new StubUser( 1234 );
@@ -72,10 +74,7 @@
);
}
- /**
- * @expectedException UsageException
- */
- public function testExecuteThrowsWhenThereAreNoClaims() {
+ private function runExecuteWithClaims( $claims ) {
$request = new FauxRequest( array(
'page_id' => 1,
'user_token' => '987654321fedcba',
@@ -83,11 +82,98 @@
'subject' => 'Vessels',
'task_token' => 'abcdef0123456789',
'task_type' => 'mobile a',
- 'claims' => '[]',
+ 'claims' => json_encode( $claims ),
'mobile_mode' => 'beta',
) );
$this->context->setRequest( $request );
$this->api->execute();
+ }
+
+ /**
+ * @expectedException UsageException
+ */
+ public function testExecuteThrowsWhenThereAreNoClaims() {
+ self::runExecuteWithClaims( array() );
+ }
+
+ /**
+ * @expectedException UsageException
+ */
+ public function testExecuteThrowsWhenClaimMissingPropId() {
+ $claims = array(
+ array(
+ // Missing propid
+ 'prop' => 'occupation',
+ 'valueid' => 'Q3423',
+ 'value' => 'insurance broker',
+ 'correct' => true,
+ )
+ );
+ self::runExecuteWithClaims( $claims );
+ }
+
+ /**
+ * @expectedException UsageException
+ */
+ public function testExecuteThrowsWhenClaimMissingProp() {
+ $claims = array(
+ array(
+ // Missing prop
+ 'propid' => 'P103',
+ 'valueid' => 'Q3423',
+ 'value' => 'insurance broker',
+ 'correct' => true,
+ )
+ );
+ self::runExecuteWithClaims( $claims );
+ }
+
+ /**
+ * @expectedException UsageException
+ */
+ public function testExecuteThrowsWhenClaimMissingValueId() {
+ $claims = array(
+ array(
+ // Missing valueid
+ 'propid' => 'P103',
+ 'prop' => 'occupation',
+ 'value' => 'insurance broker',
+ 'correct' => true,
+ )
+ );
+ self::runExecuteWithClaims( $claims );
+ }
+
+ /**
+ * @expectedException UsageException
+ */
+ public function testExecuteThrowsWhenClaimMissingValue() {
+ $claims = array(
+ array(
+ // Missing value
+ 'propid' => 'P103',
+ 'prop' => 'occupation',
+ 'valueid' => 'Q3423',
+ 'correct' => true,
+ )
+ );
+ self::runExecuteWithClaims( $claims );
+ }
+
+ /**
+ * @expectedException UsageException
+ */
+ public function testExecuteThrowsWhenClaimMissingCorrect() {
+ $claims = array(
+ array(
+ // Missing correct
+ 'propid' => 'P103',
+ 'prop' => 'occupation',
+ 'valueid' => 'Q3423',
+ 'value' => 'insurance broker',
+ )
+ );
+ self::runExecuteWithClaims( $claims );
}
/**
@@ -104,7 +190,8 @@
ApiResponse::SCHEMA,
ApiResponse::SCHEMA_REV_ID,
$expectedEvent
- );
+ )
+ ->will( $this->returnValue( true ) );
}
$this->api->execute();
@@ -178,4 +265,5 @@
),
);
}
+
}
--
To view, visit https://gerrit.wikimedia.org/r/172467
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia6fbb60d56ab9e50619e761b1210ba8933263642
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/WikiGrok
Gerrit-Branch: master
Gerrit-Owner: Robmoen <[email protected]>
Gerrit-Reviewer: Kaldari <[email protected]>
Gerrit-Reviewer: Phuedx <[email protected]>
Gerrit-Reviewer: Robmoen <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
