From d155c2728a50514fcb771caec7b71fd217586371 Mon Sep 17 00:00:00 2001 From: Joost Pastoor Date: Thu, 18 Aug 2016 08:48:14 +0200 Subject: [PATCH 1/7] Added client-side cache to Api::getResolutions and fixed issue with docblock. Added unittest. --- src/Jira/Api.php | 30 +++++++++++++++++++++++------- tests/Jira/ApiTest.php | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/Jira/Api.php b/src/Jira/Api.php index 16e321a..1c7d2d3 100644 --- a/src/Jira/Api.php +++ b/src/Jira/Api.php @@ -68,26 +68,33 @@ class Api protected $options = self::AUTOMAP_FIELDS; /** - * Fields. + * Client-side cache of Fields. * * @var array */ protected $fields; /** - * Priorities. + * Client-side cache of Priorities. * * @var array */ protected $priorities; /** - * Statuses. + * Client-side cache of Statuses. * * @var array */ protected $statuses; + /** + * Client-side cache of Resolutions. + * + * @var array + */ + protected $resolutions; + /** * Create a JIRA API client. * @@ -875,14 +882,23 @@ public function getProjectIssueTypes($project_key) /** * Returns a list of all resolutions. * - * @param string $project_key Project key. - * - * @return array|false + * @return array * @since 2.0.0 */ public function getResolutions() { - return $this->api(self::REQUEST_GET, '/rest/api/2/resolution', array(), true); + if ( !count($this->resolutions) ) { + $resolutions = array(); + $result = $this->api(self::REQUEST_GET, '/rest/api/2/resolution', array()); + + foreach ( $result->getResult() as $k => $v ) { + $resolutions[$v['id']] = $v; + } + + $this->resolutions = $resolutions; + } + + return $this->resolutions; } } diff --git a/tests/Jira/ApiTest.php b/tests/Jira/ApiTest.php index 7824f5a..aeabf44 100644 --- a/tests/Jira/ApiTest.php +++ b/tests/Jira/ApiTest.php @@ -149,6 +149,40 @@ public function testFindVersionByName() ); } + public function testGetResolutions() + { + $response = '[{"self":"https://test.atlassian.net/rest/api/2/resolution/1","id":"1","description":"A fix for this issue is checked into the tree and tested.","name":"Fixed"},{"self":"https://test.atlassian.net/rest/api/2/resolution/10000","id":"10000","description":"This issue won\'t be actioned.","name":"Won\'t Do"}]'; + + $this->expectClientCall( + Api::REQUEST_GET, + '/rest/api/2/resolution', + array(), + $response + ); + + $actual = $this->api->getResolutions(); + + $this->assertEquals(array( + '1' => array( + 'self' => 'https://test.atlassian.net/rest/api/2/resolution/1', + 'id' => '1', + 'description' => 'A fix for this issue is checked into the tree and tested.', + 'name' => 'Fixed', + ), + '10000' => array( + 'self' => 'https://test.atlassian.net/rest/api/2/resolution/10000', + 'id' => '10000', + 'description' => 'This issue won\'t be actioned.', + 'name' => 'Won\'t Do', + ), + ), $actual); + + // Second time we call the method the results should be cached and not trigger an API Request. + $this->client->sendRequest(Api::REQUEST_GET, '/rest/api/2/resolution', array(), self::ENDPOINT, $this->credential) + ->shouldNotBeCalled(); + $this->api->getResolutions(); + } + /** * Expects a particular client call. * From ff7efe2317e8fa04b7858c7ac812da4c637e7938 Mon Sep 17 00:00:00 2001 From: Joost Pastoor Date: Thu, 18 Aug 2016 12:40:24 +0200 Subject: [PATCH 2/7] Refactored local caches to be more explicitly null when not fetched Added unit tests for other local caches as well Made a helper function to clear the local caches when a new endpoint is selected --- src/Jira/Api.php | 64 +++++++++----- tests/Jira/ApiTest.php | 106 +++++++++++++++++++---- tests/Jira/resources/api_field.json | 35 ++++++++ tests/Jira/resources/api_priority.json | 18 ++++ tests/Jira/resources/api_resolution.json | 14 +++ tests/Jira/resources/api_status.json | 30 +++++++ 6 files changed, 228 insertions(+), 39 deletions(-) create mode 100644 tests/Jira/resources/api_field.json create mode 100644 tests/Jira/resources/api_priority.json create mode 100644 tests/Jira/resources/api_resolution.json create mode 100644 tests/Jira/resources/api_status.json diff --git a/src/Jira/Api.php b/src/Jira/Api.php index 1c7d2d3..5cf93bb 100644 --- a/src/Jira/Api.php +++ b/src/Jira/Api.php @@ -70,28 +70,28 @@ class Api /** * Client-side cache of Fields. * - * @var array + * @var array|null List of fields when loaded, null when nothing is fetched yet */ protected $fields; /** * Client-side cache of Priorities. * - * @var array + * @var array|null List of priorities when loaded, null when nothing is fetched yet */ protected $priorities; /** * Client-side cache of Statuses. * - * @var array + * @var array|null List of statuses when loaded, null when nothing is fetched yet */ protected $statuses; /** * Client-side cache of Resolutions. * - * @var array + * @var array|null List of resolutions when loaded, null when nothing is fetched yet */ protected $resolutions; @@ -148,12 +148,26 @@ public function getEndpoint() */ public function setEndpoint($url) { - $this->fields = array(); - // Remove trailing slash in the url. $url = rtrim($url, '/'); - $this->endpoint = $url; + if ( $url != $this->endpoint ) { + $this->endpoint = $url; + $this->clearLocalCaches(); + } + } + + /** + * Helper method to clear the local caches. Is called when switching endpoints + * + * @return void + */ + protected function clearLocalCaches() + { + $this->fields = null; + $this->priorities = null; + $this->statuses = null; + $this->resolutions = null; } /** @@ -163,13 +177,14 @@ public function setEndpoint($url) */ public function getFields() { - if ( !count($this->fields) ) { + // Fetch fields when the method is called for the first time. + if ( $this->fields === null ) { $fields = array(); - $_fields = $this->api(self::REQUEST_GET, '/rest/api/2/field', array()); + $result = $this->api(self::REQUEST_GET, '/rest/api/2/field', array(), true); /* set hash key as custom field id */ - foreach ( $_fields->getResult() as $k => $v ) { - $fields[$v['id']] = $v; + foreach ( $result as $field ) { + $fields[$field['id']] = $field; } $this->fields = $fields; @@ -463,13 +478,14 @@ public function findVersionByName($project_key, $name) */ public function getPriorities() { - if ( !count($this->priorities) ) { + // Fetch priorities when the method is called for the first time. + if ( $this->priorities === null ) { $priorities = array(); - $result = $this->api(self::REQUEST_GET, '/rest/api/2/priority', array()); + $result = $this->api(self::REQUEST_GET, '/rest/api/2/priority', array(), true); /* set hash key as custom field id */ - foreach ( $result->getResult() as $k => $v ) { - $priorities[$v['id']] = $v; + foreach ( $result as $priority ) { + $priorities[$priority['id']] = $priority; } $this->priorities = $priorities; @@ -498,13 +514,14 @@ public function getPriorties() */ public function getStatuses() { - if ( !count($this->statuses) ) { + // Fetch statuses when the method is called for the first time. + if ( $this->statuses === null ) { $statuses = array(); - $result = $this->api(self::REQUEST_GET, '/rest/api/2/status', array()); + $result = $this->api(self::REQUEST_GET, '/rest/api/2/status', array(), true); /* set hash key as custom field id */ - foreach ( $result->getResult() as $k => $v ) { - $statuses[$v['id']] = $v; + foreach ( $result as $status ) { + $statuses[$status['id']] = $status; } $this->statuses = $statuses; @@ -887,12 +904,13 @@ public function getProjectIssueTypes($project_key) */ public function getResolutions() { - if ( !count($this->resolutions) ) { + // Fetch resolutions when the method is called for the first time. + if ( $this->resolutions === null ) { $resolutions = array(); - $result = $this->api(self::REQUEST_GET, '/rest/api/2/resolution', array()); + $result = $this->api(self::REQUEST_GET, '/rest/api/2/resolution', array(), true); - foreach ( $result->getResult() as $k => $v ) { - $resolutions[$v['id']] = $v; + foreach ( $result as $resolution ) { + $resolutions[$resolution['id']] = $resolution; } $this->resolutions = $resolutions; diff --git a/tests/Jira/ApiTest.php b/tests/Jira/ApiTest.php index aeabf44..4a8b38d 100644 --- a/tests/Jira/ApiTest.php +++ b/tests/Jira/ApiTest.php @@ -151,7 +151,7 @@ public function testFindVersionByName() public function testGetResolutions() { - $response = '[{"self":"https://test.atlassian.net/rest/api/2/resolution/1","id":"1","description":"A fix for this issue is checked into the tree and tested.","name":"Fixed"},{"self":"https://test.atlassian.net/rest/api/2/resolution/10000","id":"10000","description":"This issue won\'t be actioned.","name":"Won\'t Do"}]'; + $response = file_get_contents(__DIR__ . '/resources/api_resolution.json'); $this->expectClientCall( Api::REQUEST_GET, @@ -162,25 +162,99 @@ public function testGetResolutions() $actual = $this->api->getResolutions(); - $this->assertEquals(array( - '1' => array( - 'self' => 'https://test.atlassian.net/rest/api/2/resolution/1', - 'id' => '1', - 'description' => 'A fix for this issue is checked into the tree and tested.', - 'name' => 'Fixed', - ), - '10000' => array( - 'self' => 'https://test.atlassian.net/rest/api/2/resolution/10000', - 'id' => '10000', - 'description' => 'This issue won\'t be actioned.', - 'name' => 'Won\'t Do', - ), - ), $actual); + $response_decoded = json_decode($response, true); + + $expected = array( + '1' => $response_decoded[0], + '10000' => $response_decoded[1], + ); + $this->assertEquals($expected, $actual); // Second time we call the method the results should be cached and not trigger an API Request. $this->client->sendRequest(Api::REQUEST_GET, '/rest/api/2/resolution', array(), self::ENDPOINT, $this->credential) ->shouldNotBeCalled(); - $this->api->getResolutions(); + $this->assertEquals($expected, $this->api->getResolutions(), 'Calling twice did not yield the same results'); + } + + public function testGetFields() + { + $response = file_get_contents(__DIR__ . '/resources/api_field.json'); + + $this->expectClientCall( + Api::REQUEST_GET, + '/rest/api/2/field', + array(), + $response + ); + + $actual = $this->api->getFields(); + + $response_decoded = json_decode($response, true); + + $expected = array( + 'issuetype' => $response_decoded[0], + 'timespent' => $response_decoded[1], + ); + $this->assertEquals($expected, $actual); + + // Second time we call the method the results should be cached and not trigger an API Request. + $this->client->sendRequest(Api::REQUEST_GET, '/rest/api/2/field', array(), self::ENDPOINT, $this->credential) + ->shouldNotBeCalled(); + $this->assertEquals($expected, $this->api->getFields(), 'Calling twice did not yield the same results'); + } + + public function testGetStatuses() + { + $response = file_get_contents(__DIR__ . '/resources/api_status.json'); + + $this->expectClientCall( + Api::REQUEST_GET, + '/rest/api/2/resolution', + array(), + $response + ); + + $actual = $this->api->getResolutions(); + + $response_decoded = json_decode($response, true); + + $expected = array( + '1' => $response_decoded[0], + '3' => $response_decoded[1], + ); + $this->assertEquals($expected, $actual); + + // Second time we call the method the results should be cached and not trigger an API Request. + $this->client->sendRequest(Api::REQUEST_GET, '/rest/api/2/resolution', array(), self::ENDPOINT, $this->credential) + ->shouldNotBeCalled(); + $this->assertEquals($expected, $this->api->getResolutions(), 'Calling twice did not yield the same results'); + } + + public function testGetPriorities() + { + $response = file_get_contents(__DIR__ . '/resources/api_priority.json'); + + $this->expectClientCall( + Api::REQUEST_GET, + '/rest/api/2/priority', + array(), + $response + ); + + $actual = $this->api->getPriorities(); + + $response_decoded = json_decode($response, true); + + $expected = array( + '1' => $response_decoded[0], + '5' => $response_decoded[1], + ); + $this->assertEquals($expected, $actual); + + // Second time we call the method the results should be cached and not trigger an API Request. + $this->client->sendRequest(Api::REQUEST_GET, '/rest/api/2/priority', array(), self::ENDPOINT, $this->credential) + ->shouldNotBeCalled(); + $this->assertEquals($expected, $this->api->getPriorities(), 'Calling twice did not yield the same results'); } /** diff --git a/tests/Jira/resources/api_field.json b/tests/Jira/resources/api_field.json new file mode 100644 index 0000000..32d19ec --- /dev/null +++ b/tests/Jira/resources/api_field.json @@ -0,0 +1,35 @@ +[ + { + "id": "issuetype", + "key": "issuetype", + "name": "Issue Type", + "custom": false, + "orderable": true, + "navigable": true, + "searchable": true, + "clauseNames": [ + "issuetype", + "type" + ], + "schema": { + "type": "issuetype", + "system": "issuetype" + } + }, + { + "id": "timespent", + "key": "timespent", + "name": "Time Spent", + "custom": false, + "orderable": false, + "navigable": true, + "searchable": false, + "clauseNames": [ + "timespent" + ], + "schema": { + "type": "number", + "system": "timespent" + } + } +] diff --git a/tests/Jira/resources/api_priority.json b/tests/Jira/resources/api_priority.json new file mode 100644 index 0000000..3be3a91 --- /dev/null +++ b/tests/Jira/resources/api_priority.json @@ -0,0 +1,18 @@ +[ + { + "self": "https://test.atlassian.net/rest/api/2/priority/1", + "statusColor": "#cc0000", + "description": "Blocks development and/or testing work, production could not run.", + "iconUrl": "https://test.atlassian.net/images/icons/priorities/blocker.svg", + "name": "Blocker", + "id": "1" + }, + { + "self": "https://test.atlassian.net/rest/api/2/priority/5", + "statusColor": "#003300", + "description": "Cosmetic problem like misspelt words or misaligned text.", + "iconUrl": "https://test.atlassian.net/images/icons/priorities/trivial.svg", + "name": "Trivial", + "id": "5" + } +] diff --git a/tests/Jira/resources/api_resolution.json b/tests/Jira/resources/api_resolution.json new file mode 100644 index 0000000..5faaeed --- /dev/null +++ b/tests/Jira/resources/api_resolution.json @@ -0,0 +1,14 @@ +[ + { + "self": "https://test.atlassian.net/rest/api/2/resolution/1", + "id": "1", + "description": "A fix for this issue is checked into the tree and tested.", + "name": "Fixed" + }, + { + "self": "https://test.atlassian.net/rest/api/2/resolution/10000", + "id": "10000", + "description": "This issue won't be actioned.", + "name": "Won't Do" + } +] diff --git a/tests/Jira/resources/api_status.json b/tests/Jira/resources/api_status.json new file mode 100644 index 0000000..aa6b107 --- /dev/null +++ b/tests/Jira/resources/api_status.json @@ -0,0 +1,30 @@ +[ + { + "self": "https://test.atlassian.net/rest/api/2/status/1", + "description": "The issue is open and ready for the assignee to start work on it.", + "iconUrl": "https://test.atlassian.net/images/icons/statuses/open.png", + "name": "Open", + "id": "1", + "statusCategory": { + "self": "https://test.atlassian.net/rest/api/2/statuscategory/2", + "id": 2, + "key": "new", + "colorName": "blue-gray", + "name": "To Do" + } + }, + { + "self": "https://test.atlassian.net/rest/api/2/status/3", + "description": "This issue is being actively worked on at the moment by the assignee.", + "iconUrl": "https://test.atlassian.net/images/icons/statuses/inprogress.png", + "name": "In Progress", + "id": "3", + "statusCategory": { + "self": "https://test.atlassian.net/rest/api/2/statuscategory/4", + "id": 4, + "key": "indeterminate", + "colorName": "yellow", + "name": "In Progress" + } + } +] From 19f33c7f01b518b79cc216f5d7705928a59782ff Mon Sep 17 00:00:00 2001 From: Joost Pastoor Date: Thu, 18 Aug 2016 13:22:58 +0200 Subject: [PATCH 3/7] Fixes based on aik comments --- src/Jira/Api.php | 2 +- tests/Jira/ApiTest.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Jira/Api.php b/src/Jira/Api.php index 5cf93bb..b8dc5bf 100644 --- a/src/Jira/Api.php +++ b/src/Jira/Api.php @@ -151,7 +151,7 @@ public function setEndpoint($url) // Remove trailing slash in the url. $url = rtrim($url, '/'); - if ( $url != $this->endpoint ) { + if ( $url !== $this->endpoint ) { $this->endpoint = $url; $this->clearLocalCaches(); } diff --git a/tests/Jira/ApiTest.php b/tests/Jira/ApiTest.php index 4a8b38d..a8eabd4 100644 --- a/tests/Jira/ApiTest.php +++ b/tests/Jira/ApiTest.php @@ -209,12 +209,12 @@ public function testGetStatuses() $this->expectClientCall( Api::REQUEST_GET, - '/rest/api/2/resolution', + '/rest/api/2/status', array(), $response ); - $actual = $this->api->getResolutions(); + $actual = $this->api->getStatuses(); $response_decoded = json_decode($response, true); @@ -225,9 +225,9 @@ public function testGetStatuses() $this->assertEquals($expected, $actual); // Second time we call the method the results should be cached and not trigger an API Request. - $this->client->sendRequest(Api::REQUEST_GET, '/rest/api/2/resolution', array(), self::ENDPOINT, $this->credential) + $this->client->sendRequest(Api::REQUEST_GET, '/rest/api/2/status', array(), self::ENDPOINT, $this->credential) ->shouldNotBeCalled(); - $this->assertEquals($expected, $this->api->getResolutions(), 'Calling twice did not yield the same results'); + $this->assertEquals($expected, $this->api->getStatuses(), 'Calling twice did not yield the same results'); } public function testGetPriorities() From eec6107c1c24ebae8d73453f151304299d5059bc Mon Sep 17 00:00:00 2001 From: Joost Pastoor Date: Thu, 18 Aug 2016 11:27:25 +0000 Subject: [PATCH 4/7] Fix of doc notation --- src/Jira/Api.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Jira/Api.php b/src/Jira/Api.php index b8dc5bf..ffb7b71 100644 --- a/src/Jira/Api.php +++ b/src/Jira/Api.php @@ -68,30 +68,30 @@ class Api protected $options = self::AUTOMAP_FIELDS; /** - * Client-side cache of Fields. + * Client-side cache of Fields. List of fields when loaded, null when nothing is fetched yet. * - * @var array|null List of fields when loaded, null when nothing is fetched yet + * @var array|null */ protected $fields; /** - * Client-side cache of Priorities. + * Client-side cache of Priorities. List of priorities when loaded, null when nothing is fetched yet. * - * @var array|null List of priorities when loaded, null when nothing is fetched yet + * @var array|null */ protected $priorities; /** - * Client-side cache of Statuses. + * Client-side cache of Statuses. List of statuses when loaded, null when nothing is fetched yet. * - * @var array|null List of statuses when loaded, null when nothing is fetched yet + * @var array|null */ protected $statuses; /** - * Client-side cache of Resolutions. + * Client-side cache of Resolutions. List of resolutions when loaded, null when nothing is fetched yet. * - * @var array|null List of resolutions when loaded, null when nothing is fetched yet + * @var array|null */ protected $resolutions; From 15c867e59ed5085ef5bd1880fb17e53709ec1837 Mon Sep 17 00:00:00 2001 From: Joost Pastoor Date: Thu, 18 Aug 2016 11:36:51 +0000 Subject: [PATCH 5/7] Fix of doc notation --- src/Jira/Api.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Jira/Api.php b/src/Jira/Api.php index ffb7b71..f45b9a4 100644 --- a/src/Jira/Api.php +++ b/src/Jira/Api.php @@ -68,28 +68,28 @@ class Api protected $options = self::AUTOMAP_FIELDS; /** - * Client-side cache of Fields. List of fields when loaded, null when nothing is fetched yet. + * Client-side cache of fields. List of fields when loaded, null when nothing is fetched yet. * * @var array|null */ protected $fields; /** - * Client-side cache of Priorities. List of priorities when loaded, null when nothing is fetched yet. + * Client-side cache of priorities. List of priorities when loaded, null when nothing is fetched yet. * * @var array|null */ protected $priorities; /** - * Client-side cache of Statuses. List of statuses when loaded, null when nothing is fetched yet. + * Client-side cache of statuses. List of statuses when loaded, null when nothing is fetched yet. * * @var array|null */ protected $statuses; /** - * Client-side cache of Resolutions. List of resolutions when loaded, null when nothing is fetched yet. + * Client-side cache of resolutions. List of resolutions when loaded, null when nothing is fetched yet. * * @var array|null */ From ab67c8b67c65d0ee0e79dc0a2f16ed9a5778de5d Mon Sep 17 00:00:00 2001 From: Joost Pastoor Date: Thu, 18 Aug 2016 12:39:05 +0000 Subject: [PATCH 6/7] Updated changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e55634..f570c72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Minimal supported PHP version changed from 5.2 to 5.3 by [@chobie]. - The `Api::getPriorties` renamed into `Api::getPriorities` (former method kept for BC reasons) by [@josevh]. - Remove trailing slash from endpoint url by [@Procta]. +- Clearing local caches (statuses, priorities, fields and resolutions) on endpoint change [@jpastoor]. +- Added local cache to getResolutions [@jpastoor]. ### Removed ... From 17c75f1f57a20e63c3f95e6ad8fef9d56ba78ccb Mon Sep 17 00:00:00 2001 From: Joost Pastoor Date: Thu, 18 Aug 2016 12:48:16 +0000 Subject: [PATCH 7/7] Switched comment to fixes list --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f570c72..fa8cf8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,6 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Minimal supported PHP version changed from 5.2 to 5.3 by [@chobie]. - The `Api::getPriorties` renamed into `Api::getPriorities` (former method kept for BC reasons) by [@josevh]. - Remove trailing slash from endpoint url by [@Procta]. -- Clearing local caches (statuses, priorities, fields and resolutions) on endpoint change [@jpastoor]. - Added local cache to getResolutions [@jpastoor]. ### Removed @@ -36,6 +35,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Fixed PHP deprecation notice, when creating issue attachments via `CurlClient` on PHP 5.5+ by [@DerMika]. - The `Api::getRoles` call was always retuning an error by [@aik099]. - Attempt to make a `DELETE` API call using `CurlClient` wasn't working by [@aik099]. +- Clearing local caches (statuses, priorities, fields and resolutions) on endpoint change [@jpastoor]. ## [1.0.0] - 2014-07-27 ### Added