Make LoginFormAuthenticator a service and remove Factory

src/Guzzle/AuthenticatorSubscriber.php

@@ -9,7 +9,7 @@ use GuzzleHttp\Message\RequestInterface;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
-use Wallabag\SiteConfig\Authenticator\Factory;
+use Wallabag\SiteConfig\Authenticator\Authenticator;
 use Wallabag\SiteConfig\SiteConfig;
 use Wallabag\SiteConfig\SiteConfigBuilder;
 
@@ -23,8 +23,8 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa
     /** @var SiteConfigBuilder */
     private $configBuilder;
 
-    /** @var Factory */
+    /** @var Authenticator */
-    private $authenticatorFactory;
+    private $authenticator;
 
     /** @var LoggerInterface */
     private $logger;
@@ -32,10 +32,10 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa
     /**
      * AuthenticatorSubscriber constructor.
      */
-    public function __construct(SiteConfigBuilder $configBuilder, Factory $authenticatorFactory)
+    public function __construct(SiteConfigBuilder $configBuilder, Authenticator $authenticator)
     {
         $this->configBuilder = $configBuilder;
-        $this->authenticatorFactory = $authenticatorFactory;
+        $this->authenticator = $authenticator;
         $this->logger = new NullLogger();
     }
 
@@ -62,14 +62,13 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa
         }
 
         $client = $event->getClient();
-        $authenticator = $this->authenticatorFactory->buildFromSiteConfig();
 
-        if (!$authenticator->isLoggedIn($config, $client)) {
+        if (!$this->authenticator->isLoggedIn($config, $client)) {
             $this->logger->debug('loginIfRequired> user is not logged in, attach authenticator');
 
             $emitter = $client->getEmitter();
             $emitter->detach($this);
-            $authenticator->login($config, $client);
+            $this->authenticator->login($config, $client);
             $emitter->attach($this);
         }
     }
@@ -94,8 +93,7 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa
             return;
         }
 
-        $authenticator = $this->authenticatorFactory->buildFromSiteConfig();
+        $isLoginRequired = $this->authenticator->isLoginRequired($config, $body);
-        $isLoginRequired = $authenticator->isLoginRequired($config, $body);
 
         $this->logger->debug('loginIfRequested> retry #' . $this->retries . ' with login ' . ($isLoginRequired ? '' : 'not ') . 'required');
 
@@ -104,7 +102,7 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa
 
             $emitter = $client->getEmitter();
             $emitter->detach($this);
-            $authenticator->login($config, $client);
+            $this->authenticator->login($config, $client);
             $emitter->attach($this);
 
             $event->retry();

src/SiteConfig/Authenticator/Factory.php

@@ -1,21 +0,0 @@
-<?php
-
-namespace Wallabag\SiteConfig\Authenticator;
-
-use Wallabag\SiteConfig\SiteConfig;
-
-/**
- * Builds an Authenticator based on a SiteConfig.
- */
-class Factory
-{
-    /**
-     * @return Authenticator
-     *
-     * @throw \OutOfRangeException if there are no credentials for this host
-     */
-    public function buildFromSiteConfig()
-    {
-        return new LoginFormAuthenticator();
-    }
-}

src/SiteConfig/Authenticator/LoginFormAuthenticator.php

