From cde65a2b8b4d1baf92a84e529cc65a9a2a6734dd Mon Sep 17 00:00:00 2001 From: Scott Hutchinson Date: Fri, 30 Nov 2018 14:55:23 +1300 Subject: [PATCH 01/17] Upgrade Workable to SilverStripe 4 support --- .upgrade.yml | 7 + README.md | 19 +-- _config/config.yml | 9 +- code/Workable.php | 183 ++++++++++--------------- code/WorkableRestfulServiceFactory.php | 48 ------- code/WorkableResult.php | 49 +++++++ composer.json | 41 +++--- tests/TestWorkableLogger.php | 16 --- tests/TestWorkableRestfulService.php | 41 ------ tests/WorkableTest.php | 117 +++++++--------- 10 files changed, 211 insertions(+), 319 deletions(-) create mode 100644 .upgrade.yml delete mode 100644 code/WorkableRestfulServiceFactory.php create mode 100644 code/WorkableResult.php delete mode 100644 tests/TestWorkableLogger.php delete mode 100644 tests/TestWorkableRestfulService.php diff --git a/.upgrade.yml b/.upgrade.yml new file mode 100644 index 0000000..27331ea --- /dev/null +++ b/.upgrade.yml @@ -0,0 +1,7 @@ +mappings: + WorkableTest: SilverStripe\Workable\WorkableTest + TestWorkableRestfulService: SilverStripe\Workable\TestWorkableRestfulService + TestWorkableLogger: SilverStripe\Workable\TestWorkableLogger + Workable: SilverStripe\Workable\Workable + Workable_Result: SilverStripe\Workable\Workable_Result + WorkableRestfulServiceFactory: SilverStripe\Workable\WorkableRestfulServiceFactory diff --git a/README.md b/README.md index 8569145..73a3fc6 100644 --- a/README.md +++ b/README.md @@ -2,23 +2,16 @@ Adds Workable API integration to SilverStripe projects. ## Configuration -First, add your API key using a constant, preferably in your `_ss_environment.php` file. +First, add your API key using a constant, preferably in your `.env` file. -```php -define('WORKABLE_API_KEY','your_api_key'); ``` - -Alternatively, you can add your API key in the config, although this is not recommended. - -```yaml -Workable: - apiKey: your_api_key +WORKABLE_API_KEY="your_api_key" ``` Then, just add your subdomain to the config. -``` -Workable: +```yml +SilverStripe\Workable\Workable: subdomain: example ``` @@ -35,7 +28,7 @@ This returns an `ArrayList`, so you can iterate over it on the template. ```html <% loop $Jobs %> -$Title, $Url + $Title, $Url <% end_loop %> ``` @@ -43,7 +36,7 @@ For nested properties, you can use the dot-separated syntax. ```html <% loop $Jobs %> -$Title ($Location.City) + $Title ($Location.City) <% end_loop %> ``` diff --git a/_config/config.yml b/_config/config.yml index a32cc72..3009517 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -1,12 +1,5 @@ --- Name: workable --- -Workable: +SilverStripe\Workable\Workable: cache_expiry: 3600 -Injector: - Workable: - constructor: - 0: %$WorkableRestfulService - WorkableRestfulService: - class: RestfulService - factory: WorkableRestfulServiceFactory \ No newline at end of file diff --git a/code/Workable.php b/code/Workable.php index 630383f..1f98e0d 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -1,113 +1,76 @@ - */ -class Workable extends Object { - - /** - * Reference to the RestfulService dependency - * @var RestfulService - */ - protected $restulService; - - /** - * Constructor, inject the restful service dependency - * @param RestfulService $restfulService - */ - public function __construct($restfulService) { - $this->restfulService = $restfulService; - - parent::__construct(); - } - - /** - * Gets all the jobs from the Workable API - * @param array $params Array of params, e.g. ['state' => 'published'] - * @return ArrayList - */ - public function getJobs($params = []) { - $list = ArrayList::create(); - $response = $this->callRestfulService('jobs', $params); - - if($response && isset($response['jobs']) && is_array($response['jobs'])) { - foreach($response['jobs'] as $record) { - $list->push(Workable_Result::create($record)); - } - } - - return $list; - } - - /** - * Wrapper method to configure the RestfulService, make the call, and handle errors - * @param string $url - * @param array $params - * @param string $method - * @return array JSON - */ - protected function callRestfulService($url, $params = [], $method = 'GET') { - $this->restfulService->setQueryString($params); - $response = $this->restfulService->request($url, $method, $params); - - if(!$response) { - SS_Log::log('No response from workable API endpoint ' . $url, SS_Log::WARN); - - return false; - } - else if($response->getStatusCode() !== 200) { - SS_Log::log("Received non-200 status code {$response->getStatusCode()} from workable API", SS_Log::WARN); - - return false; - } - - return Convert::json2array($response->getBody()); - } - +namespace SilverStripe\Workable; + +use RuntimeException; +use GuzzleHttp\Client; +use Psr\Log\LoggerInterface; +use SilverStripe\Core\Convert; +use SilverStripe\ORM\ArrayList; +use SilverStripe\Core\Extensible; +use SilverStripe\Core\Environment; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\Core\Config\Configurable; +use SilverStripe\Core\Injector\Injectable; + +class Workable +{ + use Extensible; + use Injectable; + use Configurable; + + private static $cache_expiry = 3600; + + /** + * Gets all the jobs from the Workable API + * @param array $params Array of params, e.g. ['state' => 'published'] + * @return ArrayList + */ + public function getJobs($params = []) + { + $list = ArrayList::create(); + $response = $this->callRestfulService('jobs', $params); + + if ($response && isset($response['jobs']) && is_array($response['jobs'])) { + foreach ($response['jobs'] as $record) { + $list->push(WorkableResult::create($record)); + } + } + + return $list; + } + + /** + * Wrapper method to configure the RestfulService, make the call, and handle errors + * @param string $url + * @param array $params + * @param string $method + * @return array JSON + */ + public function callRestfulService($url, $params = [], $method = 'GET') + { + $apiKey = Environment::getEnv('WORKABLE_API_KEY'); + $subdomain = static::config()->subdomain; + $logger = Injector::inst()->get(LoggerInterface::class); + + if (!$apiKey) { + throw new RuntimeException('WORKABLE_API_KEY Environment variable not set'); + } + + if (!$subdomain) { + throw new RuntimeException( + 'You must set a Workable subdomain in the config (SilverStripe\Workable\Workable.subdomain)' + ); + } + + $client = new Client([ + 'base_uri' => sprintf('https://%s.workable.com/spi/v3/', $subdomain), + 'headers' => [ + 'Authorization' => sprintf('Bearer %s', Environment::getEnv('WORKABLE_API_KEY')), + ] + ]); + $response = $client->request($method, $url, $params); + + return Convert::json2array($response->getBody()); + } } - - -/** - * Defines the renderable Workable data for the template. Converts UpperCamelCase properties - * to the snake_case that comes from the API - */ -class Workable_Result extends ViewableData { - - /** - * Raw data from the API - * @var array - */ - protected $apiData; - - /** - * Magic getter that converts SilverStripe $UpperCamelCase to snake_case - * e.g. $FullTitle gets full_title. You can also use dot-separated syntax, e.g. $Location.City - * @param string $prop - * @return mixed - */ - public function __get($prop) { - $snaked = ltrim(strtolower(preg_replace('/[A-Z]/', '_$0', $prop)), '_'); - - if(!isset($this->apiData[$snaked])) { - return null; - } - $data = $this->apiData[$snaked]; - - if(is_array($this->apiData[$snaked])) { - return new Workable_Result($data); - } - - return $data; - } - - /** - * constructor - * @param array $apiData - */ - public function __construct($apiData = []) { - $this->apiData = $apiData; - } -} \ No newline at end of file diff --git a/code/WorkableRestfulServiceFactory.php b/code/WorkableRestfulServiceFactory.php deleted file mode 100644 index 6dc4ece..0000000 --- a/code/WorkableRestfulServiceFactory.php +++ /dev/null @@ -1,48 +0,0 @@ -subdomain; - - if(!$subdomain) { - throw new RuntimeException('You must set a Workable subdomain in the config (Workable.subdomain)'); - } - - $rest = new $service( - sprintf('https://www.workable.com/spi/v3/accounts/%s/',$subdomain), - $config->cache_expiry - ); - - if(defined('WORKABLE_API_KEY')) { - $apiKey = WORKABLE_API_KEY; - } - else { - $apiKey = Config::inst()->get('Workable','apiKey'); - } - - if(!$apiKey) { - throw new RuntimeException('You must define an API key for Workable. Either use the WORKABLE_API_KEY constant or set Workable.apiKey in the config'); - } - - $rest->httpHeader("Authorization:Bearer $apiKey"); - $rest->httpHeader("Content-Type: application/json"); - - return $rest; - } -} \ No newline at end of file diff --git a/code/WorkableResult.php b/code/WorkableResult.php new file mode 100644 index 0000000..b4548ca --- /dev/null +++ b/code/WorkableResult.php @@ -0,0 +1,49 @@ +apiData[$snaked])) { + return null; + } + $data = $this->apiData[$snaked]; + + if (is_array($this->apiData[$snaked])) { + return new WorkableResult($data); + } + + return $data; + } + + /** + * constructor + * @param array $apiData + */ + public function __construct($apiData = []) + { + $this->apiData = $apiData; + } +} diff --git a/composer.json b/composer.json index 5fd8ae8..543777c 100644 --- a/composer.json +++ b/composer.json @@ -1,20 +1,25 @@ { - "name":"silverstripe/workable", - "type": "silverstripe-module", - "description": "Adds Workable API integration to SilverStripe projects", - "keywords": ["silverstripe"], - "license": "BSD-3-Clause", - "homepage": "https://github.com/silverstripe/workable/", - "require": { - "silverstripe/framework": ">=3.1.0", - "php": ">=5.4" - }, - "authors":[ - { - "name": "Aaron Carlino", - "homepage": "http://leftandmain.com", - "email" : "aaron@silverstripe.com" - } - - ] + "name":"silverstripe/workable", + "type": "silverstripe-vendormodule", + "description": "Adds Workable API integration to SilverStripe projects", + "keywords": ["silverstripe"], + "license": "BSD-3-Clause", + "homepage": "https://github.com/silverstripe/workable/", + "require": { + "silverstripe/framework": "^4", + "guzzlehttp/guzzle": "^6", + "php": ">=5.6" + }, + "authors":[ + { + "name": "Aaron Carlino", + "homepage": "http://leftandmain.com", + "email" : "aaron@silverstripe.com" + }, + { + "name": "Torque Foxes", + "homepage": "https://github.com/torque-foxes", + "email" : "torque-foxues@silverstripe.com" + } + ] } diff --git a/tests/TestWorkableLogger.php b/tests/TestWorkableLogger.php deleted file mode 100644 index d885286..0000000 --- a/tests/TestWorkableLogger.php +++ /dev/null @@ -1,16 +0,0 @@ -event = $event; - } - - - public static function factory ($config) { - return new TestWorkableLogger(); - } -} \ No newline at end of file diff --git a/tests/TestWorkableRestfulService.php b/tests/TestWorkableRestfulService.php deleted file mode 100644 index 29d0c3b..0000000 --- a/tests/TestWorkableRestfulService.php +++ /dev/null @@ -1,41 +0,0 @@ -params = $params; - } - - - public function request($subURL = '', $method = "GET", $data = null, $headers = null, $curlOptions = array()) { - switch($subURL) { - case 'jobs': - if($this->params['state'] === 'published') { - return new RestfulService_Response( - json_encode(['jobs' => [ - ['title' => 'Published Job 1'], - ['title' => 'Published Job 2'], - ['title' => 'Published Job 3'] - ]]), - 200 - ); - } - if($this->params['state'] === 'draft') { - return new RestfulService_Response( - json_encode(['jobs' => [ - ['title' => 'Draft Job 1'] - ]]), - 200 - ); - } - if($this->params['state'] === 'fail') { - return new RestfulService_Response('FAIL', 404); - } - break; - } - - } - -} diff --git a/tests/WorkableTest.php b/tests/WorkableTest.php index 0a3e224..64ae0b0 100644 --- a/tests/WorkableTest.php +++ b/tests/WorkableTest.php @@ -1,67 +1,54 @@ get('Injector','WorkableRestfulService'); - $config['class'] = 'TestWorkableRestfulService'; - Config::inst()->update('Injector','WorkableRestfulService', $config); - - Config::inst()->update('Workable', 'apiKey', 'test'); - Config::inst()->update('Workable', 'subdomain', 'example'); - } - - public function testThrowsIfNoSubdomain () { - Config::inst()->remove('Workable','subdomain'); - $this->setExpectedException('RuntimeException'); - - Workable::create(); - } - - public function testWillUseAPIKeyConstant () { - Config::inst()->remove('Workable','apiKey'); - if(!defined('WORKABLE_API_KEY')) { - define('WORKABLE_API_KEY','test'); - } - - Workable::create(); - } - - public function testGetsPublishedJobs () { - $result = Workable::create()->getJobs(['state' => 'published']); - - $this->assertEquals(3, $result->count()); - $this->assertEquals('Published Job 1', $result->first()->Title); - } - - public function testGetsUnpublishedJobs () { - $result = Workable::create()->getJobs(['state' => 'draft']); - - $this->assertEquals(1, $result->count()); - $this->assertEquals('Draft Job 1', $result->first()->Title); - } - - public function testLogsError () { - $logger = new TestWorkableLogger(); - SS_Log::add_writer($logger); - $result = Workable::create()->getJobs(['state' => 'fail']); - - $this->assertNotNull($logger->event); - - SS_Log::remove_writer($logger); - } - - public function testConvertsSnakeCase () { - $data = new Workable_Result(['snake_case' => 'foo']); - - $this->assertEquals('foo', $data->SnakeCase); - } - - public function testAcceptsDotSyntax () { - $data = new Workable_Result(['snake_case' => ['nested_property' => 'foo']]); - $result = $data->SnakeCase; - $this->assertInstanceOf('Workable_Result', $result); - $this->assertEquals('foo', $result->NestedProperty); - } -} \ No newline at end of file +namespace SilverStripe\Workable; + +use Psr\Log\LoggerInterface; +use SilverStripe\Core\Environment; +use SilverStripe\Dev\SapphireTest; +use SilverStripe\Workable\Workable; +use SilverStripe\Core\Config\Config; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\Workable\WorkableResult; +use SilverStripe\Workable\TestWorkableRestfulService; + +class WorkableTest extends SapphireTest +{ + public function setUp() + { + parent::setUp(); + + Environment::setEnv('WORKABLE_API_KEY', 'test'); + Config::inst()->update(Workable::class, 'subdomain', 'example'); + } + + public function testThrowsIfNoSubdomain() + { + Config::inst()->remove(Workable::class, 'subdomain'); + $this->setExpectedException('RuntimeException'); + + Workable::create()->callRestfulService('test'); + } + + public function testThrowsIfNoApiKey() + { + Environment::setEnv('WORKABLE_API_KEY', null); + $this->setExpectedException('RuntimeException'); + + Workable::create()->callRestfulService('test'); + } + + public function testConvertsSnakeCase() + { + $data = WorkableResult::create(['snake_case' => 'foo']); + + $this->assertEquals('foo', $data->SnakeCase); + } + + public function testAcceptsDotSyntax() + { + $data = WorkableResult::create(['snake_case' => ['nested_property' => 'foo']]); + $result = $data->SnakeCase; + $this->assertInstanceOf(WorkableResult::class, $result); + $this->assertEquals('foo', $result->NestedProperty); + } +} From e453c6ed1a212e3c1c79bf8bb2ec92c51dd1cfb6 Mon Sep 17 00:00:00 2001 From: Scott Hutchinson Date: Mon, 3 Dec 2018 15:56:54 +1300 Subject: [PATCH 02/17] Add caching to API calls --- .upgrade.yml | 5 +-- _config/config.yml | 8 +++-- code/Workable.php | 85 ++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 86 insertions(+), 12 deletions(-) diff --git a/.upgrade.yml b/.upgrade.yml index 27331ea..bd7f8ba 100644 --- a/.upgrade.yml +++ b/.upgrade.yml @@ -1,7 +1,4 @@ mappings: WorkableTest: SilverStripe\Workable\WorkableTest - TestWorkableRestfulService: SilverStripe\Workable\TestWorkableRestfulService - TestWorkableLogger: SilverStripe\Workable\TestWorkableLogger Workable: SilverStripe\Workable\Workable - Workable_Result: SilverStripe\Workable\Workable_Result - WorkableRestfulServiceFactory: SilverStripe\Workable\WorkableRestfulServiceFactory + Workable_Result: SilverStripe\Workable\WorkableResult diff --git a/_config/config.yml b/_config/config.yml index 3009517..5709d21 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -1,5 +1,9 @@ --- Name: workable --- -SilverStripe\Workable\Workable: - cache_expiry: 3600 +SilverStripe\Core\Injector\Injector: + Psr\SimpleCache\CacheInterface.workable: + factory: SilverStripe\Core\Cache\CacheFactory + constructor: + namespace: 'workable' + defaultLifetime: 3600 diff --git a/code/Workable.php b/code/Workable.php index 1f98e0d..84ca260 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -7,20 +7,21 @@ use Psr\Log\LoggerInterface; use SilverStripe\Core\Convert; use SilverStripe\ORM\ArrayList; +use SilverStripe\Core\Flushable; +use SilverStripe\View\ArrayData; use SilverStripe\Core\Extensible; use SilverStripe\Core\Environment; +use Psr\SimpleCache\CacheInterface; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Injector\Injectable; -class Workable +class Workable implements Flushable { use Extensible; use Injectable; use Configurable; - private static $cache_expiry = 3600; - /** * Gets all the jobs from the Workable API * @param array $params Array of params, e.g. ['state' => 'published'] @@ -28,15 +29,78 @@ class Workable */ public function getJobs($params = []) { + $cache = Injector::inst()->get(CacheInterface::class . '.workable'); + $cacheKey = 'Jobs'; $list = ArrayList::create(); $response = $this->callRestfulService('jobs', $params); + if ($cache->has($cacheKey)) { + return $cache->get($cacheKey); + } + if ($response && isset($response['jobs']) && is_array($response['jobs'])) { foreach ($response['jobs'] as $record) { $list->push(WorkableResult::create($record)); } } + $cache->set($cacheKey, $list); + + return $list; + } + + /** + * Gets information on a specific job form the Workable API + * @param string $shortcode Workable shortcode for the job, e.g. 'GROOV005' + * @param array $params Array of params, e.g. ['state' => 'published'] + * @return WorkableResult|null + */ + public function getJob($shortcode, $params = []) + { + $job = null; + $cache = Injector::inst()->get(CacheInterface::class . '.workable'); + $cacheKey = 'Job-' . $shortcode; + $response = $this->callRestfulService('jobs/' . $shortcode, $params); + + if ($cache->has($cacheKey)) { + return $cache->get($cacheKey); + } + + if ($response && isset($response['id'])) { + $job = WorkableResult::create($response); + } + + $cache->set($cacheKey, $job); + + return $job; + } + + /** + * Gets all the jobs from the workable API, populating each job with its full data + * Note: This calls the API multiple times so should be used with caution. + * @param array $params Array of params, e.g. ['state' => 'published'] + * @return ArrayList + */ + public function getFullJobs($params = []) + { + $cache = Injector::inst()->get(CacheInterface::class . '.workable'); + $cacheKey = 'FullJobs'; + $list = ArrayList::create(); + $response = $this->callRestfulService('jobs', $params); + + if ($cache->has($cacheKey)) { + return $cache->get($cacheKey); + } + + if ($response && isset($response['jobs']) && is_array($response['jobs'])) { + foreach ($response['jobs'] as $record) { + $job = $this->getJob($record['shortcode'], $params); + $list->push($job); + } + } + + $cache->set($cacheKey, $list); + return $list; } @@ -51,7 +115,6 @@ public function callRestfulService($url, $params = [], $method = 'GET') { $apiKey = Environment::getEnv('WORKABLE_API_KEY'); $subdomain = static::config()->subdomain; - $logger = Injector::inst()->get(LoggerInterface::class); if (!$apiKey) { throw new RuntimeException('WORKABLE_API_KEY Environment variable not set'); @@ -67,10 +130,20 @@ public function callRestfulService($url, $params = [], $method = 'GET') 'base_uri' => sprintf('https://%s.workable.com/spi/v3/', $subdomain), 'headers' => [ 'Authorization' => sprintf('Bearer %s', Environment::getEnv('WORKABLE_API_KEY')), - ] + ], + 'query' => $params, ]); - $response = $client->request($method, $url, $params); + $response = $client->request($method, $url); return Convert::json2array($response->getBody()); } + + /** + * Clear the cache when flush is called + */ + public static function flush() + { + $cache = Injector::inst()->get(CacheInterface::class . '.workable'); + $cache->clear(); + } } From 4fac2647d6702183a4fba46ac049b83109e5cbbf Mon Sep 17 00:00:00 2001 From: Scott Hutchinson Date: Mon, 3 Dec 2018 23:59:45 +1300 Subject: [PATCH 03/17] Use Factory for RestfulService --- .upgrade.yml | 1 + _config/config.yml | 6 ++++ code/Workable.php | 39 +++++++++------------- code/WorkableRestfulServiceFactory.php | 45 ++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 23 deletions(-) create mode 100644 code/WorkableRestfulServiceFactory.php diff --git a/.upgrade.yml b/.upgrade.yml index bd7f8ba..6e6d02f 100644 --- a/.upgrade.yml +++ b/.upgrade.yml @@ -2,3 +2,4 @@ mappings: WorkableTest: SilverStripe\Workable\WorkableTest Workable: SilverStripe\Workable\Workable Workable_Result: SilverStripe\Workable\WorkableResult + WorkableRestfulServiceFactory: SilverStripe\Workable\WorkableRestfulServiceFactory diff --git a/_config/config.yml b/_config/config.yml index 5709d21..4ecddba 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -7,3 +7,9 @@ SilverStripe\Core\Injector\Injector: constructor: namespace: 'workable' defaultLifetime: 3600 + SilverStripe\Workable\Workable: + constructor: + 0: %$WorkableRestfulService + WorkableRestfulService: + class: GuzzleHttp\Client + factory: SilverStripe\Workable\WorkableRestfulServiceFactory diff --git a/code/Workable.php b/code/Workable.php index 84ca260..3c5f803 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -3,14 +3,12 @@ namespace SilverStripe\Workable; use RuntimeException; -use GuzzleHttp\Client; use Psr\Log\LoggerInterface; use SilverStripe\Core\Convert; use SilverStripe\ORM\ArrayList; use SilverStripe\Core\Flushable; use SilverStripe\View\ArrayData; use SilverStripe\Core\Extensible; -use SilverStripe\Core\Environment; use Psr\SimpleCache\CacheInterface; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Config\Configurable; @@ -22,6 +20,21 @@ class Workable implements Flushable use Injectable; use Configurable; + /** + * Reference to the RestfulService dependency + * @var RestfulService + */ + protected $restulService; + + /** + * Constructor, inject the restful service dependency + * @param RestfulService $restfulService + */ + public function __construct($restfulService) + { + $this->restfulService = $restfulService; + } + /** * Gets all the jobs from the Workable API * @param array $params Array of params, e.g. ['state' => 'published'] @@ -113,27 +126,7 @@ public function getFullJobs($params = []) */ public function callRestfulService($url, $params = [], $method = 'GET') { - $apiKey = Environment::getEnv('WORKABLE_API_KEY'); - $subdomain = static::config()->subdomain; - - if (!$apiKey) { - throw new RuntimeException('WORKABLE_API_KEY Environment variable not set'); - } - - if (!$subdomain) { - throw new RuntimeException( - 'You must set a Workable subdomain in the config (SilverStripe\Workable\Workable.subdomain)' - ); - } - - $client = new Client([ - 'base_uri' => sprintf('https://%s.workable.com/spi/v3/', $subdomain), - 'headers' => [ - 'Authorization' => sprintf('Bearer %s', Environment::getEnv('WORKABLE_API_KEY')), - ], - 'query' => $params, - ]); - $response = $client->request($method, $url); + $response = $this->restfulService->request($method, $url, ['query' => $params]); return Convert::json2array($response->getBody()); } diff --git a/code/WorkableRestfulServiceFactory.php b/code/WorkableRestfulServiceFactory.php new file mode 100644 index 0000000..c070223 --- /dev/null +++ b/code/WorkableRestfulServiceFactory.php @@ -0,0 +1,45 @@ +subdomain; + + if (!$apiKey) { + throw new RuntimeException('WORKABLE_API_KEY Environment variable not set'); + } + + if (!$subdomain) { + throw new RuntimeException( + 'You must set a Workable subdomain in the config (SilverStripe\Workable\Workable.subdomain)' + ); + } + + return new $service([ + 'base_uri' => sprintf('https://%s.workable.com/spi/v3/', $subdomain), + 'headers' => [ + 'Authorization' => sprintf('Bearer %s', $apiKey), + ], + ]); + } +} From a4ed2de680fc36f91f36fa1fc1fe1168ed3baacc Mon Sep 17 00:00:00 2001 From: Scott Hutchinson Date: Tue, 4 Dec 2018 16:27:00 +1300 Subject: [PATCH 04/17] Add tests for API calls --- code/WorkableRestfulServiceFactory.php | 1 + tests/TestWorkableRestfulService.php | 43 ++++++++++++++++++++++++++ tests/WorkableTest.php | 34 ++++++++++++++++++-- 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 tests/TestWorkableRestfulService.php diff --git a/code/WorkableRestfulServiceFactory.php b/code/WorkableRestfulServiceFactory.php index c070223..02f3711 100644 --- a/code/WorkableRestfulServiceFactory.php +++ b/code/WorkableRestfulServiceFactory.php @@ -2,6 +2,7 @@ namespace SilverStripe\Workable; +use RuntimeException; use SilverStripe\Core\Environment; use SilverStripe\Workable\Workable; use SilverStripe\Core\Injector\Factory; diff --git a/tests/TestWorkableRestfulService.php b/tests/TestWorkableRestfulService.php new file mode 100644 index 0000000..f571489 --- /dev/null +++ b/tests/TestWorkableRestfulService.php @@ -0,0 +1,43 @@ +getMockJobs(); + case 'jobs/GROOV001': + case 'jobs/GROOV002': + return $this->getMockJob($url); + } + } + + protected function getMockJobs() + { + return new Response(200, [], json_encode(['jobs' => [ + [ + 'title' => 'Job 1', + 'shortcode' => 'GROOV001', + ], + [ + 'title' => 'Job 2', + 'shortcode' => 'GROOV002', + ], + ]])); + } + + protected function getMockJob($url) + { + return new Response(200, [], json_encode([ + 'title' => 'Job x', + 'test' => 'full data', + 'id' => 1, + 'shortcode' => substr($url, 5), + ])); + } +} diff --git a/tests/WorkableTest.php b/tests/WorkableTest.php index 64ae0b0..7cd33d5 100644 --- a/tests/WorkableTest.php +++ b/tests/WorkableTest.php @@ -1,6 +1,6 @@ get(Injector::class, 'WorkableRestfulService'); + $config['class'] = TestWorkableRestfulService::class; + Config::inst()->update(Injector::class, 'WorkableRestfulService', $config); Environment::setEnv('WORKABLE_API_KEY', 'test'); Config::inst()->update(Workable::class, 'subdomain', 'example'); @@ -51,4 +54,31 @@ public function testAcceptsDotSyntax() $this->assertInstanceOf(WorkableResult::class, $result); $this->assertEquals('foo', $result->NestedProperty); } + + public function testGetJobs() + { + $data = Workable::create()->getJobs(); + + $this->assertCount(2, $data); + } + + public function testGetJob() + { + $data = Workable::create()->getJob('GROOV001'); + + $this->assertNotNull($data); + $this->assertEquals('Job x', $data->title); + $this->assertEquals('GROOV001', $data->shortcode); + } + + public function testFullJobs() + { + $data = Workable::create()->getFullJobs(); + + $this->assertCount(2, $data); + $this->assertEquals('full data', $data[0]->test); + $this->assertEquals('GROOV001', $data[0]->shortcode); + $this->assertEquals('full data', $data[1]->test); + $this->assertEquals('GROOV002', $data[1]->shortcode); + } } From de11b3569a9a3492de59425b9dd2b592204a4457 Mon Sep 17 00:00:00 2001 From: Scott Hutchinson Date: Wed, 5 Dec 2018 13:01:25 +1300 Subject: [PATCH 05/17] Add logging for catching errors --- code/Workable.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/code/Workable.php b/code/Workable.php index 3c5f803..27298ee 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -2,6 +2,7 @@ namespace SilverStripe\Workable; +use Monolog\Logger; use RuntimeException; use Psr\Log\LoggerInterface; use SilverStripe\Core\Convert; @@ -126,7 +127,15 @@ public function getFullJobs($params = []) */ public function callRestfulService($url, $params = [], $method = 'GET') { - $response = $this->restfulService->request($method, $url, ['query' => $params]); + try { + $response = $this->restfulService->request($method, $url, ['query' => $params]); + } catch (\RuntimeException $e) { + Injector::inst()->get(LoggerInterface::class)->warning( + 'Failed to retreive valid response from workable', + ['exception' => $e] + ); + return []; + } return Convert::json2array($response->getBody()); } From 64d8b5bb375626a311081be33bfcb93d5063522b Mon Sep 17 00:00:00 2001 From: Scott Hutchinson Date: Wed, 5 Dec 2018 13:08:59 +1300 Subject: [PATCH 06/17] Only set cache for valid responses --- code/Workable.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/code/Workable.php b/code/Workable.php index 27298ee..4246e21 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -56,9 +56,9 @@ public function getJobs($params = []) foreach ($response['jobs'] as $record) { $list->push(WorkableResult::create($record)); } - } - $cache->set($cacheKey, $list); + $cache->set($cacheKey, $list); + } return $list; } @@ -82,10 +82,9 @@ public function getJob($shortcode, $params = []) if ($response && isset($response['id'])) { $job = WorkableResult::create($response); + $cache->set($cacheKey, $job); } - $cache->set($cacheKey, $job); - return $job; } @@ -111,9 +110,9 @@ public function getFullJobs($params = []) $job = $this->getJob($record['shortcode'], $params); $list->push($job); } - } - $cache->set($cacheKey, $list); + $cache->set($cacheKey, $list); + } return $list; } From b3a1bdd34503dda8f0a98018f79bf1637f8b3a3b Mon Sep 17 00:00:00 2001 From: Scott Hutchinson Date: Thu, 6 Dec 2018 16:45:02 +1300 Subject: [PATCH 07/17] Ensure cache is being used to avoid API calls + add more tests --- README.md | 2 +- code/Workable.php | 33 ++++++++----- tests/TestWorkableRestfulService.php | 74 ++++++++++++++++++++-------- tests/WorkableTest.php | 28 +++++++++++ 4 files changed, 103 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 73a3fc6..da0a4b5 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,5 @@ # Workable for SilverStripe -Adds Workable API integration to SilverStripe projects. +Adds Workable API integration to SilverStripe projects. See https://workable.readme.io/ for API docs. ## Configuration First, add your API key using a constant, preferably in your `.env` file. diff --git a/code/Workable.php b/code/Workable.php index 4246e21..9f58bfc 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -38,20 +38,22 @@ public function __construct($restfulService) /** * Gets all the jobs from the Workable API - * @param array $params Array of params, e.g. ['state' => 'published'] + * @param array $params Array of params, e.g. ['state' => 'published']. + * see https://workable.readme.io/docs/jobs for full list of query params * @return ArrayList */ public function getJobs($params = []) { $cache = Injector::inst()->get(CacheInterface::class . '.workable'); - $cacheKey = 'Jobs'; - $list = ArrayList::create(); - $response = $this->callRestfulService('jobs', $params); + $cacheKey = 'Jobs' . implode($params, '-'); if ($cache->has($cacheKey)) { return $cache->get($cacheKey); } + $list = ArrayList::create(); + $response = $this->callRestfulService('jobs', $params); + if ($response && isset($response['jobs']) && is_array($response['jobs'])) { foreach ($response['jobs'] as $record) { $list->push(WorkableResult::create($record)); @@ -66,20 +68,22 @@ public function getJobs($params = []) /** * Gets information on a specific job form the Workable API * @param string $shortcode Workable shortcode for the job, e.g. 'GROOV005' - * @param array $params Array of params, e.g. ['state' => 'published'] + * @param array $params Array of params, e.g. ['state' => 'published']. + * see https://workable.readme.io/docs/jobs for full list of query params * @return WorkableResult|null */ public function getJob($shortcode, $params = []) { - $job = null; $cache = Injector::inst()->get(CacheInterface::class . '.workable'); - $cacheKey = 'Job-' . $shortcode; - $response = $this->callRestfulService('jobs/' . $shortcode, $params); + $cacheKey = 'Job-' . $shortcode . implode($params, '-'); if ($cache->has($cacheKey)) { return $cache->get($cacheKey); } + $job = null; + $response = $this->callRestfulService('jobs/' . $shortcode, $params); + if ($response && isset($response['id'])) { $job = WorkableResult::create($response); $cache->set($cacheKey, $job); @@ -90,21 +94,24 @@ public function getJob($shortcode, $params = []) /** * Gets all the jobs from the workable API, populating each job with its full data - * Note: This calls the API multiple times so should be used with caution. - * @param array $params Array of params, e.g. ['state' => 'published'] + * Note: This calls the API multiple times so should be used with caution, see + * rate limiting docs https://workable.readme.io/docs/rate-limits + * @param array $params Array of params, e.g. ['state' => 'published']. + * see https://workable.readme.io/docs/jobs for full list of query params * @return ArrayList */ public function getFullJobs($params = []) { $cache = Injector::inst()->get(CacheInterface::class . '.workable'); - $cacheKey = 'FullJobs'; - $list = ArrayList::create(); - $response = $this->callRestfulService('jobs', $params); + $cacheKey = 'FullJobs' . implode($params, '-'); if ($cache->has($cacheKey)) { return $cache->get($cacheKey); } + $list = ArrayList::create(); + $response = $this->callRestfulService('jobs', $params); + if ($response && isset($response['jobs']) && is_array($response['jobs'])) { foreach ($response['jobs'] as $record) { $job = $this->getJob($record['shortcode'], $params); diff --git a/tests/TestWorkableRestfulService.php b/tests/TestWorkableRestfulService.php index f571489..7967265 100644 --- a/tests/TestWorkableRestfulService.php +++ b/tests/TestWorkableRestfulService.php @@ -10,34 +10,68 @@ public function request($method, $url, $params = []) { switch ($url) { case 'jobs': - return $this->getMockJobs(); + return $this->getMockJobs($params); case 'jobs/GROOV001': case 'jobs/GROOV002': - return $this->getMockJob($url); + return $this->getMockJob($url, $params); } } - protected function getMockJobs() + protected function getMockJobs($params) { - return new Response(200, [], json_encode(['jobs' => [ - [ - 'title' => 'Job 1', - 'shortcode' => 'GROOV001', - ], - [ - 'title' => 'Job 2', - 'shortcode' => 'GROOV002', - ], - ]])); + $state = isset($params['query']['state']) ? $params['query']['state'] : ''; + $response = []; + + switch ($state) { + case 'draft': + $response = ['jobs' => [ + [ + 'title' => 'draft job', + 'shortcode' => 'GROOV001', + ], + ]]; + break; + default: + $response = ['jobs' => [ + [ + 'title' => 'Job 1', + 'shortcode' => 'GROOV001', + ], + [ + 'title' => 'Job 2', + 'shortcode' => 'GROOV002', + ], + ]]; + break; + } + + return new Response(200, [], json_encode($response)); } - protected function getMockJob($url) + protected function getMockJob($url, $params) { - return new Response(200, [], json_encode([ - 'title' => 'Job x', - 'test' => 'full data', - 'id' => 1, - 'shortcode' => substr($url, 5), - ])); + $state = isset($params['query']['state']) ? $params['query']['state'] : ''; + $response = []; + + switch ($state) { + case 'draft': + $response = [ + 'title' => 'Draft Job x', + 'test' => 'full draft data', + 'id' => 1, + 'shortcode' => substr($url, 5), + ]; + break; + default: + $response = [ + 'title' => 'Job x', + 'test' => 'full data', + 'id' => 1, + 'shortcode' => substr($url, 5), + ]; + break; + } + + return new Response(200, [], json_encode($response)); } } diff --git a/tests/WorkableTest.php b/tests/WorkableTest.php index 7cd33d5..7124e6d 100644 --- a/tests/WorkableTest.php +++ b/tests/WorkableTest.php @@ -60,6 +60,15 @@ public function testGetJobs() $data = Workable::create()->getJobs(); $this->assertCount(2, $data); + $this->assertEquals('Job 1', $data[0]->title); + $this->assertEquals('Job 2', $data[1]->title); + } + + public function testGetJobsWithDraftState() + { + $data = Workable::create()->getJobs(['state' => 'draft']); + + $this->assertCount(1, $data); } public function testGetJob() @@ -71,6 +80,15 @@ public function testGetJob() $this->assertEquals('GROOV001', $data->shortcode); } + public function testGetJobWithDraftState() + { + $data = Workable::create()->getJob('GROOV001', ['state' => 'draft']); + + $this->assertNotNull($data); + $this->assertEquals('Draft Job x', $data->title); + $this->assertEquals('GROOV001', $data->shortcode); + } + public function testFullJobs() { $data = Workable::create()->getFullJobs(); @@ -81,4 +99,14 @@ public function testFullJobs() $this->assertEquals('full data', $data[1]->test); $this->assertEquals('GROOV002', $data[1]->shortcode); } + + public function testFullJobsWithDraftState() + { + $data = Workable::create()->getFullJobs(['state' => 'draft']); + + $this->assertCount(1, $data); + $this->assertEquals('Draft Job x', $data[0]->title); + $this->assertEquals('full draft data', $data[0]->test); + $this->assertEquals('GROOV001', $data[0]->shortcode); + } } From 2d3c077e543b8552e2518d9d0e5dba01cad309eb Mon Sep 17 00:00:00 2001 From: Andrew Aitken-Fincham Date: Fri, 6 Nov 2020 12:20:52 +1300 Subject: [PATCH 08/17] inject cache, fix spelling and deprecated method --- _config/config.yml | 1 + code/Workable.php | 42 +++++++++++++++++++++++------------------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/_config/config.yml b/_config/config.yml index 4ecddba..1103037 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -10,6 +10,7 @@ SilverStripe\Core\Injector\Injector: SilverStripe\Workable\Workable: constructor: 0: %$WorkableRestfulService + 1: %$Psr\SimpleCache\CacheInterface.workable WorkableRestfulService: class: GuzzleHttp\Client factory: SilverStripe\Workable\WorkableRestfulServiceFactory diff --git a/code/Workable.php b/code/Workable.php index 9f58bfc..7c09205 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -25,15 +25,23 @@ class Workable implements Flushable * Reference to the RestfulService dependency * @var RestfulService */ - protected $restulService; + protected $restfulService; + + /** + * Reference to the Cache dependency + * @var CacheInterface + */ + protected $cache; /** * Constructor, inject the restful service dependency * @param RestfulService $restfulService + * @param CacheInterface $cache */ - public function __construct($restfulService) + public function __construct($restfulService, $cache) { $this->restfulService = $restfulService; + $this->cache = $cache; } /** @@ -44,11 +52,10 @@ public function __construct($restfulService) */ public function getJobs($params = []) { - $cache = Injector::inst()->get(CacheInterface::class . '.workable'); $cacheKey = 'Jobs' . implode($params, '-'); - if ($cache->has($cacheKey)) { - return $cache->get($cacheKey); + if ($this->cache->has($cacheKey)) { + return $this->cache->get($cacheKey); } $list = ArrayList::create(); @@ -59,7 +66,7 @@ public function getJobs($params = []) $list->push(WorkableResult::create($record)); } - $cache->set($cacheKey, $list); + $this->cache->set($cacheKey, $list); } return $list; @@ -74,11 +81,10 @@ public function getJobs($params = []) */ public function getJob($shortcode, $params = []) { - $cache = Injector::inst()->get(CacheInterface::class . '.workable'); $cacheKey = 'Job-' . $shortcode . implode($params, '-'); - if ($cache->has($cacheKey)) { - return $cache->get($cacheKey); + if ($this->cache->has($cacheKey)) { + return $this->cache->get($cacheKey); } $job = null; @@ -86,7 +92,7 @@ public function getJob($shortcode, $params = []) if ($response && isset($response['id'])) { $job = WorkableResult::create($response); - $cache->set($cacheKey, $job); + $this->cache->set($cacheKey, $job); } return $job; @@ -102,11 +108,10 @@ public function getJob($shortcode, $params = []) */ public function getFullJobs($params = []) { - $cache = Injector::inst()->get(CacheInterface::class . '.workable'); $cacheKey = 'FullJobs' . implode($params, '-'); - if ($cache->has($cacheKey)) { - return $cache->get($cacheKey); + if ($this->cache->has($cacheKey)) { + return $this->cache->get($cacheKey); } $list = ArrayList::create(); @@ -118,7 +123,7 @@ public function getFullJobs($params = []) $list->push($job); } - $cache->set($cacheKey, $list); + $this->cache->set($cacheKey, $list); } return $list; @@ -129,7 +134,7 @@ public function getFullJobs($params = []) * @param string $url * @param array $params * @param string $method - * @return array JSON + * @return array JSON as array */ public function callRestfulService($url, $params = [], $method = 'GET') { @@ -137,13 +142,13 @@ public function callRestfulService($url, $params = [], $method = 'GET') $response = $this->restfulService->request($method, $url, ['query' => $params]); } catch (\RuntimeException $e) { Injector::inst()->get(LoggerInterface::class)->warning( - 'Failed to retreive valid response from workable', + 'Failed to retrieve valid response from workable', ['exception' => $e] ); return []; } - return Convert::json2array($response->getBody()); + return json_decode($response->getBody(), true); } /** @@ -151,7 +156,6 @@ public function callRestfulService($url, $params = [], $method = 'GET') */ public static function flush() { - $cache = Injector::inst()->get(CacheInterface::class . '.workable'); - $cache->clear(); + $this->cache->clear(); } } From d8081a92860c72ea23839b8b2004c41cea4f982b Mon Sep 17 00:00:00 2001 From: Andrew Aitken-Fincham Date: Fri, 6 Nov 2020 13:19:20 +1300 Subject: [PATCH 09/17] revert usage of this in a static function --- code/Workable.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code/Workable.php b/code/Workable.php index 7c09205..c2a8eed 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -156,6 +156,7 @@ public function callRestfulService($url, $params = [], $method = 'GET') */ public static function flush() { - $this->cache->clear(); + $cache = Injector::inst()->get(CacheInterface::class . '.workable'); + $cache->clear(); } } From 4dca957a6770a2e3b60fc52d59b22f550df35e4e Mon Sep 17 00:00:00 2001 From: Andrew Aitken-Fincham Date: Fri, 6 Nov 2020 16:08:40 +1300 Subject: [PATCH 10/17] fix implode deprecation warnings --- code/Workable.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/code/Workable.php b/code/Workable.php index c2a8eed..e346b15 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -52,8 +52,7 @@ public function __construct($restfulService, $cache) */ public function getJobs($params = []) { - $cacheKey = 'Jobs' . implode($params, '-'); - + $cacheKey = 'Jobs' . implode('-', $params); if ($this->cache->has($cacheKey)) { return $this->cache->get($cacheKey); } @@ -81,7 +80,7 @@ public function getJobs($params = []) */ public function getJob($shortcode, $params = []) { - $cacheKey = 'Job-' . $shortcode . implode($params, '-'); + $cacheKey = 'Job-' . $shortcode . implode('-', $params); if ($this->cache->has($cacheKey)) { return $this->cache->get($cacheKey); @@ -108,7 +107,7 @@ public function getJob($shortcode, $params = []) */ public function getFullJobs($params = []) { - $cacheKey = 'FullJobs' . implode($params, '-'); + $cacheKey = 'FullJobs' . implode('-', $params); if ($this->cache->has($cacheKey)) { return $this->cache->get($cacheKey); From cfd624432439a82b1f3bcf08d9ddf741d0f719ab Mon Sep 17 00:00:00 2001 From: Andrew Aitken-Fincham Date: Mon, 9 Nov 2020 11:21:54 +1300 Subject: [PATCH 11/17] php7-ify, early returns, move api key to config --- _config/config.yml | 4 +- code/Workable.php | 55 +++++++++++++++++--------- code/WorkableRestfulServiceFactory.php | 14 +++---- code/WorkableResult.php | 5 +-- composer.json | 2 +- 5 files changed, 48 insertions(+), 32 deletions(-) diff --git a/_config/config.yml b/_config/config.yml index 1103037..19ad3db 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -10,7 +10,9 @@ SilverStripe\Core\Injector\Injector: SilverStripe\Workable\Workable: constructor: 0: %$WorkableRestfulService - 1: %$Psr\SimpleCache\CacheInterface.workable + 1: '%$Psr\SimpleCache\CacheInterface.workable' WorkableRestfulService: class: GuzzleHttp\Client factory: SilverStripe\Workable\WorkableRestfulServiceFactory +SilverStripe\Workable\Workable: + apikey: '`WORKABLE_API_KEY`' \ No newline at end of file diff --git a/code/Workable.php b/code/Workable.php index e346b15..1ba4ea8 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -5,7 +5,6 @@ use Monolog\Logger; use RuntimeException; use Psr\Log\LoggerInterface; -use SilverStripe\Core\Convert; use SilverStripe\ORM\ArrayList; use SilverStripe\Core\Flushable; use SilverStripe\View\ArrayData; @@ -25,20 +24,32 @@ class Workable implements Flushable * Reference to the RestfulService dependency * @var RestfulService */ - protected $restfulService; + private $restfulService; /** * Reference to the Cache dependency * @var CacheInterface */ - protected $cache; + private $cache; + + /** + * API key used to connect to Workable, set via WORKABLE_API_KEY env var or in config + * @config + */ + private $apiKey; + + /** + * Subdomain for Workable API call (e.g. $subdomain.workable.com) + * @config + */ + private $subdomain; /** * Constructor, inject the restful service dependency * @param RestfulService $restfulService * @param CacheInterface $cache */ - public function __construct($restfulService, $cache) + public function __construct(RestfulService $restfulService, CacheInterface $cache) { $this->restfulService = $restfulService; $this->cache = $cache; @@ -50,7 +61,7 @@ public function __construct($restfulService, $cache) * see https://workable.readme.io/docs/jobs for full list of query params * @return ArrayList */ - public function getJobs($params = []) + public function getJobs(array $params = []): ArrayList { $cacheKey = 'Jobs' . implode('-', $params); if ($this->cache->has($cacheKey)) { @@ -60,14 +71,17 @@ public function getJobs($params = []) $list = ArrayList::create(); $response = $this->callRestfulService('jobs', $params); - if ($response && isset($response['jobs']) && is_array($response['jobs'])) { - foreach ($response['jobs'] as $record) { - $list->push(WorkableResult::create($record)); - } + if (!$response) { + return $list; + } - $this->cache->set($cacheKey, $list); + $jobs = $response['jobs'] ?? []; + foreach ($jobs as $record) { + $list->push(WorkableResult::create($record)); } + $this->cache->set($cacheKey, $list); + return $list; } @@ -78,7 +92,7 @@ public function getJobs($params = []) * see https://workable.readme.io/docs/jobs for full list of query params * @return WorkableResult|null */ - public function getJob($shortcode, $params = []) + public function getJob(string $shortcode, array $params = []): ?WorkableResult { $cacheKey = 'Job-' . $shortcode . implode('-', $params); @@ -116,15 +130,18 @@ public function getFullJobs($params = []) $list = ArrayList::create(); $response = $this->callRestfulService('jobs', $params); - if ($response && isset($response['jobs']) && is_array($response['jobs'])) { - foreach ($response['jobs'] as $record) { - $job = $this->getJob($record['shortcode'], $params); - $list->push($job); - } + if (!$response) { + return $list; + } - $this->cache->set($cacheKey, $list); + $jobs = $response['jobs'] ?? []; + foreach ($jobs as $record) { + $job = $this->getJob($record['shortcode'], $params); + $list->push($job); } + $this->cache->set($cacheKey, $list); + return $list; } @@ -135,7 +152,7 @@ public function getFullJobs($params = []) * @param string $method * @return array JSON as array */ - public function callRestfulService($url, $params = [], $method = 'GET') + public function callRestfulService(string $url, array $params = [], string $method = 'GET'): array { try { $response = $this->restfulService->request($method, $url, ['query' => $params]); @@ -155,7 +172,7 @@ public function callRestfulService($url, $params = [], $method = 'GET') */ public static function flush() { - $cache = Injector::inst()->get(CacheInterface::class . '.workable'); + $cache = static::singleton()->getCache(); $cache->clear(); } } diff --git a/code/WorkableRestfulServiceFactory.php b/code/WorkableRestfulServiceFactory.php index 02f3711..48150ca 100644 --- a/code/WorkableRestfulServiceFactory.php +++ b/code/WorkableRestfulServiceFactory.php @@ -3,7 +3,6 @@ namespace SilverStripe\Workable; use RuntimeException; -use SilverStripe\Core\Environment; use SilverStripe\Workable\Workable; use SilverStripe\Core\Injector\Factory; @@ -17,29 +16,30 @@ */ class WorkableRestfulServiceFactory implements Factory { + + /** * Create the RestfulService (or whatever dependency you've injected) * @return RestfulService */ public function create($service, array $params = []) { - $apiKey = Environment::getEnv('WORKABLE_API_KEY'); - $subdomain = Workable::config()->subdomain; + $config = Workable::config(); - if (!$apiKey) { + if (!$config->apiKey) { throw new RuntimeException('WORKABLE_API_KEY Environment variable not set'); } - if (!$subdomain) { + if (!$config->subdomain) { throw new RuntimeException( 'You must set a Workable subdomain in the config (SilverStripe\Workable\Workable.subdomain)' ); } return new $service([ - 'base_uri' => sprintf('https://%s.workable.com/spi/v3/', $subdomain), + 'base_uri' => sprintf('https://%s.workable.com/spi/v3/', $config->subdomain), 'headers' => [ - 'Authorization' => sprintf('Bearer %s', $apiKey), + 'Authorization' => sprintf('Bearer %s', $config->apiKey), ], ]); } diff --git a/code/WorkableResult.php b/code/WorkableResult.php index b4548ca..84e6e5c 100644 --- a/code/WorkableResult.php +++ b/code/WorkableResult.php @@ -26,10 +26,7 @@ public function __get($prop) { $snaked = ltrim(strtolower(preg_replace('/[A-Z]/', '_$0', $prop)), '_'); - if (!isset($this->apiData[$snaked])) { - return null; - } - $data = $this->apiData[$snaked]; + $data = $this->apiData[$snaked] ?? null; if (is_array($this->apiData[$snaked])) { return new WorkableResult($data); diff --git a/composer.json b/composer.json index 543777c..016444f 100644 --- a/composer.json +++ b/composer.json @@ -8,7 +8,7 @@ "require": { "silverstripe/framework": "^4", "guzzlehttp/guzzle": "^6", - "php": ">=5.6" + "php": ">=7.1" }, "authors":[ { From bdccef24bf2214339206726ec6cf695ef708958a Mon Sep 17 00:00:00 2001 From: Andrew Aitken-Fincham Date: Tue, 10 Nov 2020 09:57:34 +1300 Subject: [PATCH 12/17] rework based on testing --- _config/config.yml | 9 ++-- code/Workable.php | 65 +++++++++++++++++--------- code/WorkableRestfulServiceFactory.php | 27 ++++++++--- 3 files changed, 69 insertions(+), 32 deletions(-) diff --git a/_config/config.yml b/_config/config.yml index 19ad3db..ff3dfca 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -9,10 +9,11 @@ SilverStripe\Core\Injector\Injector: defaultLifetime: 3600 SilverStripe\Workable\Workable: constructor: - 0: %$WorkableRestfulService + 0: '%$GuzzleHttp\ClientInterface.workable' 1: '%$Psr\SimpleCache\CacheInterface.workable' - WorkableRestfulService: + SilverStripe\Workable\WorkableRestfulServiceFactory: + constructor: + apikey: '`WORKABLE_API_KEY`' + GuzzleHttp\ClientInterface.workable: class: GuzzleHttp\Client factory: SilverStripe\Workable\WorkableRestfulServiceFactory -SilverStripe\Workable\Workable: - apikey: '`WORKABLE_API_KEY`' \ No newline at end of file diff --git a/code/Workable.php b/code/Workable.php index 1ba4ea8..99a4b60 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -2,6 +2,8 @@ namespace SilverStripe\Workable; +use GuzzleHttp\ClientInterface; +use GuzzleHttp\Exception\ClientException; use Monolog\Logger; use RuntimeException; use Psr\Log\LoggerInterface; @@ -21,10 +23,10 @@ class Workable implements Flushable use Configurable; /** - * Reference to the RestfulService dependency - * @var RestfulService + * Reference to the HTTP Client dependency + * @var ClientInterface */ - private $restfulService; + private $httpClient; /** * Reference to the Cache dependency @@ -32,12 +34,6 @@ class Workable implements Flushable */ private $cache; - /** - * API key used to connect to Workable, set via WORKABLE_API_KEY env var or in config - * @config - */ - private $apiKey; - /** * Subdomain for Workable API call (e.g. $subdomain.workable.com) * @config @@ -46,12 +42,12 @@ class Workable implements Flushable /** * Constructor, inject the restful service dependency - * @param RestfulService $restfulService + * @param ClientInterface $httpClient * @param CacheInterface $cache */ - public function __construct(RestfulService $restfulService, CacheInterface $cache) + public function __construct(ClientInterface $httpClient, CacheInterface $cache) { - $this->restfulService = $restfulService; + $this->httpClient = $httpClient; $this->cache = $cache; } @@ -69,7 +65,7 @@ public function getJobs(array $params = []): ArrayList } $list = ArrayList::create(); - $response = $this->callRestfulService('jobs', $params); + $response = $this->callHttpClient('jobs', $params); if (!$response) { return $list; @@ -101,7 +97,7 @@ public function getJob(string $shortcode, array $params = []): ?WorkableResult } $job = null; - $response = $this->callRestfulService('jobs/' . $shortcode, $params); + $response = $this->callHttpClient('jobs/' . $shortcode, $params); if ($response && isset($response['id'])) { $job = WorkableResult::create($response); @@ -128,7 +124,7 @@ public function getFullJobs($params = []) } $list = ArrayList::create(); - $response = $this->callRestfulService('jobs', $params); + $response = $this->callHttpClient('jobs', $params); if (!$response) { return $list; @@ -150,29 +146,56 @@ public function getFullJobs($params = []) * @param string $url * @param array $params * @param string $method + * + * @throws RuntimeException if client is not configured correctly + * @throws ClientException if request fails + * * @return array JSON as array */ - public function callRestfulService(string $url, array $params = [], string $method = 'GET'): array + public function callHttpClient(string $url, array $params = [], string $method = 'GET'): array { try { - $response = $this->restfulService->request($method, $url, ['query' => $params]); + $response = $this->httpClient->request($method, $url, ['query' => $params]); } catch (\RuntimeException $e) { Injector::inst()->get(LoggerInterface::class)->warning( 'Failed to retrieve valid response from workable', ['exception' => $e] ); - return []; + + throw $e; } return json_decode($response->getBody(), true); } /** - * Clear the cache when flush is called + * Flush any cached data */ public static function flush() { - $cache = static::singleton()->getCache(); - $cache->clear(); + static::singleton()->getCache()->clear(); + } + + /** + * @return CacheInterface + */ + public function getCache(): CacheInterface + { + if (!$this->cache) { + $this->setCache(Injector::inst()->get(CacheInterface::class . '.workable')); + } + + return $this->cache; + } + + /** + * @param CacheInterface $cache + * @return self + */ + public function setCache(CacheInterface $cache): self + { + $this->cache = $cache; + + return $this; } } diff --git a/code/WorkableRestfulServiceFactory.php b/code/WorkableRestfulServiceFactory.php index 48150ca..2f4ac5b 100644 --- a/code/WorkableRestfulServiceFactory.php +++ b/code/WorkableRestfulServiceFactory.php @@ -2,6 +2,7 @@ namespace SilverStripe\Workable; +use GuzzleHttp\ClientInterface; use RuntimeException; use SilverStripe\Workable\Workable; use SilverStripe\Core\Injector\Factory; @@ -16,30 +17,42 @@ */ class WorkableRestfulServiceFactory implements Factory { + /** + * Set via ENV variable WORKABLE_API_KEY (see config.yml) + * @var string + */ + private $apiKey; - + public function __construct(?string $apiKey) + { + $this->apiKey = $apiKey; + } /** * Create the RestfulService (or whatever dependency you've injected) - * @return RestfulService + * + * @throws RuntimeException + * + * @return ClientInterface */ public function create($service, array $params = []) { - $config = Workable::config(); - if (!$config->apiKey) { + if (!$this->apiKey) { throw new RuntimeException('WORKABLE_API_KEY Environment variable not set'); } - if (!$config->subdomain) { + $subdomain = Workable::config()->subdomain; + + if (!$subdomain) { throw new RuntimeException( 'You must set a Workable subdomain in the config (SilverStripe\Workable\Workable.subdomain)' ); } return new $service([ - 'base_uri' => sprintf('https://%s.workable.com/spi/v3/', $config->subdomain), + 'base_uri' => sprintf('https://%s.workable.com/spi/v3/', $subdomain), 'headers' => [ - 'Authorization' => sprintf('Bearer %s', $config->apiKey), + 'Authorization' => sprintf('Bearer %s', $this->apiKey), ], ]); } From 468aef1ffd74a45814f4e08e60ca646959e0c21f Mon Sep 17 00:00:00 2001 From: Andrew Aitken-Fincham Date: Wed, 11 Nov 2020 14:38:13 +1300 Subject: [PATCH 13/17] private static for config --- code/Workable.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/Workable.php b/code/Workable.php index 99a4b60..9a89bc5 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -38,7 +38,7 @@ class Workable implements Flushable * Subdomain for Workable API call (e.g. $subdomain.workable.com) * @config */ - private $subdomain; + private static $subdomain; /** * Constructor, inject the restful service dependency From 1d41d3a0591f4349d5c5988042927656bb8d2931 Mon Sep 17 00:00:00 2001 From: Andrew Aitken-Fincham Date: Thu, 12 Nov 2020 10:45:59 +1300 Subject: [PATCH 14/17] fix tests, revert flush change --- code/Workable.php | 2 +- code/WorkableResult.php | 2 +- tests/TestWorkableRestfulService.php | 5 +++-- tests/WorkableTest.php | 27 +++++++++++++++++++-------- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/code/Workable.php b/code/Workable.php index 9a89bc5..c820449 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -173,7 +173,7 @@ public function callHttpClient(string $url, array $params = [], string $method = */ public static function flush() { - static::singleton()->getCache()->clear(); + Injector::inst()->get(CacheInterface::class . '.workable')->clear(); } /** diff --git a/code/WorkableResult.php b/code/WorkableResult.php index 84e6e5c..2b7bd04 100644 --- a/code/WorkableResult.php +++ b/code/WorkableResult.php @@ -28,7 +28,7 @@ public function __get($prop) $data = $this->apiData[$snaked] ?? null; - if (is_array($this->apiData[$snaked])) { + if (is_array($data)) { return new WorkableResult($data); } diff --git a/tests/TestWorkableRestfulService.php b/tests/TestWorkableRestfulService.php index 7967265..a50a243 100644 --- a/tests/TestWorkableRestfulService.php +++ b/tests/TestWorkableRestfulService.php @@ -2,11 +2,12 @@ namespace SilverStripe\Workable\Tests; +use GuzzleHttp\Client; use GuzzleHttp\Psr7\Response; -class TestWorkableRestfulService +class TestWorkableRestfulService extends Client { - public function request($method, $url, $params = []) + public function request($method, $url = '', $params = []) { switch ($url) { case 'jobs': diff --git a/tests/WorkableTest.php b/tests/WorkableTest.php index 7124e6d..2127ed0 100644 --- a/tests/WorkableTest.php +++ b/tests/WorkableTest.php @@ -2,26 +2,37 @@ namespace SilverStripe\Workable\Tests; +use GuzzleHttp\ClientInterface; use Psr\Log\LoggerInterface; +use Psr\SimpleCache\CacheInterface; +use SilverStripe\Config\Collections\CachedConfigCollection; +use SilverStripe\Core\Cache\DefaultCacheFactory; use SilverStripe\Core\Environment; +use SilverStripe\Core\Injector\InjectorLoader; use SilverStripe\Dev\SapphireTest; +use SilverStripe\Versioned\Caching\VersionedCacheAdapter; +use SilverStripe\Workable\Tests\TestWorkableRestfulService; use SilverStripe\Workable\Workable; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Workable\WorkableRestfulServiceFactory; use SilverStripe\Workable\WorkableResult; -use SilverStripe\Workable\Tests\TestWorkableRestfulService; class WorkableTest extends SapphireTest { - public function setUp() + public static function setUpBeforeClass() { - parent::setUp(); - $config = Config::inst()->get(Injector::class, 'WorkableRestfulService'); + parent::setUpBeforeClass(); + Workable::config()->set('subdomain', 'example'); + $config = Config::inst()->get(Injector::class, 'GuzzleHttp\ClientInterface.workable'); $config['class'] = TestWorkableRestfulService::class; - Config::inst()->update(Injector::class, 'WorkableRestfulService', $config); + Config::inst()->merge(Injector::class, 'GuzzleHttp\ClientInterface.workable', $config); + } + protected function setUp() + { + parent::setUp(); Environment::setEnv('WORKABLE_API_KEY', 'test'); - Config::inst()->update(Workable::class, 'subdomain', 'example'); } public function testThrowsIfNoSubdomain() @@ -29,7 +40,7 @@ public function testThrowsIfNoSubdomain() Config::inst()->remove(Workable::class, 'subdomain'); $this->setExpectedException('RuntimeException'); - Workable::create()->callRestfulService('test'); + Workable::create()->callHttpClient('test'); } public function testThrowsIfNoApiKey() @@ -37,7 +48,7 @@ public function testThrowsIfNoApiKey() Environment::setEnv('WORKABLE_API_KEY', null); $this->setExpectedException('RuntimeException'); - Workable::create()->callRestfulService('test'); + Workable::create()->callHttpClient('test'); } public function testConvertsSnakeCase() From 0b099be07d05e12530ca2c45fc6d4c722fd3d793 Mon Sep 17 00:00:00 2001 From: Danni Dickson Date: Mon, 5 Jul 2021 13:57:19 +1200 Subject: [PATCH 15/17] Add rate limit and sleep interval --- code/Workable.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/code/Workable.php b/code/Workable.php index c820449..cdb0f6b 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -165,6 +165,23 @@ public function callHttpClient(string $url, array $params = [], string $method = throw $e; } + + // remaining retries within the current interval. + $remainingRequests = $response->getHeader('X-Rate-Limit-Remaining'); + $sleepInterval = 10; + + // If we hit the rate-limit, let's back off and try again + if ($remainingRequests === 0 || $response->getStatusCode() === 429) { + $nextIntervalTimestamp = $response->getHeader('X-Rate-Limit-Reset'); + if (!empty($nextIntervalTimestamp)) { + time_sleep_until($nextIntervalTimestamp); + } else { + // or wait roughly until the next interval. + sleep($sleepInterval); + } + return $this->callRestfulService($url, $params, $method); + } + return json_decode($response->getBody(), true); } From fd3d84254f42a3bca379a014745ab93ba7084dba Mon Sep 17 00:00:00 2001 From: Danni Dickson Date: Mon, 5 Jul 2021 15:50:39 +1200 Subject: [PATCH 16/17] Use the values from the headers. --- code/Workable.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/code/Workable.php b/code/Workable.php index cdb0f6b..2c47c01 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -164,22 +164,22 @@ public function callHttpClient(string $url, array $params = [], string $method = throw $e; } + + // remaining retries within the current interval. + $remainingRequests = (int) $response->getHeader('X-Rate-Limit-Remaining')[0]; + $sleepInterval = 10; + // If we are about to hit the rate-limit, we wait for the next limit interval, or wait 10 seconds. + if ($remainingRequests === 1) { + $nextIntervalTimestamp = $response->getHeader('X-Rate-Limit-Reset')[0]; - // remaining retries within the current interval. - $remainingRequests = $response->getHeader('X-Rate-Limit-Remaining'); - $sleepInterval = 10; - - // If we hit the rate-limit, let's back off and try again - if ($remainingRequests === 0 || $response->getStatusCode() === 429) { - $nextIntervalTimestamp = $response->getHeader('X-Rate-Limit-Reset'); - if (!empty($nextIntervalTimestamp)) { - time_sleep_until($nextIntervalTimestamp); + if (!empty($nextIntervalTimestamp)) { + time_sleep_until($nextIntervalTimestamp); } else { - // or wait roughly until the next interval. sleep($sleepInterval); } - return $this->callRestfulService($url, $params, $method); + + return $this->callHttpClient($url, $params, $method); } return json_decode($response->getBody(), true); From 293c56654c36b212378fefc7e9cbbc785c7549d6 Mon Sep 17 00:00:00 2001 From: Danni Dickson Date: Wed, 7 Jul 2021 11:58:02 +1200 Subject: [PATCH 17/17] Handle Rate Limit in catch block, this avoids code repeatition. --- code/Workable.php | 94 ++++++++++++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 38 deletions(-) diff --git a/code/Workable.php b/code/Workable.php index 2c47c01..2f11339 100644 --- a/code/Workable.php +++ b/code/Workable.php @@ -3,13 +3,10 @@ namespace SilverStripe\Workable; use GuzzleHttp\ClientInterface; -use GuzzleHttp\Exception\ClientException; -use Monolog\Logger; -use RuntimeException; +use GuzzleHttp\Exception\RequestException; use Psr\Log\LoggerInterface; use SilverStripe\ORM\ArrayList; use SilverStripe\Core\Flushable; -use SilverStripe\View\ArrayData; use SilverStripe\Core\Extensible; use Psr\SimpleCache\CacheInterface; use SilverStripe\Core\Injector\Injector; @@ -65,7 +62,7 @@ public function getJobs(array $params = []): ArrayList } $list = ArrayList::create(); - $response = $this->callHttpClient('jobs', $params); + $response = $this->callWorkableApi('jobs', $params); if (!$response) { return $list; @@ -97,7 +94,7 @@ public function getJob(string $shortcode, array $params = []): ?WorkableResult } $job = null; - $response = $this->callHttpClient('jobs/' . $shortcode, $params); + $response = $this->callWorkableApi('jobs/' . $shortcode, $params); if ($response && isset($response['id'])) { $job = WorkableResult::create($response); @@ -109,8 +106,7 @@ public function getJob(string $shortcode, array $params = []): ?WorkableResult /** * Gets all the jobs from the workable API, populating each job with its full data - * Note: This calls the API multiple times so should be used with caution, see - * rate limiting docs https://workable.readme.io/docs/rate-limits + * Note: This calls the API multiple times so should be used with caution * @param array $params Array of params, e.g. ['state' => 'published']. * see https://workable.readme.io/docs/jobs for full list of query params * @return ArrayList @@ -124,7 +120,7 @@ public function getFullJobs($params = []) } $list = ArrayList::create(); - $response = $this->callHttpClient('jobs', $params); + $response = $this->callWorkableApi('jobs', $params); if (!$response) { return $list; @@ -142,47 +138,67 @@ public function getFullJobs($params = []) } /** - * Wrapper method to configure the RestfulService, make the call, and handle errors + * Sends request to Workable API. + * Should it exceed the rate limit, this is caught and put to sleep until the next interval. + * The interval duration is provided by Workable via a header. + * When its awaken, it will call itself again, this repeats until its complete. + * This returns a json body from the response. + * + * Note: See rate limit docs from Workable https://workable.readme.io/docs/rate-limits * @param string $url * @param array $params * @param string $method * - * @throws RuntimeException if client is not configured correctly - * @throws ClientException if request fails - * + * @throws RequestException if client is not configured correctly, handles 429 error + * @return array JSON as array */ - public function callHttpClient(string $url, array $params = [], string $method = 'GET'): array + public function callWorkableApi(string $url, array $params = [], string $method = 'GET'): array { try { $response = $this->httpClient->request($method, $url, ['query' => $params]); - } catch (\RuntimeException $e) { - Injector::inst()->get(LoggerInterface::class)->warning( - 'Failed to retrieve valid response from workable', - ['exception' => $e] - ); - - throw $e; - } - - // remaining retries within the current interval. - $remainingRequests = (int) $response->getHeader('X-Rate-Limit-Remaining')[0]; - $sleepInterval = 10; - - // If we are about to hit the rate-limit, we wait for the next limit interval, or wait 10 seconds. - if ($remainingRequests === 1) { - $nextIntervalTimestamp = $response->getHeader('X-Rate-Limit-Reset')[0]; - - if (!empty($nextIntervalTimestamp)) { - time_sleep_until($nextIntervalTimestamp); - } else { - sleep($sleepInterval); + return json_decode($response->getBody(), true); + } + catch(RequestException $e){ + if($e->hasResponse()){ + $errorResponse = $e->getResponse(); + $statusCode = $errorResponse->getStatusCode(); + + if($statusCode === 429) { + Injector::inst()->get(LoggerInterface::class)->info( + 'Rate limit exceeded - sleeping until next interval' + ); + + $this->sleepUntil($errorResponse->getHeader('X-Rate-Limit-Reset')); + + return $this->callWorkableApi($url, $params, $method); + } + else { + Injector::inst()->get(LoggerInterface::class)->warning( + 'Failed to retrieve valid response from workable', + ['exception' => $e] + ); + + throw $e; + } } - - return $this->callHttpClient($url, $params, $method); } + } - return json_decode($response->getBody(), true); + /** + * Sleeps until the next interval. + * Should the interval header be empty, the script sleeps for 10 seconds - Workable's default interval. + * @param array $resetIntervalHeader + */ + private function sleepUntil($resetIntervalHeader){ + $defaultSleepInterval = 10; + + if(!empty($resetIntervalHeader)){ + time_sleep_until($resetIntervalHeader[0]); + } + else { + sleep($defaultSleepInterval); + } } /** @@ -194,6 +210,7 @@ public static function flush() } /** + * Gets any cached data. If there is no cached data, a blank cache is created. * @return CacheInterface */ public function getCache(): CacheInterface @@ -206,6 +223,7 @@ public function getCache(): CacheInterface } /** + * Sets the cache. * @param CacheInterface $cache * @return self */