From 6018a6dcaa59bf8a57cfde0f8ecf7f01a321a0dd Mon Sep 17 00:00:00 2001
From: Nabeel S <nabeelio@users.noreply.github.com>
Date: Thu, 22 Aug 2019 14:32:49 -0400
Subject: [PATCH] Add Contract interface for airport lookup functionality
 (#365)

* Add Contract interface for airport lookup functionality

* style ci fixes
---
 app/Contracts/AirportLookup.php               | 27 +++++++++++++
 .../Controllers/Api/AirportController.php     | 24 ++++--------
 app/Providers/BindServiceProviders.php        | 29 ++++++++++++++
 app/Providers/WeatherServiceProvider.php      | 17 ---------
 .../AirportLookup/VaCentralLookup.php         | 28 ++++++++++++++
 app/Services/AirportService.php               | 38 +++++++++++++++++++
 config/app.php                                |  2 +-
 config/phpvms.php                             |  8 +++-
 phpunit.xml                                   |  2 +-
 9 files changed, 138 insertions(+), 37 deletions(-)
 create mode 100644 app/Contracts/AirportLookup.php
 create mode 100755 app/Providers/BindServiceProviders.php
 delete mode 100755 app/Providers/WeatherServiceProvider.php
 create mode 100644 app/Services/AirportLookup/VaCentralLookup.php

diff --git a/app/Contracts/AirportLookup.php b/app/Contracts/AirportLookup.php
new file mode 100644
index 00000000..80097eaf
--- /dev/null
+++ b/app/Contracts/AirportLookup.php
@@ -0,0 +1,27 @@
+<?php
+
+namespace App\Contracts;
+
+use App\Models\Airport;
+
+abstract class AirportLookup
+{
+    /**
+     * Lookup the information for an airport. Needs to return an instance of the
+     * Airport model, or an array with the properties listed in the Airport model.
+     *
+     * The in-use implementation can be changed in the config/phpvms.php file, so
+     * different services can be used, in-case vaCentral one isn't working or there
+     * is a better one available. Don't handle any caching in this layer, that happens
+     * at the service layer
+     *
+     * Return null if there's an error or nothing was found
+     *
+     * @example App\Services\AirportLookup\VaCentralLookup
+     *
+     * @param string $icao
+     *
+     * @return Airport|null
+     */
+    abstract public function getAirport($icao);
+}
diff --git a/app/Http/Controllers/Api/AirportController.php b/app/Http/Controllers/Api/AirportController.php
index f52b5055..e39650cc 100644
--- a/app/Http/Controllers/Api/AirportController.php
+++ b/app/Http/Controllers/Api/AirportController.php
@@ -5,10 +5,8 @@ namespace App\Http\Controllers\Api;
 use App\Contracts\Controller;
 use App\Http\Resources\Airport as AirportResource;
 use App\Repositories\AirportRepository;
+use App\Services\AirportService;
 use Illuminate\Http\Request;
-use Illuminate\Support\Facades\Cache;
-use Log;
-use VaCentral\Airport as AirportLookup;
 
 /**
  * Class AirportController
@@ -16,16 +14,20 @@ use VaCentral\Airport as AirportLookup;
 class AirportController extends Controller
 {
     private $airportRepo;
+    private $airportSvc;
 
     /**
      * AirportController constructor.
      *
      * @param AirportRepository $airportRepo
+     * @param AirportService    $airportSvc
      */
     public function __construct(
-        AirportRepository $airportRepo
+        AirportRepository $airportRepo,
+        AirportService $airportSvc
     ) {
         $this->airportRepo = $airportRepo;
+        $this->airportSvc = $airportSvc;
     }
 
     /**
@@ -88,19 +90,7 @@ class AirportController extends Controller
      */
     public function lookup($id)
     {
-        $airport = Cache::remember(
-            config('cache.keys.AIRPORT_VACENTRAL_LOOKUP.key').$id,
-            config('cache.keys.AIRPORT_VACENTRAL_LOOKUP.time'),
-            function () use ($id) {
-                try {
-                    return AirportLookup::get($id);
-                } catch (\VaCentral\HttpException $e) {
-                    Log::error($e);
-                    return [];
-                }
-            }
-        );
-
+        $airport = $this->airportSvc->lookupAirport($id);
         return new AirportResource(collect($airport));
     }
 }
