Make HttpAdapter explicit dependency for PublicSuffixListManager (#200)

Author: jeremykendall

bin/update-psl

@@ -21,7 +21,11 @@ if ($argc == 2 && is_dir($argv[1])) {
 
 try {
     echo 'Updating Public Suffix List.' . PHP_EOL;
-    $manager = new \Pdp\PublicSuffixListManager($cacheDir);
+    /**
+     * @TODO: Having the adapter baked in with no way for the library user to provide
+     * their own is problematic. Fix it or leave it? No one has ever complained.
+     */
+    $manager = new \Pdp\PublicSuffixListManager(new \Pdp\Http\CurlHttpAdapter(), $cacheDir);
     $manager->refreshPublicSuffixList();
     echo 'Update complete.' . PHP_EOL;
     exit(0);

src/HttpAdapter/CurlHttpAdapter.php renamed to src/Http/CurlHttpAdapter.php

@@ -9,7 +9,7 @@
  */
 declare(strict_types=1);
 
-namespace Pdp\HttpAdapter;
+namespace Pdp\Http;
 
 /**
  * cURL http adapter.
@@ -22,7 +22,7 @@
  * @author William Durand <[email protected]>
  * @author Jeremy Kendall <[email protected]>
  */
-class CurlHttpAdapter implements HttpAdapterInterface
+class CurlHttpAdapter implements HttpAdapter
 {
     /**
      * {@inheritdoc}

src/HttpAdapter/HttpAdapterInterface.php renamed to src/Http/HttpAdapter.php

@@ -9,7 +9,7 @@
  */
 declare(strict_types=1);
 
-namespace Pdp\HttpAdapter;
+namespace Pdp\Http;
 
 /**
  * Interface for http adapters.
@@ -22,7 +22,7 @@
  * @author William Durand <[email protected]>
  * @author Jeremy Kendall <[email protected]>
  */
-interface HttpAdapterInterface
+interface HttpAdapter
 {
     /**
      * Returns the content fetched from a given URL.

src/PublicSuffixListManager.php

@@ -12,8 +12,7 @@
 namespace Pdp;
 
 use Exception;
-use Pdp\HttpAdapter\CurlHttpAdapter;
-use Pdp\HttpAdapter\HttpAdapterInterface;
+use Pdp\Http\HttpAdapter;
 use SplFileObject;
 
 /**
@@ -51,42 +50,22 @@ class PublicSuffixListManager
     private $cacheDir;
 
     /**
-     * @var HttpAdapterInterface Http adapter
+     * @var HttpAdapter Http adapter
      */
     private $httpAdapter;
 
     /**
      * Public constructor.
      *
-     * @param string $cacheDir Optional cache directory
+     * @param HttpAdapter $httpAdapter
+     * @param string      $cacheDir    Optional cache directory
      */
-    public function __construct(string $cacheDir = null)
+    public function __construct(HttpAdapter $httpAdapter, string $cacheDir = null)
     {
         $this->cacheDir = $cacheDir ?? realpath(dirname(__DIR__) . DIRECTORY_SEPARATOR . 'data');
-    }
-
-    /**
-     * Sets http adapter.
-     *
-     * @param HttpAdapterInterface $httpAdapter
-     */
-    public function setHttpAdapter(HttpAdapterInterface $httpAdapter)
-    {
         $this->httpAdapter = $httpAdapter;
     }
 
-    /**
-     * Returns http adapter. Returns default http adapter if one is not set.
-     *
-     * @return HttpAdapterInterface
-     */
-    public function getHttpAdapter(): HttpAdapterInterface
-    {
-        $this->httpAdapter = $this->httpAdapter ?? new CurlHttpAdapter();
-
-        return $this->httpAdapter;
-    }
-
     /**
      * Gets Public Suffix List.
      *
@@ -111,7 +90,7 @@ public function getList($list = self::ALL_DOMAINS): PublicSuffixList
      */
     public function refreshPublicSuffixList()
     {
-        $publicSuffixList = $this->getHttpAdapter()->getContent(self::PUBLIC_SUFFIX_LIST_URL);
+        $publicSuffixList = $this->httpAdapter->getContent(self::PUBLIC_SUFFIX_LIST_URL);
         $this->cache(self::PDP_PSL_TEXT_FILE, $publicSuffixList);
 
         $publicSuffixListArray = $this->convertListToArray();

tests/HttpAdapter/CurlHttpAdapterTest.php

@@ -2,7 +2,7 @@
 
 namespace Pdp\Tests\HttpAdapter;
 
-use Pdp\HttpAdapter\CurlHttpAdapter;
+use Pdp\Http\CurlHttpAdapter;
 use PHPUnit\Framework\TestCase;
 
 /**

tests/PublicSuffixListManagerTest.php

@@ -7,8 +7,8 @@
 use Exception;
 use org\bovigo\vfs\vfsStream;
 use org\bovigo\vfs\vfsStreamDirectory;
-use Pdp\HttpAdapter\CurlHttpAdapter;
-use Pdp\HttpAdapter\HttpAdapterInterface;
+use Pdp\Http\CurlHttpAdapter;
+use Pdp\Http\HttpAdapter;
 use Pdp\PublicSuffixList;
 use Pdp\PublicSuffixListManager;
 use PHPUnit\Framework\TestCase;
@@ -46,7 +46,7 @@ class PublicSuffixListManagerTest extends TestCase
     protected $dataDir;
 
     /**
-     * @var HttpAdapterInterface|\PHPUnit_Framework_MockObject_MockObject Http adapter
+     * @var HttpAdapter|\PHPUnit_Framework_MockObject_MockObject Http adapter
      */
     protected $httpAdapter;
 
@@ -60,10 +60,9 @@ protected function setUp()
         vfsStream::create(['cache' => []], $this->root);
         $this->cacheDir = vfsStream::url('pdp/cache');
 
-        $this->listManager = new PublicSuffixListManager($this->cacheDir);
+        $this->httpAdapter = $this->getMock(HttpAdapter::class);
 
-        $this->httpAdapter = $this->getMock(HttpAdapterInterface::class);
-        $this->listManager->setHttpAdapter($this->httpAdapter);
+        $this->listManager = new PublicSuffixListManager($this->httpAdapter, $this->cacheDir);
     }
 
     protected function tearDown()
@@ -104,17 +103,11 @@ public function testRefreshPublicSuffixList()
         );
     }
 
-    public function testGetHttpAdapterReturnsDefaultCurlAdapterIfAdapterNotSet()
-    {
-        $listManager = new PublicSuffixListManager($this->cacheDir);
-        $this->assertInstanceOf(CurlHttpAdapter::class, $listManager->getHttpAdapter());
-    }
-
     public function testWriteThrowsExceptionIfCanNotWrite()
     {
         $this->expectException(Exception::class);
         $this->expectExceptionMessage("Cannot write '/does/not/exist/public-suffix-list.txt'");
-        $manager = new PublicSuffixListManager('/does/not/exist');
+        $manager = new PublicSuffixListManager(new CurlHttpAdapter(), '/does/not/exist');
         $manager->refreshPublicSuffixList();
     }
 
@@ -142,7 +135,7 @@ public function testGetListWithoutCache()
 
         /** @var PublicSuffixListManager|\PHPUnit_Framework_MockObject_MockObject $listManager */
         $listManager = $this->getMockBuilder(PublicSuffixListManager::class)
-            ->setConstructorArgs([$this->cacheDir])
+            ->setConstructorArgs([$this->httpAdapter, $this->cacheDir])
             ->setMethods(['refreshPublicSuffixList'])
             ->getMock();
 
@@ -162,7 +155,7 @@ public function testGetListWithoutCache()
     public function testGetProvidedListFromDefaultCacheDir()
     {
         // By not providing cache I'm forcing use of default cache dir
-        $listManager = new PublicSuffixListManager();
+        $listManager = new PublicSuffixListManager($this->httpAdapter);
         $publicSuffixList = $listManager->getList();
         $this->assertGreaterThanOrEqual(300, count($publicSuffixList->getRules()));
     }
@@ -176,7 +169,7 @@ private function getMock(string $class)
 
     public function testGetDifferentPublicList()
     {
-        $listManager = new PublicSuffixListManager();
+        $listManager = new PublicSuffixListManager($this->httpAdapter);
         $publicList = $listManager->getList();
         $invalidList = $listManager->getList('invalid type');
         $this->assertEquals($publicList, $invalidList);