@@ -16,7 +16,7 @@ class LoginFormAuthenticator implements Authenticator
         $postFields = [
             $siteConfig->getUsernameField() => $siteConfig->getUsername(),
             $siteConfig->getPasswordField() => $siteConfig->getPassword(),
-        ] + $this->getExtraFields($guzzle);
+        ] + $this->getExtraFields($siteConfig, $guzzle);
 
         $guzzle->post(
             $siteConfig->getLoginUri(),

tests/Guzzle/AuthenticatorSubscriberTest.php

@@ -15,15 +15,18 @@ use PHPUnit\Framework\TestCase;
 use Wallabag\Guzzle\AuthenticatorSubscriber;
 use Wallabag\SiteConfig\ArraySiteConfigBuilder;
 use Wallabag\SiteConfig\Authenticator\Authenticator;
-use Wallabag\SiteConfig\Authenticator\Factory;
 
 class AuthenticatorSubscriberTest extends TestCase
 {
     public function testGetEvents()
     {
+        $authenticator = $this->getMockBuilder(Authenticator::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
         $subscriber = new AuthenticatorSubscriber(
             new ArraySiteConfigBuilder(),
-            new Factory()
+            $authenticator
         );
         $events = $subscriber->getEvents();
 
@@ -35,8 +38,12 @@ class AuthenticatorSubscriberTest extends TestCase
 
     public function testLoginIfRequiredNotRequired()
     {
+        $authenticator = $this->getMockBuilder(Authenticator::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
         $builder = new ArraySiteConfigBuilder(['example.com' => []]);
-        $subscriber = new AuthenticatorSubscriber($builder, new Factory());
+        $subscriber = new AuthenticatorSubscriber($builder, $authenticator);
 
         $logger = new Logger('foo');
         $handler = new TestHandler();
@@ -75,16 +82,8 @@ class AuthenticatorSubscriberTest extends TestCase
         $authenticator->expects($this->once())
             ->method('login');
 
-        $factory = $this->getMockBuilder(Factory::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-
-        $factory->expects($this->once())
-            ->method('buildFromSiteConfig')
-            ->willReturn($authenticator);
-
         $builder = new ArraySiteConfigBuilder(['example.com' => ['requiresLogin' => true]]);
-        $subscriber = new AuthenticatorSubscriber($builder, $factory);
+        $subscriber = new AuthenticatorSubscriber($builder, $authenticator);
 
         $logger = new Logger('foo');
         $handler = new TestHandler();
@@ -124,8 +123,12 @@ class AuthenticatorSubscriberTest extends TestCase
 
     public function testLoginIfRequestedNotRequired()
     {
+        $authenticator = $this->getMockBuilder(Authenticator::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
         $builder = new ArraySiteConfigBuilder(['example.com' => []]);
-        $subscriber = new AuthenticatorSubscriber($builder, new Factory());
+        $subscriber = new AuthenticatorSubscriber($builder, $authenticator);
 
         $logger = new Logger('foo');
         $handler = new TestHandler();
@@ -161,19 +164,11 @@ class AuthenticatorSubscriberTest extends TestCase
             ->method('isLoginRequired')
             ->willReturn(false);
 
-        $factory = $this->getMockBuilder(Factory::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-
-        $factory->expects($this->once())
-            ->method('buildFromSiteConfig')
-            ->willReturn($authenticator);
-
         $builder = new ArraySiteConfigBuilder(['example.com' => [
             'requiresLogin' => true,
             'notLoggedInXpath' => '//html',
         ]]);
-        $subscriber = new AuthenticatorSubscriber($builder, $factory);
+        $subscriber = new AuthenticatorSubscriber($builder, $authenticator);
 
         $logger = new Logger('foo');
         $handler = new TestHandler();
@@ -221,19 +216,11 @@ class AuthenticatorSubscriberTest extends TestCase
         $authenticator->expects($this->once())
             ->method('login');
 
-        $factory = $this->getMockBuilder(Factory::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-
-        $factory->expects($this->once())
-            ->method('buildFromSiteConfig')
-            ->willReturn($authenticator);
-
         $builder = new ArraySiteConfigBuilder(['example.com' => [
             'requiresLogin' => true,
             'notLoggedInXpath' => '//html',
         ]]);
-        $subscriber = new AuthenticatorSubscriber($builder, $factory);
+        $subscriber = new AuthenticatorSubscriber($builder, $authenticator);
 
         $logger = new Logger('foo');
         $handler = new TestHandler();
@@ -276,7 +263,7 @@ class AuthenticatorSubscriberTest extends TestCase
 
     public function testLoginIfRequestedRedirect()
     {
-        $factory = $this->getMockBuilder(Factory::class)
+        $authenticator = $this->getMockBuilder(Authenticator::class)
             ->disableOriginalConstructor()
             ->getMock();
 
@@ -284,7 +271,7 @@ class AuthenticatorSubscriberTest extends TestCase
             'requiresLogin' => true,
             'notLoggedInXpath' => '//html',
         ]]);
-        $subscriber = new AuthenticatorSubscriber($builder, $factory);
+        $subscriber = new AuthenticatorSubscriber($builder, $authenticator);
 
         $logger = new Logger('foo');
         $handler = new TestHandler();