Add tagged services for import

- list services in /import
- add url to import service
- ImportBundle routing are now prefixed by /import
- optimize flush in each import (flushing each 20 contents)
- improve design of each import
- add more tests

app/config/routing.yml

@@ -1,7 +1,7 @@
 wallabag_import:
     resource: "@WallabagImportBundle/Controller/"
     type:     annotation
-    prefix:   /
+    prefix:   /import
 
 wallabag_api:
     resource: "@WallabagApiBundle/Resources/config/routing.yml"

src/Wallabag/CoreBundle/Resources/views/themes/material/public/css/main.css

@@ -497,4 +497,8 @@ footer [class^="icon-"]:hover, footer [class*=" icon-"]:hover {
 /* force height on non-input field in the settings page */
 div.settings div.input-field div, div.settings div.input-field ul {
     margin-top: 40px;
-}
+}
+/* but avoid to kill all file input */
+div.settings div.file-field div {
+    margin-top: inherit;
+}

src/Wallabag/ImportBundle/Controller/ImportController.php

@@ -8,10 +8,12 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
 class ImportController extends Controller
 {
     /**
-     * @Route("/import", name="import")
+     * @Route("/", name="import")
      */
     public function importAction()
     {
-        return $this->render('WallabagImportBundle:Import:index.html.twig', []);
+        return $this->render('WallabagImportBundle:Import:index.html.twig', [
+            'imports' => $this->get('wallabag_import.chain')->getAll(),
+        ]);
     }
 }

src/Wallabag/ImportBundle/Controller/PocketController.php

