Merge pull request #1665 from bburnichon/bug/invalid-checker-configuration-PHRAS-927

Add missing applicability test in BorderManager

lib/Alchemy/Phrasea/Border/Checker/AbstractChecker.php

@@ -19,8 +19,19 @@ use Alchemy\Phrasea\Border\File;
  */
 abstract class AbstractChecker implements CheckerInterface
 {
+    /**
+     * @var Application
+     */
     protected $app;
+
+    /**
+     * @var \databox[]
+     */
     protected $databoxes = [];
+
+    /**
+     * @var \collection[]
+     */
     protected $collections = [];
 
     public function __construct(Application $app)
@@ -32,8 +43,8 @@ abstract class AbstractChecker implements CheckerInterface
      * Restrict the checker to a set of databoxes.
      * Warning, you can not restrict on both databoxes and collections
      *
-     * @param  databox|array $databoxes A databox or an array of databoxes
+     * @param  \databox[] $databoxes A databox or an array of databoxes
-     * @return Boolean
+     * @return bool
      *
      * @throws \LogicException           If already restricted to collections
      * @throws \InvalidArgumentException In case invalid databoxes are provided
@@ -60,8 +71,8 @@ abstract class AbstractChecker implements CheckerInterface
      * Restrict the checker to a set of collections.
      * Warning, you can not restrict on both databoxes and collections
      *
-     * @param  collection|array $collections
+     * @param  \collection[] $collections
-     * @return Boolean
+     * @return bool
      *
      * @throws \LogicException           If already restricted to databoxes
      * @throws \InvalidArgumentException In case invalid collections are provided
@@ -93,6 +104,10 @@ abstract class AbstractChecker implements CheckerInterface
      */
     public function isApplicable(File $file)
     {
+        if (empty($this->databoxes) && empty($this->collections)) {
+            return true;
+        }
+
         if (null === $file->getCollection()) {
             return true;
         }

lib/Alchemy/Phrasea/Border/Manager.php

@@ -42,6 +42,9 @@ use Symfony\Component\Filesystem\Exception\IOException;
  */
 class Manager
 {
+    /**
+     * @var CheckerInterface[]
+     */
     protected $checkers = [];
     protected $app;
     protected $filesystem;
@@ -157,7 +160,9 @@ class Manager
         }
 
         foreach ($this->checkers as $checker) {
-            $visa->addResponse($checker->check($this->app['orm.em'], $file));
+            if ($checker->isApplicable($file)) {
+                $visa->addResponse($checker->check($this->app['orm.em'], $file));
+            }
         }
 
         return $visa;

tests/Alchemy/Tests/Phrasea/Border/Checker/AbstractCheckerTest.php

@@ -2,141 +2,99 @@
 
 namespace Alchemy\Tests\Phrasea\Border\Checker;
 
+use Alchemy\Phrasea\Application;
 use Alchemy\Phrasea\Border\Checker\AbstractChecker;
 use Alchemy\Phrasea\Border\File;
-use Doctrine\ORM\EntityManager;
-use Symfony\Component\Translation\TranslatorInterface;
 
-/**
+class AbstractCheckerTest extends \PHPUnit_Framework_TestCase
- * @group functional
- * @group legacy
- */
-class AbstractCheckerTest extends \PhraseanetTestCase
 {
+    /**
+     * @var Application
+     */
+    private $app;
+
     /**
      * @var AbstractChecker
      */
-    protected $object;
+    private $sut;
 
     public function setUp()
     {
         parent::setUp();
 
-        $this->object = $this->getMockBuilder(AbstractChecker::class)
+        $this->app = $this->prophesize(Application::class);
-            ->setConstructorArgs([self::$DI['app']])
+
-            ->getMockForAbstractClass();
+        $this->sut = $this->getMockBuilder(AbstractChecker::class)
-        $this->file = $this->getMock('\\Alchemy\\Phrasea\\Border\\File', ['getCollection'], [], 'CheckerTesterMock' . mt_rand(), false);
+            ->setConstructorArgs([$this->app->reveal()])
-    }
+            ->getMockForAbstractClass();
+
 
-    public function tearDown()
-    {
-        $this->file = null;
-        parent::tearDown();
     }
 
     /**
-     * @covers Alchemy\Phrasea\Border\Checker\AbstractChecker::restrictToDataboxes
+     * @dataProvider getDataboxesCombination
-     * @covers Alchemy\Phrasea\Border\Checker\AbstractChecker::isApplicable
-     * @dataProvider getDataboxesCombinaison
      */
     public function testRestrictToDataboxes($databoxes, $collection, $assertion)
     {
-        $this->file->expects($this->any())
+        $file = $this->prophesize(File::class);
-            ->method('getCollection')
+        $file->getCollection()->willReturn($collection);
-            ->will($this->returnValue($collection));
 
-        $this->object->restrictToDataboxes($databoxes);
+        $this->sut->restrictToDataboxes($databoxes);
 
-        $this->assertEquals($assertion, $this->object->isApplicable($this->file));
+        $this->assertEquals($assertion, $this->sut->isApplicable($file->reveal()));
     }
 
-    public function getDataboxesCombinaison()
+    public function getDataboxesCombination()
     {
-        $databox = $collection = null;
+        $databox = $this->prophesize(\databox::class);
-        $app = $this->loadApp();
+        $databox->get_sbas_id()->willReturn(1);
 
-        foreach ($app->getDataboxes() as $db) {
+        $collection = $this->prophesize(\collection::class);
-            if (! $collection) {
+        $collection->get_databox()->willReturn($databox->reveal());
-                foreach ($db->get_collections() as $coll) {
+        $collection->get_base_id()->willReturn(2);
-                    $collection = $coll;
-                    break;
-                }
-            }
-            if (! $collection) {
-                $this->fail('Unable to get a collection');
-            }
 
-            if ($db->get_sbas_id() != $collection->get_databox()->get_sbas_id()) {
+        $anotherDatabox = $this->prophesize(\databox::class);
-                $databox = $db;
+        $anotherDatabox->get_sbas_id()->willReturn(3);
-                break;
-            }
-        }
 
-        $ret = [
+        return [
-            [[$collection->get_databox()], $collection, true],
+            [[], $collection, true],
-            [$collection->get_databox(), $collection, true],
+            [[$databox->reveal()], $collection, true],
-            [$collection->get_databox(), null, true],
+            [$databox->reveal(), $collection, true],
+            [$databox->reveal(), null, true],
+            [$anotherDatabox->reveal(), $collection, false],
         ];
-
-        if ($databox) {
-            $ret[] = [$databox, $collection, false];
-        }
-
-        return $ret;
     }
 
     /**
-     * @covers Alchemy\Phrasea\Border\Checker\AbstractChecker::restrictToCollections
+     * @dataProvider getCollectionsCombination
-     * @covers Alchemy\Phrasea\Border\Checker\AbstractChecker::isApplicable
-     * @dataProvider getCollectionsCombinaison
      */
     public function testRestrictToCollections($collection, $othercollection, $assertion)
     {
-        $this->file->expects($this->any())
+        $file = $this->prophesize(File::class);
-            ->method('getCollection')
+        $file->getCollection()->willReturn($othercollection);
-            ->will($this->returnValue($othercollection));
 
-        $this->object->restrictToCollections($collection);
+        $this->sut->restrictToCollections($collection);
 
-        $this->assertEquals($assertion, $this->object->isApplicable($this->file));
+        $this->assertEquals($assertion, $this->sut->isApplicable($file->reveal()));
     }
 
-    public function getCollectionsCombinaison()
+    public function getCollectionsCombination()
     {
-        $othercollection = $collection = null;
+        $databox = $this->prophesize(\databox::class);
-        $app = $this->loadApp();
+        $databox->get_sbas_id()->willReturn(1);
-        $databoxes = $app->getDataboxes();
-        if (count($databoxes) === 0) {
-            $this->fail('Unable to find collections');
-        }
-        $databox = array_pop($databoxes);
 
-        foreach ($databoxes as $db) {
+        $collectionProphecy = $this->prophesize(\collection::class);
-            if (! $collection) {
+        $collectionProphecy->get_databox()->willReturn($databox->reveal());
-                foreach ($db->get_collections() as $coll) {
+        $collectionProphecy->get_base_id()->willReturn(2);
-                    $collection = $coll;
+        $collection = $collectionProphecy->reveal();
-                    break;
-                }
-            }
 
-            if (! $othercollection && $collection) {
+        $otherCollectionProphecy = $this->prophesize(\collection::class);
-                foreach ($db->get_collections() as $coll) {
+        $otherCollectionProphecy->get_databox()->willReturn($databox->reveal());
-                    if ($coll->get_base_id() != $collection->get_base_id()) {
+        $otherCollectionProphecy->get_base_id()->willReturn(3);
-                        $othercollection = $coll;
+        $othercollection = $otherCollectionProphecy->reveal();
-                        break;
-                    }
-                }
-            }
-        }
-
-        if (null === $othercollection) {
-            $othercollection = \collection::create($app, $databox, $app['phraseanet.appbox'], 'other coll');
-        }
-        if (null === $collection) {
-            $collection = \collection::create($app, $databox, $app['phraseanet.appbox'], 'other coll');
-        }
 
         return [
+            [[], $collection, true],
             [[$collection], $collection, true],
             [$collection, $collection, true],
             [$collection, null, true],
@@ -151,8 +109,8 @@ class AbstractCheckerTest extends \PhraseanetTestCase
      */
     public function testMixCollectionFirst($databox, $collection)
     {
-        $this->object->restrictToCollections($collection);
+        $this->sut->restrictToCollections($collection);
-        $this->object->restrictToDataboxes($databox);
+        $this->sut->restrictToDataboxes($databox);
     }
 
     /**
@@ -161,8 +119,8 @@ class AbstractCheckerTest extends \PhraseanetTestCase
      */
     public function testMixDataboxFirst($databox, $collection)
     {
-        $this->object->restrictToDataboxes($databox);
+        $this->sut->restrictToDataboxes($databox);
-        $this->object->restrictToCollections($collection);
+        $this->sut->restrictToCollections($collection);
     }
 
     /**
@@ -171,7 +129,7 @@ class AbstractCheckerTest extends \PhraseanetTestCase
      */
     public function testInvalidDatabox($databox, $collection)
     {
-        $this->object->restrictToDataboxes($collection);
+        $this->sut->restrictToDataboxes($collection);
     }
 
     /**
@@ -180,25 +138,17 @@ class AbstractCheckerTest extends \PhraseanetTestCase
      */
     public function testInvalidCollection($databox, $collection)
     {
-        $this->object->restrictToCollections($databox);
+        $this->sut->restrictToCollections($databox);
     }
 
     public function getDataboxAndCollection()
     {
-        $databox = $collection = null;
+        $databox = $this->prophesize(\databox::class);
-        $app = $this->loadApp();
+        $databox->get_sbas_id()->willReturn(1);
 
-        foreach ($app->getDataboxes() as $db) {
+        $collection = $this->prophesize(\collection::class);
-            if (! $databox) {
+        $collection->get_databox()->willReturn($databox->reveal());
-                $databox = $db;
+        $collection->get_base_id()->willReturn(2);
-            }
-            if (! $collection) {
-                foreach ($db->get_collections() as $coll) {
-                    $collection = $coll;
-                    break;
-                }
-            }
-        }
 
         return [
             [$databox, $collection],

tests/Alchemy/Tests/Phrasea/Border/ManagerGetVisaTest.php

@@ -0,0 +1,79 @@
+<?php
+/**
+ * This file is part of Phraseanet
+ *
+ * (c) 2005-2016 Alchemy
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Alchemy\Tests\Phrasea\Border;
+
+
+use Alchemy\Phrasea\Application;
+use Alchemy\Phrasea\Border\Checker\CheckerInterface;
+use Alchemy\Phrasea\Border\Checker\Response;
+use Alchemy\Phrasea\Border\File;
+use Alchemy\Phrasea\Border\Manager;
+use Alchemy\Phrasea\Border\Visa;
+use Doctrine\ORM\EntityManager;
+use Prophecy\Argument;
+use Prophecy\Prophecy\ObjectProphecy;
+
+class ManagerGetVisaTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @var ObjectProphecy
+     */
+    private $app;
+
+    /**
+     * @var Manager
+     */
+    private $sut;
+
+    protected function setUp()
+    {
+        $this->app = $this->prophesize(Application::class);
+        $this->app->offsetGet('orm.em')->willReturn($this->prophesize(EntityManager::class));
+
+        $this->sut = new Manager($this->app->reveal());
+    }
+
+    public function testGetVisaWithoutAnyCheckers()
+    {
+        $file = $this->prophesize(File::class);
+
+        $visa = $this->sut->getVisa($file->reveal());
+
+        $this->assertInstanceOf(Visa::class, $visa);
+    }
+
+    public function testGetVisaWithOnlyOneCheckerApplicable()
+    {
+        $file = $this->prophesize(File::class);
+
+        $checker1 = $this->prophesize(CheckerInterface::class);
+        $checker1->isApplicable($file->reveal())
+            ->willReturn(false);
+        $checker1->check(Argument::any(), $file->reveal())
+            ->shouldNotBeCalled();
+
+        $checker2 = $this->prophesize(CheckerInterface::class);
+        $checker2->isApplicable($file->reveal())
+            ->willReturn(true);
+        $checker2->check(Argument::any(), $file->reveal())
+            ->willReturn(new Response(true, $checker2->reveal()));
+
+        $this->sut->registerCheckers([$checker1->reveal(), $checker2->reveal()]);
+
+        $visa = $this->sut->getVisa($file->reveal());
+
+        $this->assertInstanceOf(Visa::class, $visa);
+        $responses = $visa->getResponses();
+
+        $this->assertCount(1, $responses);
+        $this->assertSame($checker2->reveal(), $responses[0]->getChecker());
+    }
+}

tests/Alchemy/Tests/Phrasea/Border/ManagerTest.php

@@ -550,47 +550,6 @@ class ManagerTest extends \PhraseanetAuthenticatedWebTestCase
         $this->assertEquals(['Phraseanet:tf-duration'], $toFound);
     }
 
-    /**
-     * @covers Alchemy\Phrasea\Border\Manager::getVisa
-     */
-    public function testGetVisa()
-    {
-        $records = [];
-
-        $postProcessRecord = function ($record) use (&$records) {
-                $records[] = $record;
-            };
-
-        self::$DI['app']['phraseanet.SE'] = $this->createSearchEngineMock();
-        $visa = $this->object->getVisa(File::buildFromPathfile(self::$file1, self::$DI['collection'], self::$DI['app']));
-
-        $this->assertInstanceOf('\\Alchemy\\Phrasea\\Border\\Visa', $visa);
-
-        $this->assertTrue($visa->isValid());
-
-        $this->object->process($this->session, File::buildFromPathfile(self::$file1, self::$DI['collection'], self::$DI['app']), $postProcessRecord);
-
-        $visa = $this->object->getVisa(File::buildFromPathfile(self::$file1, self::$DI['collection'], self::$DI['app']));
-
-        $this->assertInstanceOf('\\Alchemy\\Phrasea\\Border\\Visa', $visa);
-
-        $this->assertTrue($visa->isValid());
-
-        $this->object->registerChecker(new Sha256(self::$DI['app']));
-
-        $visa = $this->object->getVisa(File::buildFromPathfile(self::$file1, self::$DI['collection'], self::$DI['app']));
-
-        $this->assertInstanceOf('\\Alchemy\\Phrasea\\Border\\Visa', $visa);
-
-        $this->assertFalse($visa->isValid());
-
-        foreach ($records as $record) {
-            if ($record instanceof \record_adapter) {
-                $record->delete();
-            }
-        }
-    }
-
     /**
      * @covers Alchemy\Phrasea\Border\Manager::registerChecker
      * @covers Alchemy\Phrasea\Border\Manager::getCheckers