Merge pull request #5850 from owncloud/oc_api_tests

Add unit tests for OC_API::mergeResponses and fix error with api returning incorrect status codes.

2 files changed, 174 insertions, 17 deletions

diff --git a/lib/private/api.php b/lib/private/api.php
index eac4a825e07..03d7b7382a5 100644
--- a/lib/private/api.php
+++ b/lib/private/api.php
@@ -96,6 +96,7 @@ class OC_API {
 				$responses[] = array(
 					'app' => $action['app'],
 					'response' => new OC_OCS_Result(null, OC_API::RESPOND_UNAUTHORISED, 'Unauthorised'),
+					'shipped' => OC_App::isShipped($action['app']),
 					);
 				continue;
 			}
@@ -103,6 +104,7 @@ class OC_API {
 				$responses[] = array(
 					'app' => $action['app'],
 					'response' => new OC_OCS_Result(null, OC_API::RESPOND_NOT_FOUND, 'Api method not found'),
+					'shipped' => OC_App::isShipped($action['app']),
 					);
 				continue;
 			}
@@ -110,6 +112,7 @@ class OC_API {
 			$responses[] = array(
 				'app' => $action['app'],
 				'response' => call_user_func($action['action'], $parameters),
+				'shipped' => OC_App::isShipped($action['app']),
 				);
 		}
 		$response = self::mergeResponses($responses);