@@ -8,15 +8,17 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
 class PocketController extends Controller
 {
     /**
-     * @Route("/import/pocket", name="import_pocket")
+     * @Route("/pocket", name="import_pocket")
      */
     public function indexAction()
     {
-        return $this->render('WallabagImportBundle:Pocket:index.html.twig', []);
+        return $this->render('WallabagImportBundle:Pocket:index.html.twig', [
+            'import' => $this->get('wallabag_import.pocket.import'),
+        ]);
     }
 
     /**
-     * @Route("/import/pocket/auth", name="import_pocket_auth")
+     * @Route("/pocket/auth", name="import_pocket_auth")
      */
     public function authAction()
     {
@@ -32,7 +34,7 @@ class PocketController extends Controller
     }
 
     /**
-     * @Route("/import/pocket/callback", name="import_pocket_callback")
+     * @Route("/pocket/callback", name="import_pocket_callback")
      */
     public function callbackAction()
     {

src/Wallabag/ImportBundle/Controller/WallabagV1Controller.php

@@ -10,20 +10,20 @@ use Wallabag\ImportBundle\Form\Type\UploadImportType;
 class WallabagV1Controller extends Controller
 {
     /**
-     * @Route("/import/wallabag-v1", name="import_wallabag_v1")
+     * @Route("/wallabag-v1", name="import_wallabag_v1")
      */
     public function indexAction(Request $request)
     {
-        $importForm = $this->createForm(new UploadImportType());
-        $importForm->handleRequest($request);
-        $user = $this->getUser();
+        $form = $this->createForm(new UploadImportType());
+        $form->handleRequest($request);
 
-        if ($importForm->isValid()) {
-            $file = $importForm->get('file')->getData();
-            $name = $user->getId().'.json';
+        $wallabag = $this->get('wallabag_import.wallabag_v1.import');
+
+        if ($form->isValid()) {
+            $file = $form->get('file')->getData();
+            $name = $this->getUser()->getId().'.json';
 
             if (in_array($file->getClientMimeType(), $this->getParameter('wallabag_import.allow_mimetypes')) && $file->move($this->getParameter('wallabag_import.resource_dir'), $name)) {
-                $wallabag = $this->get('wallabag_import.wallabag_v1.import');
                 $res = $wallabag
                     ->setUser($this->getUser())
                     ->setFilepath($this->getParameter('wallabag_import.resource_dir').'/'.$name)
@@ -34,7 +34,7 @@ class WallabagV1Controller extends Controller
                     $summary = $wallabag->getSummary();
                     $message = 'Import summary: '.$summary['imported'].' imported, '.$summary['skipped'].' already saved.';
 
-                    @unlink($this->getParameter('wallabag_import.resource_dir').'/'.$name);
+                    unlink($this->getParameter('wallabag_import.resource_dir').'/'.$name);
                 }
 
                 $this->get('session')->getFlashBag()->add(
@@ -52,7 +52,8 @@ class WallabagV1Controller extends Controller
         }
 
         return $this->render('WallabagImportBundle:WallabagV1:index.html.twig', [
-            'form' => $importForm->createView(),
+            'form' => $form->createView(),
+            'import' => $wallabag,
         ]);
     }
 }

src/Wallabag/ImportBundle/Form/Type/UploadImportType.php

@@ -15,13 +15,6 @@ class UploadImportType extends AbstractType
         ;
     }
 
-    public function getDefaultOptions(array $options)
-    {
-        return array(
-            'csrf_protection' => false,
-        );
-    }
-
     public function getName()
     {
         return 'upload_import_file';

src/Wallabag/ImportBundle/Import/ImportChain.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace Wallabag\ImportBundle\Import;
+
+class ImportChain
+{
+    private $imports;
+
+    public function __construct()
+    {
+        $this->imports = [];
+    }
+
+    /**
+     * Add an import to the chain.
+     *
+     * @param ImportInterface $import
+     * @param string          $alias
+     */
+    public function addImport(ImportInterface $import, $alias)
+    {
+        $this->imports[$alias] = $import;
+    }
+
+    /**
+     * Get all imports.
+     *
+     * @return array<ImportInterface>
+     */
+    public function getAll()
+    {
+        return $this->imports;
+    }
+}

src/Wallabag/ImportBundle/Import/ImportCompilerPass.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace Wallabag\ImportBundle\Import;
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\Reference;
+
+class ImportCompilerPass implements CompilerPassInterface
+{
+    public function process(ContainerBuilder $container)
+    {
+        if (!$container->hasDefinition('wallabag_import.chain')) {
+            return;
+        }
+
+        $definition = $container->getDefinition(
+            'wallabag_import.chain'
+        );
+
+        $taggedServices = $container->findTaggedServiceIds(
+            'wallabag_import.import'
+        );
+        foreach ($taggedServices as $id => $tagAttributes) {
+            foreach ($tagAttributes as $attributes) {
+                $definition->addMethodCall(
+                    'addImport',
+                    [new Reference($id), $attributes['alias']]
+                );
+            }
+        }
+    }
+}

src/Wallabag/ImportBundle/Import/ImportInterface.php

@@ -13,6 +13,13 @@ interface ImportInterface extends LoggerAwareInterface
      */
     public function getName();
 
+    /**
+     * Url to start the import.
+     *
+     * @return string
+     */
+    public function getUrl();
+
     /**
      * Description of the import.
      *

src/Wallabag/ImportBundle/Import/PocketImport.php

@@ -45,12 +45,20 @@ class PocketImport implements ImportInterface
         return 'Pocket';
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function getUrl()
+    {
+        return 'import_pocket';
+    }
+
     /**
      * {@inheritdoc}
      */
     public function getDescription()
     {
-        return 'This importer will import all your <a href="https://getpocket.com">Pocket</a> data.';
+        return 'This importer will import all your <a href="https://getpocket.com">Pocket</a> data. Pocket doesn\'t allow us to retrieve content from their service, so the readable content of each article will be re-fetched by Wallabag.';
     }
 
     /**
@@ -196,6 +204,8 @@ class PocketImport implements ImportInterface
      */
     private function parseEntries($entries)
     {
+        $i = 1;
+
         foreach ($entries as $pocketEntry) {
             $url = isset($pocketEntry['resolved_url']) && $pocketEntry['resolved_url'] != '' ? $pocketEntry['resolved_url'] : $pocketEntry['given_url'];
 
@@ -241,6 +251,12 @@ class PocketImport implements ImportInterface
 
             $this->em->persist($entry);
             ++$this->importedEntries;
+
+            // flush every 20 entries
+            if (($i % 20) === 0) {
+                $em->flush();
+            }
+            ++$i;
         }
 
         $this->em->flush();

src/Wallabag/ImportBundle/Import/WallabagV1Import.php

@@ -50,12 +50,20 @@ class WallabagV1Import implements ImportInterface
         return 'Wallabag v1';
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function getUrl()
+    {
+        return 'import_wallabag_v1';
+    }
+
     /**
      * {@inheritdoc}
      */
     public function getDescription()
     {
-        return 'This importer will import all your wallabag v1 articles.';
+        return 'This importer will import all your wallabag v1 articles. On your config page, click on "JSON export" in the "Export your wallabag data" section. You will have a "wallabag-export-1-xxxx-xx-xx.json" file.';
     }
 
     /**
@@ -75,7 +83,13 @@ class WallabagV1Import implements ImportInterface
             return false;
         }
 
-        $this->parseEntries(json_decode(file_get_contents($this->filepath), true));
+        $data = json_decode(file_get_contents($this->filepath), true);
+
+        if (empty($data)) {
+            return false;
+        }
+
+        $this->parseEntries($data);
 
         return true;
     }
@@ -108,6 +122,8 @@ class WallabagV1Import implements ImportInterface
      */
     private function parseEntries($entries)
     {
+        $i = 1;
+
         foreach ($entries as $importedEntry) {
             $existingEntry = $this->em
                 ->getRepository('WallabagCoreBundle:Entry')
@@ -130,6 +146,12 @@ class WallabagV1Import implements ImportInterface
 
             $this->em->persist($entry);
             ++$this->importedEntries;
+
+            // flush every 20 entries
+            if (($i % 20) === 0) {
+                $em->flush();
+            }
+            ++$i;
         }
 
         $this->em->flush();

src/Wallabag/ImportBundle/Resources/config/services.yml

@@ -1,4 +1,7 @@
 services:
+    wallabag_import.chain:
+        class: Wallabag\ImportBundle\Import\ImportChain
+
     wallabag_import.pocket.client:
         class: GuzzleHttp\Client
         arguments:
@@ -18,6 +21,8 @@ services:
         calls:
             - [ setClient, [ "@wallabag_import.pocket.client" ] ]
             - [ setLogger, [ "@logger" ]]
+        tags:
+            -  { name: wallabag_import.import, alias: pocket }
 
     wallabag_import.wallabag_v1.import:
         class: Wallabag\ImportBundle\Import\WallabagV1Import
@@ -25,3 +30,5 @@ services:
             - "@doctrine.orm.entity_manager"
         calls:
             - [ setLogger, [ "@logger" ]]
+        tags:
+            -  { name: wallabag_import.import, alias: wallabag_v1 }

src/Wallabag/ImportBundle/Resources/views/Import/index.html.twig

@@ -1,15 +1,19 @@
 {% extends "WallabagCoreBundle::layout.html.twig" %}
-{% block title %}{% trans %}import{% endtrans %}{% endblock %}
+{% block title %}{% trans %}Import{% endtrans %}{% endblock %}
 
 {% block content %}
-
 <div class="row">
     <div class="col s12">
         <div class="card-panel settings">
             {% trans %}Welcome on wallabag importer. Please select your previous service that you want to migrate.{% endtrans %}
             <ul>
-                <li><a href="{{ path('import_pocket') }}">Pocket</a></li>
-                <li><a href="{{ path('import_wallabag_v1') }}">Wallabag v1</a></li>
+                {% for import in imports %}
+                    <li>
+                        <h5>{{ import.name }}</h5>
+                        <blockquote>{{ import.description|raw }}</blockquote>
+                        <p><a class="waves-effect waves-light btn" href="{{ path(import.url) }}">Import contents</a></p>
+                    </li>
+                {% endfor %}
             </ul>
         </div>
     </div>

src/Wallabag/ImportBundle/Resources/views/Pocket/index.html.twig

@@ -1,14 +1,16 @@
 {% extends "WallabagCoreBundle::layout.html.twig" %}
-{% block title %}{% trans %}import{% endtrans %}{% endblock %}
+{% block title %}{% trans %}Import > Pocket{% endtrans %}{% endblock %}
 
 {% block content %}
-
 <div class="row">
     <div class="col s12">
         <div class="card-panel settings">
-            {% trans %}You can import your data from your Pocket account. You just have to click on the below button and authorize the application to connect to getpocket.com.{% endtrans %}
+            <blockquote>{{ import.description|raw }}</blockquote>
+            <p>{% trans %}You can import your data from your Pocket account. You just have to click on the below button and authorize the application to connect to getpocket.com.{% endtrans %}</p>
             <form method="post" action="{{ path('import_pocket_auth') }}">
-                <input type="submit" value="Connect to Pocket and import data" />
+                <button class="btn waves-effect waves-light" type="submit" name="action">
+                    Connect to Pocket and import data
+                </button>
             </form>
         </div>
     </div>

src/Wallabag/ImportBundle/Resources/views/WallabagV1/index.html.twig

@@ -1,20 +1,26 @@
 {% extends "WallabagCoreBundle::layout.html.twig" %}
-{% block title %}{% trans %}import{% endtrans %}{% endblock %}
+{% block title %}{% trans %}Import > Wallabag v1{% endtrans %}{% endblock %}
 
 {% block content %}
-
 <div class="row">
     <div class="col s12">
         <div class="card-panel settings">
             <div class="row">
+                <blockquote>{{ import.description|raw }}</blockquote>
+                <p>{% trans %}Please select your wallabag export and click on the below button to upload and import it.{% endtrans %}</p>
                 <div class="col s12">
                     {{ form_start(form, {'method': 'POST'}) }}
                         {{ form_errors(form) }}
                         <div class="row">
-                            <div class="input-field col s12">
-                                <p>{% trans %}Please select your wallabag export and click on the below button to upload and import it.{% endtrans %}</p>
+                            <div class="file-field input-field col s12">
                                 {{ form_errors(form.file) }}
-                                {{ form_widget(form.file) }}
+                                <div class="btn">
+                                    <span>File</span>
+                                    {{ form_widget(form.file) }}
+                                </div>
+                                <div class="file-path-wrapper">
+                                    <input class="file-path validate" type="text">
+                                </div>
                             </div>
                         </div>
                         <div class="hidden">{{ form_rest(form) }}</div>

src/Wallabag/ImportBundle/Tests/Controller/ImportControllerTest.php

@@ -0,0 +1,29 @@
+<?php
+
+namespace Wallabag\ImportBundle\Tests\Controller;
+
+use Wallabag\CoreBundle\Tests\WallabagCoreTestCase;
+
+class ImportControllerTest extends WallabagCoreTestCase
+{
+    public function testLogin()
+    {
+        $client = $this->getClient();
+
+        $client->request('GET', '/import/');
+
+        $this->assertEquals(302, $client->getResponse()->getStatusCode());
+        $this->assertContains('login', $client->getResponse()->headers->get('location'));
+    }
+
+    public function testImportList()
+    {
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        $crawler = $client->request('GET', '/import/');
+
+        $this->assertEquals(200, $client->getResponse()->getStatusCode());
+        $this->assertEquals(2, $crawler->filter('blockquote')->count());
+    }
+}

src/Wallabag/ImportBundle/Tests/Controller/PocketControllerTest.php

@@ -0,0 +1,42 @@
+<?php
+
+namespace Wallabag\ImportBundle\Tests\Controller;
+
+use Wallabag\CoreBundle\Tests\WallabagCoreTestCase;
+
+class PocketControllerTest extends WallabagCoreTestCase
+{
+    public function testImportPocket()
+    {
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        $crawler = $client->request('GET', '/import/pocket');
+
+        $this->assertEquals(200, $client->getResponse()->getStatusCode());
+        $this->assertEquals(1, $crawler->filter('button[type=submit]')->count());
+    }
+
+    public function testImportPocketAuth()
+    {
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        $crawler = $client->request('GET', '/import/pocket/auth');
+
+        $this->assertEquals(301, $client->getResponse()->getStatusCode());
+        $this->assertContains('getpocket.com/auth/authorize', $client->getResponse()->headers->get('location'));
+    }
+
+    public function testImportPocketCallbackWithBadToken()
+    {
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        $crawler = $client->request('GET', '/import/pocket/callback');
+
+        $this->assertEquals(302, $client->getResponse()->getStatusCode());
+        $this->assertContains('import/pocket', $client->getResponse()->headers->get('location'));
+        $this->assertEquals('Import failed, please try again.', $client->getContainer()->get('session')->getFlashBag()->peek('notice')[0]);
+    }
+}

src/Wallabag/ImportBundle/Tests/Controller/WallabagV1ControllerTest.php

@@ -0,0 +1,69 @@
+<?php
+
+namespace Wallabag\ImportBundle\Tests\Controller;
+
+use Wallabag\CoreBundle\Tests\WallabagCoreTestCase;
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+
+class WallabagV1ControllerTest extends WallabagCoreTestCase
+{
+    public function testImportWallabag()
+    {
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        $crawler = $client->request('GET', '/import/wallabag-v1');
+
+        $this->assertEquals(200, $client->getResponse()->getStatusCode());
+        $this->assertEquals(1, $crawler->filter('form[name=upload_import_file] > button[type=submit]')->count());
+        $this->assertEquals(1, $crawler->filter('input[type=file]')->count());
+    }
+
+    public function testImportWallabagWithFile()
+    {
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        $crawler = $client->request('GET', '/import/wallabag-v1');
+        $form = $crawler->filter('form[name=upload_import_file] > button[type=submit]')->form();
+
+        $file = new UploadedFile(__DIR__.'/../fixtures/wallabag-v1.json', 'wallabag-v1.json');
+
+        $data = array(
+            'upload_import_file[file]' => $file,
+        );
+
+        $client->submit($form, $data);
+
+        $this->assertEquals(302, $client->getResponse()->getStatusCode());
+
+        $crawler = $client->followRedirect();
+
+        $this->assertGreaterThan(1, $alert = $crawler->filter('div.messages.success')->extract(array('_text')));
+        $this->assertContains('Import summary', $alert[0]);
+    }
+
+    public function testImportWallabagWithEmptyFile()
+    {
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        $crawler = $client->request('GET', '/import/wallabag-v1');
+        $form = $crawler->filter('form[name=upload_import_file] > button[type=submit]')->form();
+
+        $file = new UploadedFile(__DIR__.'/../fixtures/test.txt', 'test.txt');
+
+        $data = array(
+            'upload_import_file[file]' => $file,
+        );
+
+        $client->submit($form, $data);
+
+        $this->assertEquals(302, $client->getResponse()->getStatusCode());
+
+        $crawler = $client->followRedirect();
+
+        $this->assertGreaterThan(1, $alert = $crawler->filter('div.messages.success')->extract(array('_text')));
+        $this->assertContains('Import failed, please try again', $alert[0]);
+    }
+}

src/Wallabag/ImportBundle/Tests/Import/ImportChainTest.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace Wallabag\ImportBundle\Tests\Import;
+
+use Wallabag\ImportBundle\Import\ImportChain;
+
+class ImportChainTest extends \PHPUnit_Framework_TestCase
+{
+    public function testGetAll()
+    {
+        $import = $this->getMockBuilder('Wallabag\ImportBundle\Import\ImportInterface')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $importChain = new ImportChain();
+        $importChain->addImport($import, 'alias');
+
+        $this->assertCount(1, $importChain->getAll());
+        $this->assertEquals($import, $importChain->getAll()['alias']);
+    }
+}

src/Wallabag/ImportBundle/Tests/Import/ImportCompilerPassTest.php

@@ -0,0 +1,47 @@
+<?php
+
+namespace Wallabag\ImportBundle\Tests\Import;
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Wallabag\ImportBundle\Import\ImportCompilerPass;
+
+class ImportCompilerPassTest extends \PHPUnit_Framework_TestCase
+{
+    public function testProcessNoDefinition()
+    {
+        $container = new ContainerBuilder();
+        $res = $this->process($container);
+
+        $this->assertNull($res);
+    }
+
+    public function testProcess()
+    {
+        $container = new ContainerBuilder();
+        $container
+            ->register('wallabag_import.chain')
+            ->setPublic(false)
+        ;
+
+        $container
+            ->register('foo')
+            ->addTag('wallabag_import.import', array('alias' => 'pocket'))
+        ;
+
+        $this->process($container);
+
+        $this->assertTrue($container->hasDefinition('wallabag_import.chain'));
+
+        $definition = $container->getDefinition('wallabag_import.chain');
+        $this->assertTrue($definition->hasMethodCall('addImport'));
+
+        $calls = $definition->getMethodCalls();
+        $this->assertEquals('pocket', $calls[0][1][1]);
+    }
+
+    protected function process(ContainerBuilder $container)
+    {
+        $repeatedPass = new ImportCompilerPass();
+        $repeatedPass->process($container);
+    }
+}

src/Wallabag/ImportBundle/Tests/Import/PocketImportTest.php

@@ -74,7 +74,8 @@ class PocketImportTest extends \PHPUnit_Framework_TestCase
         $pocketImport = $this->getPocketImport();
 
         $this->assertEquals('Pocket', $pocketImport->getName());
-        $this->assertEquals('This importer will import all your <a href="https://getpocket.com">Pocket</a> data.', $pocketImport->getDescription());
+        $this->assertNotEmpty($pocketImport->getUrl());
+        $this->assertContains('This importer will import all your <a href="https://getpocket.com">Pocket</a> data.', $pocketImport->getDescription());
     }
 
     public function testOAuthRequest()

src/Wallabag/ImportBundle/Tests/Import/WallabagV1ImportTest.php

@@ -39,7 +39,8 @@ class WallabagV1ImportTest extends \PHPUnit_Framework_TestCase
         $wallabagV1Import = $this->getWallabagV1Import();
 
         $this->assertEquals('Wallabag v1', $wallabagV1Import->getName());
-        $this->assertEquals('This importer will import all your wallabag v1 articles.', $wallabagV1Import->getDescription());
+        $this->assertNotEmpty($wallabagV1Import->getUrl());
+        $this->assertContains('This importer will import all your wallabag v1 articles.', $wallabagV1Import->getDescription());
     }
 
     public function testImport()

src/Wallabag/ImportBundle/WallabagImportBundle.php

@@ -3,7 +3,15 @@
 namespace Wallabag\ImportBundle;
 
 use Symfony\Component\HttpKernel\Bundle\Bundle;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Wallabag\ImportBundle\Import\ImportCompilerPass;
 
 class WallabagImportBundle extends Bundle
 {
+    public function build(ContainerBuilder $container)
+    {
+        parent::build($container);
+
+        $container->addCompilerPass(new ImportCompilerPass());
+    }
 }