From 243e72474bd0c87b8647efe1edeb1116a0a5fcd0 Mon Sep 17 00:00:00 2001 From: Frédéric Guillot Date: Wed, 17 Sep 2014 14:47:41 +0200 Subject: Improve board API calls --- app/Controller/Board.php | 2 +- app/Model/Board.php | 43 +++--- docs/api-json-rpc.markdown | 310 ++++++++++++++++++++++++++++++++++-------- jsonrpc.php | 13 +- tests/functionals/ApiTest.php | 26 +++- tests/units/BoardTest.php | 86 ++++++++++++ vendor/JsonRPC/Client.php | 20 ++- vendor/JsonRPC/Server.php | 137 +++++++++++++------ 8 files changed, 513 insertions(+), 124 deletions(-) diff --git a/app/Controller/Board.php b/app/Controller/Board.php index 2ee43c76..f643408d 100644 --- a/app/Controller/Board.php +++ b/app/Controller/Board.php @@ -328,7 +328,7 @@ class Board extends Base if ($valid) { - if ($this->board->add($data)) { + if ($this->board->addColumn($project['id'], $data['title'])) { $this->session->flash(t('Board updated successfully.')); $this->response->redirect('?controller=board&action=edit&project_id='.$project['id']); } diff --git a/app/Model/Board.php b/app/Model/Board.php index f8df4069..ac9cbdf9 100644 --- a/app/Model/Board.php +++ b/app/Model/Board.php @@ -83,13 +83,19 @@ class Board extends Base * Add a new column to the board * * @access public - * @param array $values ['title' => X, 'project_id' => X] + * @param integer $project_id Project id + * @param string $title Column title + * @param integer $task_limit Task limit * @return boolean */ - public function add(array $values) + public function addColumn($project_id, $title, $task_limit = 0) { - $values['position'] = $this->getLastColumnPosition($values['project_id']) + 1; - return $this->db->table(self::TABLE)->save($values); + return $this->db->table(self::TABLE)->save(array( + 'project_id' => $project_id, + 'title' => $title, + 'task_limit' => $task_limit, + 'position' => $this->getLastColumnPosition($project_id) + 1, + )); } /** @@ -101,17 +107,18 @@ class Board extends Base */ public function update(array $values) { - $this->db->startTransaction(); + $columns = array(); foreach (array('title', 'task_limit') as $field) { - foreach ($values[$field] as $column_id => $field_value) { + foreach ($values[$field] as $column_id => $value) { + $columns[$column_id][$field] = $value; + } + } - if ($field === 'task_limit' && empty($field_value)) { - $field_value = 0; - } + $this->db->startTransaction(); - $this->updateColumn($column_id, array($field => $field_value)); - } + foreach ($columns as $column_id => $values) { + $this->updateColumn($column_id, $values['title'], (int) $values['task_limit']); } $this->db->closeTransaction(); @@ -123,13 +130,17 @@ class Board extends Base * Update a column * * @access public - * @param integer $column_id Column id - * @param array $values Form values + * @param integer $column_id Column id + * @param string $title Column title + * @param integer $task_limit Task limit * @return boolean */ - public function updateColumn($column_id, array $values) + public function updateColumn($column_id, $title, $task_limit = 0) { - return $this->db->table(self::TABLE)->eq('id', $column_id)->update($values); + return $this->db->table(self::TABLE)->eq('id', $column_id)->update(array( + 'title' => $title, + 'task_limit' => $task_limit, + )); } /** @@ -195,7 +206,7 @@ class Board extends Base * * @access public * @param integer $project_id Project id - * @param array $filters + * @param array $filters * @return array */ public function get($project_id, array $filters = array()) diff --git a/docs/api-json-rpc.markdown b/docs/api-json-rpc.markdown index d32da9af..61ea9d85 100644 --- a/docs/api-json-rpc.markdown +++ b/docs/api-json-rpc.markdown @@ -431,14 +431,12 @@ Response example: } ``` +### getAllowedUsers - -### getBoard - -- Purpose: **Get all necessary information to display a board** +- Purpose: **Get allowed users for a given project** - Parameters: **project_id** (integer) -- Result on success: **board properties** -- Result on failure: **null** +- Result on success: Key/value pair of user_id and username +- Result on failure: **false** Request example: @@ -452,12 +450,12 @@ Response example: ``` -### getColumns +### revokeUser -- Purpose: **Get all columns information for a given project** -- Parameters: **project_id** (integer) -- Result on success: **columns properties** -- Result on failure: **null** +- Purpose: **Revoke user access for a given project** +- Parameters: **project_id** (integer), **user_id** (integer) +- Result on success: **true** +- Result on failure: **false** Request example: @@ -471,10 +469,10 @@ Response example: ``` -### moveColumnUp +### allowUser -- Purpose: **Move up the column position** -- Parameters: **project_id** (integer), **column_id** (integer) +- Purpose: **Grant user access for a given project** +- Parameters: **project_id** (integer), **user_id** (integer) - Result on success: **true** - Result on failure: **false** @@ -490,137 +488,343 @@ Response example: ``` -### moveColumnDown -- Purpose: **Move down the column position** -- Parameters: **project_id** (integer), **column_id** (integer) -- Result on success: **true** -- Result on failure: **false** +### getBoard + +- Purpose: **Get all necessary information to display a board** +- Parameters: + - **project_id** (integer, required) +- Result on success: **board properties** +- Result on failure: **empty list** Request example: ```json - +{ + "jsonrpc": "2.0", + "method": "getBoard", + "id": 1627282648, + "params": [ + 1 + ] +} ``` Response example: ```json - +{ + "jsonrpc": "2.0", + "id": 1627282648, + "result": [ + { + "id": "1", + "title": "Backlog", + "position": "1", + "project_id": "1", + "task_limit": "0", + "tasks": [] + }, + { + "id": "2", + "title": "Ready", + "position": "2", + "project_id": "1", + "task_limit": "0", + "tasks": [] + }, + { + "id": "3", + "title": "Work in progress", + "position": "3", + "project_id": "1", + "task_limit": "0", + "tasks": [ + { + "nb_comments": "0", + "nb_files": "0", + "nb_subtasks": "1", + "nb_completed_subtasks": "0", + "id": "1", + "title": "Task with comment", + "description": "", + "date_creation": "1410952872", + "date_modification": "1410952872", + "date_completed": null, + "date_due": "0", + "color_id": "red", + "project_id": "1", + "column_id": "3", + "owner_id": "1", + "creator_id": "0", + "position": "1", + "is_active": "1", + "score": "0", + "category_id": "0", + "assignee_username": "admin", + "assignee_name": null + } + ] + }, + ... + ] +} ``` -### updateColumn +### getColumns -- Purpose: **Update column properties** -- Parameters: **column_id** (integer), **values** (**title** string, **task_limit** integer) -- Result on success: **true** -- Result on failure: **false** +- Purpose: **Get all columns information for a given project** +- Parameters: + - **project_id** (integer, required) +- Result on success: **columns properties** +- Result on failure: **empty list** Request example: ```json - +{ + "jsonrpc": "2.0", + "method": "getColumns", + "id": 887036325, + "params": [ + 1 + ] +} ``` Response example: ```json +{ + "jsonrpc": "2.0", + "id": 887036325, + "result": [ + { + "id": "1", + "title": "Backlog", + "position": "1", + "project_id": "1", + "task_limit": "0" + }, + { + "id": "2", + "title": "Ready", + "position": "2", + "project_id": "1", + "task_limit": "0" + }, + { + "id": "3", + "title": "Work in progress", + "position": "3", + "project_id": "1", + "task_limit": "0" + } + ] +} +``` +### getColumn + +- Purpose: **Get a single column** +- Parameters: + - **column_id** (integer, required) +- Result on success: **column properties** +- Result on failure: **null** + +Request example: + +```json +{ + "jsonrpc": "2.0", + "method": "getColumn", + "id": 1242049935, + "params": [ + 2 + ] +} ``` -### addColumn +Response example: -- Purpose: **Add a new column** -- Parameters: **project_id** (integer), **values** (**title** string, **task_limit** integer) +```json +{ + "jsonrpc": "2.0", + "id": 1242049935, + "result": { + "id": "2", + "title": "Youpi", + "position": "2", + "project_id": "1", + "task_limit": "5" + } +} +``` + +### moveColumnUp + +- Purpose: **Move up the column position** +- Parameters: + - **project_id** (integer, required) + - **column_id** (integer, required) - Result on success: **true** - Result on failure: **false** Request example: ```json - +{ + "jsonrpc": "2.0", + "method": "moveColumnUp", + "id": 99275573, + "params": [ + 1, + 2 + ] +} ``` Response example: ```json - +{ + "jsonrpc": "2.0", + "id": 99275573, + "result": true +} ``` -### removeColumn +### moveColumnDown -- Purpose: **Remove a column** -- Parameters: **column_id** (integer) +- Purpose: **Move down the column position** +- Parameters: + - **project_id** (integer, required) + - **column_id** (integer, required) - Result on success: **true** - Result on failure: **false** Request example: ```json - +{ + "jsonrpc": "2.0", + "method": "moveColumnDown", + "id": 957090649, + "params": { + "project_id": 1, + "column_id": 2 + } +} ``` Response example: ```json - +{ + "jsonrpc": "2.0", + "id": 957090649, + "result": true +} ``` -### getAllowedUsers +### updateColumn -- Purpose: **Get allowed users for a given project** -- Parameters: **project_id** (integer) -- Result on success: Key/value pair of user_id and username +- Purpose: **Update column properties** +- Parameters: + - **column_id** (integer, required) + - **title** (string, required) + - **task_limit** (integer, optional) +- Result on success: **true** - Result on failure: **false** Request example: ```json - +{ + "jsonrpc": "2.0", + "method": "updateColumn", + "id": 480740641, + "params": [ + 2, + "Boo", + 5 + ] +} ``` Response example: ```json - +{ + "jsonrpc": "2.0", + "id": 480740641, + "result": true +} ``` -### revokeUser +### addColumn -- Purpose: **Revoke user access for a given project** -- Parameters: **project_id** (integer), **user_id** (integer) +- Purpose: **Add a new column** +- Parameters: + - **project_id** (integer, required) + - **title** (string, required) + - **task_limit** (integer, optional) - Result on success: **true** - Result on failure: **false** Request example: ```json - +{ + "jsonrpc": "2.0", + "method": "addColumn", + "id": 638544704, + "params": [ + 1, + "Boo" + ] +} ``` Response example: ```json - +{ + "jsonrpc": "2.0", + "id": 638544704, + "result": true +} ``` -### allowUser +### removeColumn -- Purpose: **Grant user access for a given project** -- Parameters: **project_id** (integer), **user_id** (integer) +- Purpose: **Remove a column** +- Parameters: + - **column_id** (integer, required) - Result on success: **true** - Result on failure: **false** Request example: ```json - +{ + "jsonrpc": "2.0", + "method": "removeColumn", + "id": 1433237746, + "params": [ + 1 + ] +} ``` Response example: ```json - +{ + "jsonrpc": "2.0", + "id": 1433237746, + "result": true +} ``` diff --git a/jsonrpc.php b/jsonrpc.php index 1a411718..72c9c297 100644 --- a/jsonrpc.php +++ b/jsonrpc.php @@ -114,6 +114,10 @@ $server->register('getColumns', function($project_id) use ($board) { return $board->getColumns($project_id); }); +$server->register('getColumn', function($column_id) use ($board) { + return $board->getColumn($column_id); +}); + $server->register('moveColumnUp', function($project_id, $column_id) use ($board) { return $board->moveUp($project_id, $column_id); }); @@ -122,13 +126,12 @@ $server->register('moveColumnDown', function($project_id, $column_id) use ($boar return $board->moveDown($project_id, $column_id); }); -$server->register('updateColumn', function($column_id, array $values) use ($board) { - return $board->updateColumn($column_id, $values); +$server->register('updateColumn', function($column_id, $title, $task_limit = 0) use ($board) { + return $board->updateColumn($column_id, $title, $task_limit); }); -$server->register('addColumn', function($project_id, array $values) use ($board) { - $values += array('project_id' => $project_id); - return $board->add($values); +$server->register('addColumn', function($project_id, $title, $task_limit = 0) use ($board) { + return $board->addColumn($project_id, $title, $task_limit); }); $server->register('removeColumn', function($column_id) use ($board) { diff --git a/tests/functionals/ApiTest.php b/tests/functionals/ApiTest.php index 04b22856..13c25a7b 100644 --- a/tests/functionals/ApiTest.php +++ b/tests/functionals/ApiTest.php @@ -38,6 +38,7 @@ class Api extends PHPUnit_Framework_TestCase { $this->client = new JsonRPC\Client(API_URL); $this->client->authentication('jsonrpc', API_KEY); + //$this->client->debug = true; } private function getTaskId() @@ -126,7 +127,7 @@ class Api extends PHPUnit_Framework_TestCase public function testUpdateColumn() { - $this->assertTrue($this->client->updateColumn(4, array('title' => 'Boo', 'task_limit' => 2))); + $this->assertTrue($this->client->updateColumn(4, 'Boo', 2)); $columns = $this->client->getColumns(1); $this->assertTrue(is_array($columns)); @@ -136,7 +137,7 @@ class Api extends PHPUnit_Framework_TestCase public function testAddColumn() { - $this->assertTrue($this->client->addColumn(1, array('title' => 'New column'))); + $this->assertTrue($this->client->addColumn(1, 'New column')); $columns = $this->client->getColumns(1); $this->assertTrue(is_array($columns)); @@ -164,14 +165,20 @@ class Api extends PHPUnit_Framework_TestCase ); $this->assertTrue($this->client->execute('createTask', $task)); + } + /** + * @expectedException BadFunctionCallException + */ + public function testCreateTaskWithBadParams() + { $task = array( 'title' => 'Task #1', 'color_id' => 'blue', 'owner_id' => 1, ); - $this->assertNull($this->client->createTask($task)); + $this->client->createTask($task); } public function testGetTask() @@ -236,7 +243,13 @@ class Api extends PHPUnit_Framework_TestCase ); $this->assertTrue($this->client->execute('createUser', $user)); + } + /** + * @expectedException BadFunctionCallException + */ + public function testCreateUserWithBadParams() + { $user = array( 'name' => 'Titi', 'password' => '123456', @@ -481,9 +494,14 @@ class Api extends PHPUnit_Framework_TestCase ); $this->assertFalse($this->client->execute('createCategory', $category)); + } + /** + * @expectedException BadFunctionCallException + */ + public function testCategoryCreationWithBadParams() + { // Missing project id - $category = array( 'name' => 'Category', ); diff --git a/tests/units/BoardTest.php b/tests/units/BoardTest.php index 35e102b4..21d65543 100644 --- a/tests/units/BoardTest.php +++ b/tests/units/BoardTest.php @@ -39,6 +39,92 @@ class BoardTest extends Base $this->assertEquals('column #2', $columns[6]); } + public function testGetBoard() + { + $p = new Project($this->registry); + $b = new Board($this->registry); + + $this->assertEquals(1, $p->create(array('name' => 'UnitTest1'))); + + $board = $b->get(1); + $this->assertNotEmpty($board); + $this->assertEquals(4, count($board)); + $this->assertTrue(array_key_exists('tasks', $board[2])); + $this->assertTrue(array_key_exists('title', $board[2])); + } + + public function testGetColumn() + { + $p = new Project($this->registry); + $b = new Board($this->registry); + + $this->assertEquals(1, $p->create(array('name' => 'UnitTest1'))); + + $column = $b->getColumn(3); + $this->assertNotEmpty($column); + $this->assertEquals('Work in progress', $column['title']); + + $column = $b->getColumn(33); + $this->assertEmpty($column); + } + + public function testRemoveColumn() + { + $p = new Project($this->registry); + $b = new Board($this->registry); + + $this->assertEquals(1, $p->create(array('name' => 'UnitTest1'))); + $this->assertTrue($b->removeColumn(3)); + $this->assertFalse($b->removeColumn(322)); + + $columns = $b->getColumns(1); + $this->assertTrue(is_array($columns)); + $this->assertEquals(3, count($columns)); + } + + public function testUpdateColumn() + { + $p = new Project($this->registry); + $b = new Board($this->registry); + + $this->assertEquals(1, $p->create(array('name' => 'UnitTest1'))); + + $this->assertTrue($b->updateColumn(3, 'blah', 5)); + $this->assertTrue($b->updateColumn(2, 'boo')); + + $column = $b->getColumn(3); + $this->assertNotEmpty($column); + $this->assertEquals('blah', $column['title']); + $this->assertEquals(5, $column['task_limit']); + + $column = $b->getColumn(2); + $this->assertNotEmpty($column); + $this->assertEquals('boo', $column['title']); + $this->assertEquals(0, $column['task_limit']); + } + + public function testAddColumn() + { + $p = new Project($this->registry); + $b = new Board($this->registry); + + $this->assertEquals(1, $p->create(array('name' => 'UnitTest1'))); + $this->assertTrue($b->addColumn(1, 'another column')); + $this->assertTrue($b->addColumn(1, 'one more', 3)); + + $columns = $b->getColumns(1); + $this->assertTrue(is_array($columns)); + $this->assertEquals(6, count($columns)); + + $this->assertEquals('another column', $columns[4]['title']); + $this->assertEquals(0, $columns[4]['task_limit']); + $this->assertEquals(5, $columns[4]['position']); + + $this->assertEquals('one more', $columns[5]['title']); + $this->assertEquals(3, $columns[5]['task_limit']); + $this->assertEquals(6, $columns[5]['position']); + } + public function testMoveColumns() { $p = new Project($this->registry); diff --git a/vendor/JsonRPC/Client.php b/vendor/JsonRPC/Client.php index dda78094..65aed223 100644 --- a/vendor/JsonRPC/Client.php +++ b/vendor/JsonRPC/Client.php @@ -2,11 +2,13 @@ namespace JsonRPC; +use BadFunctionCallException; + /** * JsonRPC client class * * @package JsonRPC - * @author Frderic Guillot + * @author Frederic Guillot * @license Unlicense http://unlicense.org/ */ class Client @@ -43,6 +45,14 @@ class Client */ private $password; + /** + * Enable debug output to the php error log + * + * @access public + * @var boolean + */ + public $debug = false; + /** * Default HTTP headers to send to the server * @@ -100,6 +110,7 @@ class Client * Execute * * @access public + * @throws BadFunctionCallException Exception thrown when a bad request is made (missing argument/procedure) * @param string $procedure Procedure name * @param array $params Procedure arguments * @return mixed @@ -124,7 +135,7 @@ class Client return $result['result']; } - return null; + throw new BadFunctionCallException('Bad Request'); } /** @@ -154,6 +165,11 @@ class Client $result = curl_exec($ch); $response = json_decode($result, true); + if ($this->debug) { + error_log('==> Request: '.PHP_EOL.json_encode($payload, JSON_PRETTY_PRINT)); + error_log('==> Response: '.PHP_EOL.json_encode($response, JSON_PRETTY_PRINT)); + } + curl_close($ch); return is_array($response) ? $response : array(); diff --git a/vendor/JsonRPC/Server.php b/vendor/JsonRPC/Server.php index f80531fd..72d4e27e 100644 --- a/vendor/JsonRPC/Server.php +++ b/vendor/JsonRPC/Server.php @@ -9,7 +9,7 @@ use Closure; * JsonRPC server class * * @package JsonRPC - * @author Frderic Guillot + * @author Frederic Guillot * @license Unlicense http://unlicense.org/ */ class Server @@ -155,16 +155,18 @@ class Server * @param array $request_params Incoming arguments * @param array $method_params Procedure arguments * @param array $params Arguments to pass to the callback + * @param integer $nb_required_params Number of required parameters * @return bool */ - public function mapParameters(array $request_params, array $method_params, array &$params) + public function mapParameters(array $request_params, array $method_params, array &$params, $nb_required_params) { + if (count($request_params) < $nb_required_params) { + return false; + } + // Positional parameters if (array_keys($request_params) === range(0, count($request_params) - 1)) { - - if (count($request_params) !== count($method_params)) return false; $params = $request_params; - return true; } @@ -188,14 +190,13 @@ class Server } /** - * Parse incoming requests + * Parse the payload and test if the parsed JSON is ok * * @access public - * @return string + * @return boolean */ - public function execute() + public function isValidJsonFormat() { - // Parse payload if (empty($this->payload)) { $this->payload = file_get_contents('php://input'); } @@ -204,53 +205,102 @@ class Server $this->payload = json_decode($this->payload, true); } - // Check JSON format - if (! is_array($this->payload)) { + return is_array($this->payload); + } - return $this->getResponse(array( - 'error' => array( - 'code' => -32700, - 'message' => 'Parse error' - )), - array('id' => null) - ); + /** + * Test if all required JSON-RPC parameters are here + * + * @access public + * @return boolean + */ + public function isValidJsonRpcFormat() + { + if (! isset($this->payload['jsonrpc']) || + ! isset($this->payload['method']) || + ! is_string($this->payload['method']) || + $this->payload['jsonrpc'] !== '2.0' || + (isset($this->payload['params']) && ! is_array($this->payload['params']))) { + + return false; } - // Handle batch request - if (array_keys($this->payload) === range(0, count($this->payload) - 1)) { + return true; + } - $responses = array(); + /** + * Return true if we have a batch request + * + * @access public + * @return boolean + */ + private function isBatchRequest() + { + return array_keys($this->payload) === range(0, count($this->payload) - 1); + } + + /** + * Handle batch request + * + * @access private + * @return string + */ + private function handleBatchRequest() + { + $responses = array(); - foreach ($this->payload as $payload) { + foreach ($this->payload as $payload) { - if (! is_array($payload)) { + if (! is_array($payload)) { - $responses[] = $this->getResponse(array( - 'error' => array( - 'code' => -32600, - 'message' => 'Invalid Request' - )), - array('id' => null) - ); - } - else { + $responses[] = $this->getResponse(array( + 'error' => array( + 'code' => -32600, + 'message' => 'Invalid Request' + )), + array('id' => null) + ); + } + else { - $server = new Server($payload); - $response = $server->execute(); + $server = new Server($payload); + $response = $server->execute(); - if ($response) $responses[] = $response; + if ($response) { + $responses[] = $response; } } + } - return empty($responses) ? '' : '['.implode(',', $responses).']'; + return empty($responses) ? '' : '['.implode(',', $responses).']'; + } + + /** + * Parse incoming requests + * + * @access public + * @return string + */ + public function execute() + { + // Invalid Json + if (! $this->isValidJsonFormat()) { + return $this->getResponse(array( + 'error' => array( + 'code' => -32700, + 'message' => 'Parse error' + )), + array('id' => null) + ); } - // Check JSON-RPC format - if (! isset($this->payload['jsonrpc']) || - ! isset($this->payload['method']) || - ! is_string($this->payload['method']) || - $this->payload['jsonrpc'] !== '2.0' || - (isset($this->payload['params']) && ! is_array($this->payload['params']))) { + // Handle batch request + if ($this->isBatchRequest()){ + return $this->handleBatchRequest(); + } + + // Invalid JSON-RPC format + if (! $this->isValidJsonRpcFormat()) { return $this->getResponse(array( 'error' => array( @@ -273,6 +323,7 @@ class Server ); } + // Execute the procedure $callback = self::$procedures[$this->payload['method']]; $params = array(); @@ -282,7 +333,7 @@ class Server $parameters = $reflection->getParameters(); - if (! $this->mapParameters($this->payload['params'], $parameters, $params)) { + if (! $this->mapParameters($this->payload['params'], $parameters, $params, $reflection->getNumberOfRequiredParameters())) { return $this->getResponse(array( 'error' => array( -- cgit v1.2.3