@@ -127,7 +130,7 @@ class OC_API {
 	 * merge the returned result objects into one response
 	 * @param array $responses
 	 */
-	private static function mergeResponses($responses) {
+	public static function mergeResponses($responses) {
 		$response = array();
 		// Sort into shipped and thirdparty
 		$shipped = array(
@@ -140,50 +143,76 @@ class OC_API {
 			);
 
 		foreach($responses as $response) {
-			if(OC_App::isShipped($response['app']) || ($response['app'] === 'core')) {
+			if($response['shipped'] || ($response['app'] === 'core')) {
 				if($response['response']->succeeded()) {
-					$shipped['succeeded'][$response['app']] = $response['response'];
+					$shipped['succeeded'][$response['app']] = $response;
 				} else {
-					$shipped['failed'][$response['app']] = $response['response'];
+					$shipped['failed'][$response['app']] = $response;
 				}
 			} else {
 				if($response['response']->succeeded()) {
-					$thirdparty['succeeded'][$response['app']] = $response['response'];
+					$thirdparty['succeeded'][$response['app']] = $response;
 				} else {
-					$thirdparty['failed'][$response['app']] = $response['response'];
+					$thirdparty['failed'][$response['app']] = $response;
 				}
 			}
 		}
 
 		// Remove any error responses if there is one shipped response that succeeded
-		if(!empty($shipped['succeeded'])) {
-			$responses = array_merge($shipped['succeeded'], $thirdparty['succeeded']);
-		} else if(!empty($shipped['failed'])) {
+		if(!empty($shipped['failed'])) {
 			// Which shipped response do we use if they all failed?
 			// They may have failed for different reasons (different status codes)
 			// Which reponse code should we return?
 			// Maybe any that are not OC_API::RESPOND_SERVER_ERROR
-			$response = reset($shipped['failed']);
+			// Merge failed responses if more than one
+			$data = array();
+			$meta = array();
+			foreach($shipped['failed'] as $failure) {
+				$data = array_merge_recursive($data, $failure['response']->getData());
+			}
+			$picked = reset($shipped['failed']);
+			$code = $picked['response']->getStatusCode();
+			$response = new OC_OCS_Result($data, $code);
 			return $response;
+		} elseif(!empty($shipped['succeeded'])) {
+			$responses = array_merge($shipped['succeeded'], $thirdparty['succeeded']);
 		} elseif(!empty($thirdparty['failed'])) {
-			// Return the third party failure result
-			$response = reset($thirdparty['failed']);
+			// Merge failed responses if more than one
+			$data = array();
+			$meta = array();
+			foreach($thirdparty['failed'] as $failure) {
+				$data = array_merge_recursive($data, $failure['response']->getData());
+			}
+			$picked = reset($thirdparty['failed']);
+			$code = $picked['response']->getStatusCode();
+			$response = new OC_OCS_Result($data, $code);
 			return $response;
 		} else {
-			$responses = array_merge($shipped['succeeded'], $thirdparty['succeeded']);
+			$responses = $thirdparty['succeeded'];
 		}
 		// Merge the successful responses
 		$meta = array();
 		$data = array();
 
 		foreach($responses as $app => $response) {
-			if(OC_App::isShipped($app)) {
-				$data = array_merge_recursive($response->getData(), $data);
+			if($response['shipped']) {
+				$data = array_merge_recursive($response['response']->getData(), $data);
 			} else {
-				$data = array_merge_recursive($data, $response->getData());
+				$data = array_merge_recursive($data, $response['response']->getData());
 			}
+			$codes[] = $response['response']->getStatusCode();
 		}
-		$result = new OC_OCS_Result($data, 100);
+
+		// Use any non 100 status codes
+		$statusCode = 100;
+		foreach($codes as $code) {
+			if($code != 100) {
+				$statusCode = $code;
+				break;
+			}
+		}
+
+		$result = new OC_OCS_Result($data, $statusCode);
 		return $result;
 	}
 	
diff --git a/tests/lib/api.php b/tests/lib/api.php
new file mode 100644
index 00000000000..95d75f15311
--- /dev/null
+++ b/tests/lib/api.php
@@ -0,0 +1,128 @@
+<?php
+/**
+ * Copyright (c) 2013 Tom Needham <tom@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+class Test_API extends PHPUnit_Framework_TestCase {
+	
+	// Helps build a response variable
+	function buildResponse($shipped, $data, $code) {
+		return array(
+			'shipped' => $shipped,
+			'response' => new OC_OCS_Result($data, $code),
+			'app' => uniqid('testapp_', true),
+			);
+	}
+
+	// Validate details of the result
+	function checkResult($result, $success) {
+		// Check response is of correct type
+		$this->assertInstanceOf('OC_OCS_Result', $result);
+		// Check if it succeeded
+		/** @var $result OC_OCS_Result */
+		$this->assertEquals($success, $result->succeeded());
+	}
+
+	function dataProviderTestOneResult() {
+		return array(
+			array(100, true),
+			array(101, true),
+			array(997, false),
+		);
+	}
+
+	/**
+	 * @dataProvider dataProviderTestOneResult
+	 *
+	 * @param $statusCode
+	 * @param $succeeded
+	 */
+	public function testOneResult($statusCode, $succeeded) {
+		// Setup some data arrays
+		$data1 = array(
+			'users' => array(
+				'tom' => array(
+					'key' => 'value',
+				),
+				'frank' => array(
+					'key' => 'value',
+				),
+			));
+
+		// Test merging one success result
+		$response = $this->buildResponse(true, $data1, $statusCode);
+		$result = OC_API::mergeResponses(array($response));
+		$this->assertEquals($response['response'], $result);
+		$this->checkResult($result, $succeeded);
+	}
+
+	function dataProviderTestMergeResponses() {
+		return array(
+			// Two shipped success results
+			array(true, 100, true, 100, true),
+			// Two shipped results, one success and one failure
+			array(true, 100, true, 997, false),
+			// Two shipped results, both failure
+			array(true, 997, true, 997, false),
+			// Two third party success results
+			array(false, 100, false, 100, true),
+			// Two third party results, one success and one failure
+			array(false, 100, false, 997, false),
+			// Two third party results, both failure
+			array(false, 997, false, 997, false),
+			// One of each, both success
+			array(false, 100, true, 100, true),
+			array(true, 100, false, 100, true),
+			// One of each, both failure
+			array(false, 997, true, 997, false),
+			// One of each, shipped success
+			array(false, 997, true, 100, true),
+			// One of each, third party success
+			array(false, 100, true, 997, false),
+		);
+	}
+	/**
+	 * @dataProvider dataProviderTestMergeResponses
+	 *
+	 * Test the merging of multiple responses
+	 * @param $statusCode1
+	 * @param $statusCode2
+	 * @param $succeeded
+	 */
+	public function testMultipleMergeResponses($shipped1, $statusCode1, $shipped2, $statusCode2, $succeeded){
+		// Tests that app responses are merged correctly
+		// Setup some data arrays
+		$data1 = array(
+			'users' => array(
+				'tom' => array(
+					'key' => 'value',
+				),
+				'frank' => array(
+					'key' => 'value',
+				),
+			));
+
+		$data2 = array(
+			'users' => array(
+				'tom' => array(
+					'key' => 'newvalue',
+				),
+				'jan' => array(
+					'key' => 'value',
+				),
+			));
+
+		// Two shipped success results
+		$result = OC_API::mergeResponses(array(
+			$this->buildResponse($shipped1, $data1, $statusCode1),
+			$this->buildResponse($shipped2, $data2, $statusCode2),
+		));
+		$this->checkResult($result, $succeeded);
+		$resultData = $result->getData();
+		$this->assertArrayHasKey('jan', $resultData['users']);
+	}
+
+}