diff --git a/app/Providers/BindServiceProviders.php b/app/Providers/BindServiceProviders.php
new file mode 100755
index 00000000..b6a88a52
--- /dev/null
+++ b/app/Providers/BindServiceProviders.php
@@ -0,0 +1,29 @@
+<?php
+
+namespace App\Providers;
+
+use App\Contracts\AirportLookup;
+use App\Contracts\Metar;
+use Illuminate\Support\ServiceProvider;
+
+class BindServiceProviders extends ServiceProvider
+{
+    public function boot(): void
+    {
+        /*
+         * Bind the class used to fullfill the Metar class contract
+         */
+        $this->app->bind(
+            Metar::class,
+            config('phpvms.metar_lookup')
+        );
+
+        /*
+         * Bind the class used to fullfill the AirportLookup class contract
+         */
+        $this->app->bind(
+            AirportLookup::class,
+            config('phpvms.airport_lookup')
+        );
+    }
+}
diff --git a/app/Providers/WeatherServiceProvider.php b/app/Providers/WeatherServiceProvider.php
deleted file mode 100755
index 3e84c531..00000000
--- a/app/Providers/WeatherServiceProvider.php
+++ /dev/null
@@ -1,17 +0,0 @@
-<?php
-
-namespace App\Providers;
-
-use App\Contracts\Metar;
-use Illuminate\Support\ServiceProvider;
-
-class WeatherServiceProvider extends ServiceProvider
-{
-    public function boot(): void
-    {
-        $this->app->bind(
-            Metar::class,
-            config('phpvms.metar')
-        );
-    }
-}
diff --git a/app/Services/AirportLookup/VaCentralLookup.php b/app/Services/AirportLookup/VaCentralLookup.php
new file mode 100644
index 00000000..930c8842
--- /dev/null
+++ b/app/Services/AirportLookup/VaCentralLookup.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace App\Services\AirportLookup;
+
+use App\Contracts\AirportLookup;
+use Illuminate\Support\Facades\Log;
+use VaCentral\Airport;
+use VaCentral\HttpException;
+
+class VaCentralLookup extends AirportLookup
+{
+    /**
+     * Lookup the information for an airport
+     *
+     * @param string $icao
+     *
+     * @return array
+     */
+    public function getAirport($icao)
+    {
+        try {
+            return Airport::get($icao);
+        } catch (HttpException $e) {
+            Log::error($e);
+            return;
+        }
+    }
+}
diff --git a/app/Services/AirportService.php b/app/Services/AirportService.php
index 05d60460..efb85336 100644
--- a/app/Services/AirportService.php
+++ b/app/Services/AirportService.php
@@ -2,20 +2,27 @@
 
 namespace App\Services;
 
+use App\Contracts\AirportLookup as AirportLookupProvider;
 use App\Contracts\Metar as MetarProvider;
 use App\Contracts\Service;
 use App\Support\Metar;
+use Illuminate\Support\Facades\Cache;
+use VaCentral\Airport;
 
 /**
  * Class AnalyticsService
  */
 class AirportService extends Service
 {
+    private $lookupProvider;
     private $metarProvider;
 
     public function __construct(
+        AirportLookupProvider $lookupProvider,
         MetarProvider $metarProvider
+
     ) {
+        $this->lookupProvider = $lookupProvider;
         $this->metarProvider = $metarProvider;
     }
 
@@ -38,4 +45,35 @@ class AirportService extends Service
             return new Metar($raw_metar);
         }
     }
+
+    /**
+     * Lookup an airport's information from a remote provider. This handles caching
+     * the data internally
+     *
+     * @param string $icao ICAO
+     *
+     * @return Airport|array
+     */
+    public function lookupAirport($icao)
+    {
+        $key = config('cache.keys.AIRPORT_VACENTRAL_LOOKUP.key').$icao;
+
+        $airport = Cache::get($key);
+        if ($airport) {
+            return $airport;
+        }
+
+        $airport = $this->lookupProvider->getAirport($icao);
+        if ($airport === null) {
+            return [];
+        }
+
+        Cache::add(
+            $key,
+            $airport,
+            config('cache.keys.AIRPORT_VACENTRAL_LOOKUP.time')
+        );
+
+        return $airport;
+    }
 }
diff --git a/config/app.php b/config/app.php
index 09574c41..2843d633 100755
--- a/config/app.php
+++ b/config/app.php
@@ -87,7 +87,7 @@ return [
         App\Providers\vaCentralServiceProvider::class,
         App\Providers\ExtendedTimezonelistProvider::class,
         App\Providers\MeasurementsProvider::class,
-        App\Providers\WeatherServiceProvider::class,
+        App\Providers\BindServiceProviders::class,
     ],
 
     'aliases' => [
diff --git a/config/phpvms.php b/config/phpvms.php
index 61b66a3c..3f890fcb 100644
--- a/config/phpvms.php
+++ b/config/phpvms.php
@@ -47,7 +47,13 @@ return [
      * Point to the class to use to retrieve the METAR string. If this
      * goes inactive at some date, it can be replaced
      */
-    'metar' => App\Services\Metar\AviationWeather::class,
+    'metar_lookup' => App\Services\Metar\AviationWeather::class,
+
+    /*
+     * Point to the class used to retrieve the airport information.
+     * If this goes inactive at some date, it can be replaced
+     */
+    'airport_lookup' => App\Services\AirportLookup\VaCentralLookup::class,
 
     /*
      * Your vaCentral API key
diff --git a/phpunit.xml b/phpunit.xml
index 09c8eced..874dadaf 100755
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -26,7 +26,7 @@
         <env name="APP_ENV" value="unittest"/>
         <env name="APP_KEY" value="base64:ve66Z5Kt/zTN3p++0zOPu854PHfZkwJE5VuoFAlzHtI="/>
         <env name="APP_DEBUG" value="true"/>
-        <env name="APP_LOG_LEVEL" value="info"/>
+        <env name="APP_LOG_LEVEL" value="error"/>
         <env name="DB_CONNECTION" value="memory"/>
         <env name="CACHE_DRIVER" value="array"/>
         <env name="SESSION_DRIVER" value="array"/>