Merge pull request #2 from andrewandante/feature/ss4

Responds to PR feedback from UC
silverstripe-archive · Nov 11, 2020 · afe77cc · afe77cc
2 parents fae31a1 + 468aef1
commit afe77cc
Show file tree

Hide file tree

Showing 5 changed files with 97 additions and 44 deletions.
diff --git a/_config/config.yml b/_config/config.yml
@@ -9,8 +9,11 @@ SilverStripe\Core\Injector\Injector:
       defaultLifetime: 3600
   SilverStripe\Workable\Workable:
     constructor:
-      0: %$WorkableRestfulService
-      1: %$Psr\SimpleCache\CacheInterface.workable
-  WorkableRestfulService:
+      0: '%$GuzzleHttp\ClientInterface.workable'
+      1: '%$Psr\SimpleCache\CacheInterface.workable'
+  SilverStripe\Workable\WorkableRestfulServiceFactory:
+    constructor:
+      apikey: '`WORKABLE_API_KEY`'
+  GuzzleHttp\ClientInterface.workable:
     class: GuzzleHttp\Client
     factory: SilverStripe\Workable\WorkableRestfulServiceFactory
diff --git a/code/Workable.php b/code/Workable.php
@@ -2,10 +2,11 @@
 
 namespace SilverStripe\Workable;
 
+use GuzzleHttp\ClientInterface;
+use GuzzleHttp\Exception\ClientException;
 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;
@@ -22,25 +23,31 @@ class Workable implements Flushable
     use Configurable;
 
     /**
-     * Reference to the RestfulService dependency
-     * @var RestfulService
+     * Reference to the HTTP Client dependency
+     * @var ClientInterface
      */
-    protected $restfulService;
+    private $httpClient;
 
     /**
      * Reference to the Cache dependency
      * @var CacheInterface
      */
-    protected $cache;
+    private $cache;
+
+    /**
+     * Subdomain for Workable API call (e.g. $subdomain.workable.com)
+     * @config
+     */
+    private static $subdomain;
 
     /**
      * Constructor, inject the restful service dependency
-     * @param RestfulService $restfulService
+     * @param ClientInterface $httpClient
      * @param CacheInterface $cache
      */
-    public function __construct($restfulService, $cache)
+    public function __construct(ClientInterface $httpClient, CacheInterface $cache)
     {
-        $this->restfulService = $restfulService;
+        $this->httpClient = $httpClient;
         $this->cache = $cache;
     }
 
@@ -50,24 +57,27 @@ 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)) {
             return $this->cache->get($cacheKey);
         }
 
         $list = ArrayList::create();
-        $response = $this->callRestfulService('jobs', $params);
+        $response = $this->callHttpClient('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 +88,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);
 
@@ -87,7 +97,7 @@ public function getJob($shortcode, $params = [])
         }
 
         $job = null;
-        $response = $this->callRestfulService('jobs/' . $shortcode, $params);
+        $response = $this->callHttpClient('jobs/' . $shortcode, $params);
 
         if ($response && isset($response['id'])) {
             $job = WorkableResult::create($response);
@@ -114,17 +124,20 @@ public function getFullJobs($params = [])
         }
 
         $list = ArrayList::create();
-        $response = $this->callRestfulService('jobs', $params);
+        $response = $this->callHttpClient('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;
     }
 
@@ -133,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($url, $params = [], $method = 'GET')
+    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 = Injector::inst()->get(CacheInterface::class . '.workable');
-        $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
@@ -2,8 +2,8 @@
 
 namespace SilverStripe\Workable;
 
+use GuzzleHttp\ClientInterface;
 use RuntimeException;
-use SilverStripe\Core\Environment;
 use SilverStripe\Workable\Workable;
 use SilverStripe\Core\Injector\Factory;
 
@@ -17,19 +17,32 @@
  */
 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 = [])
     {
-        $apiKey = Environment::getEnv('WORKABLE_API_KEY');
-        $subdomain = Workable::config()->subdomain;
 
-        if (!$apiKey) {
+        if (!$this->apiKey) {
             throw new RuntimeException('WORKABLE_API_KEY Environment variable not set');
         }
 
+        $subdomain = Workable::config()->subdomain;
+
         if (!$subdomain) {
             throw new RuntimeException(
                 'You must set a Workable subdomain in the config (SilverStripe\Workable\Workable.subdomain)'
@@ -39,7 +52,7 @@ public function create($service, array $params = [])
         return new $service([
             'base_uri' => sprintf('https://%s.workable.com/spi/v3/', $subdomain),
             'headers' => [
-                'Authorization' => sprintf('Bearer %s', $apiKey),
+                'Authorization' => sprintf('Bearer %s', $this->apiKey),
             ],
         ]);
     }

diff --git 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
@@ -8,7 +8,7 @@
   "require": {
     "silverstripe/framework": "^4",
     "guzzlehttp/guzzle": "^6",
-    "php": ">=5.6"
+    "php": ">=7.1"
   },
   "authors":[
     {