Fix thesaurus regression from f25bdf4171

- Field class is now immutable, mergeWith() returns a new instance
- Thesaurus roots are recursively merged
- In case of field merge, Structure class do not index the previous field
- Added failing test case for previous bugs
- Added merge tests on Field class
- Added tests for all indexed stuff inside Structure class

composer.json

@@ -83,7 +83,8 @@
         "twig/twig":                             "~1.14, >=1.14.2",
         "vierbergenlars/php-semver":             "~2.1",
         "willdurand/negotiation":                "~1.3",
-        "zend/gdata":                            "~1.12.1"
+        "zend/gdata":                            "~1.12.1",
+        "beberlei/assert": "2.3"
     },
     "require-dev":  {
         "phpunit/phpunit": "~4.5",

composer.lock

@@ -1,10 +1,10 @@
 {
     "_readme": [
         "This file locks the dependencies of your project to a known state",
-        "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file",
+        "Read more about it at http://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file",
         "This file is @generated automatically"
     ],
-    "hash": "0cc03f42a7df15fa96138931fa1a454d",
+    "hash": "8be397f1ceeee3b9dba740812467849d",
     "packages": [
         {
             "name": "alchemy-fr/tcpdf-clone",
@@ -418,6 +418,55 @@
             ],
             "time": "2014-12-10 15:03:17"
         },
+        {
+            "name": "beberlei/assert",
+            "version": "v2.3",
+            "source": {
+                "type": "git",
+                "url": "https://github.com/beberlei/assert.git",
+                "reference": "160eba4d1fbe692e42b3cf8a20b92ab23e3a8759"
+            },
+            "dist": {
+                "type": "zip",
+                "url": "https://api.github.com/repos/beberlei/assert/zipball/160eba4d1fbe692e42b3cf8a20b92ab23e3a8759",
+                "reference": "160eba4d1fbe692e42b3cf8a20b92ab23e3a8759",
+                "shasum": ""
+            },
+            "require": {
+                "ext-mbstring": "*"
+            },
+            "type": "library",
+            "extra": {
+                "branch-alias": {
+                    "dev-master": "2.3-dev"
+                }
+            },
+            "autoload": {
+                "psr-0": {
+                    "Assert": "lib/"
+                },
+                "files": [
+                    "lib/Assert/functions.php"
+                ]
+            },
+            "notification-url": "https://packagist.org/downloads/",
+            "license": [
+                "BSD-2-Clause"
+            ],
+            "authors": [
+                {
+                    "name": "Benjamin Eberlei",
+                    "email": "kontakt@beberlei.de"
+                }
+            ],
+            "description": "Thin assertion library for input validation in business models.",
+            "keywords": [
+                "assert",
+                "assertion",
+                "validation"
+            ],
+            "time": "2014-12-18 19:12:40"
+        },
         {
             "name": "behat/transliterator",
             "version": "v1.0.1",
@@ -1070,7 +1119,7 @@
             "dist": {
                 "type": "zip",
                 "url": "https://api.github.com/repos/doctrine/migrations/zipball/67f02686c6c779ae50489728b91026bc8199720c",
-                "reference": "67f02686c6c779ae50489728b91026bc8199720c",
+                "reference": "a4f14d3a3d397104e557ec65d1a4e43bb86e4ddf",
                 "shasum": ""
             },
             "require": {
@@ -3965,7 +4014,7 @@
             "dist": {
                 "type": "zip",
                 "url": "https://api.github.com/repos/Roave/SecurityAdvisories/zipball/99371b09caff9ba2afd5453f0ba2b5b5ddd95b57",
-                "reference": "99371b09caff9ba2afd5453f0ba2b5b5ddd95b57",
+                "reference": "8428cb8041648066a7a2f37adeab2f89ba938358",
                 "shasum": ""
             },
             "conflict": {

lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Field.php

@@ -4,7 +4,9 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\Structure;
 
 use Alchemy\Phrasea\SearchEngine\Elastic\Exception\MergeException;
 use Alchemy\Phrasea\SearchEngine\Elastic\Mapping;
+use Alchemy\Phrasea\SearchEngine\Elastic\Thesaurus\Concept;
 use Alchemy\Phrasea\SearchEngine\Elastic\Thesaurus\Helper as ThesaurusHelper;
+use Assert\Assertion;
 use databox_field;
 
 /**
@@ -32,14 +34,12 @@ class Field
             $roots = null;
         }
 
-        return new self(
-            $field->get_name(),
-            $type,
-            $field->is_indexable(),
-            $field->isBusiness(),
-            $field->isAggregable(),
-            $roots
-        );
+        return new self($field->get_name(), $type, [
+            'searchable' => $field->is_indexable(),
+            'private' => $field->isBusiness(),
+            'facet' => $field->isAggregable(),
+            'thesaurus_roots' => $roots
+        ]);
     }
 
     private static function getTypeFromLegacy(databox_field $field)
@@ -58,14 +58,21 @@ class Field
         }
     }
 
-    public function __construct($name, $type, $searchable = true, $private = false, $facet = false, array $thesaurus_roots = null)
+    public function __construct($name, $type, array $options = [])
     {
         $this->name = (string) $name;
-        $this->type = (string) $type;
-        $this->is_searchable = (bool) $searchable;
-        $this->is_private = (bool) $private;
-        $this->is_facet = (bool) $facet;
-        $this->thesaurus_roots = $thesaurus_roots;
+        $this->type = $type;
+        $this->is_searchable   = \igorw\get_in($options, ['searchable'], true);
+        $this->is_private      = \igorw\get_in($options, ['private'], false);
+        $this->is_facet        = \igorw\get_in($options, ['facet'], false);
+        $this->thesaurus_roots = \igorw\get_in($options, ['thesaurus_roots'], null);
+
+        Assertion::boolean($this->is_searchable);
+        Assertion::boolean($this->is_private);
+        Assertion::boolean($this->is_facet);
+        if ($this->thesaurus_roots !== null) {
+            Assertion::allIsInstanceOf($this->thesaurus_roots, Concept::class);
+        }
     }
 
     public function getName()
@@ -112,6 +119,13 @@ class Field
         return $this->thesaurus_roots;
     }
 
+    /**
+     * Merge with another field, returning the new instance
+     *
+     * @param Field $other
+     * @return Field
+     * @throws MergeException
+     */
     public function mergeWith(Field $other)
     {
         if (($name = $other->getName()) !== $this->name) {
@@ -131,11 +145,26 @@ class Field
         }
 
         if ($other->isSearchable() !== $this->is_searchable) {
-            throw new MergeException(sprintf("Field %s can't be merged, incompatible searchable state", $name));
+            throw new MergeException(sprintf("Field %s can't be merged, incompatible searchablility", $name));
         }
 
         if ($other->isFacet() !== $this->is_facet) {
-            throw new MergeException(sprintf("Field %s can't be merged, incompatible to_aggregate state", $name));
+            throw new MergeException(sprintf("Field %s can't be merged, incompatible facet eligibility", $name));
         }
+
+        $thesaurus_roots = null;
+        if ($this->thesaurus_roots !== null || $other->thesaurus_roots !== null) {
+            $thesaurus_roots = array_merge(
+                (array) $this->thesaurus_roots,
+                (array) $other->thesaurus_roots
+            );
+        }
+
+        return new self($this->name, $this->type, [
+            'searchable' => $this->is_searchable,
+            'private' => $this->is_private,
+            'facet' => $this->is_facet,
+            'thesaurus_roots' => $thesaurus_roots
+        ]);
     }
 }

lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Structure.php

@@ -12,7 +12,6 @@ class Structure
     private $thesaurus_fields = array();
     private $private = array();
     private $facets = array();
-    private $aliases = array();
 
     /**
      * @param \databox[] $databoxes
@@ -34,10 +33,9 @@ class Structure
     {
         $name = $field->getName();
         if (isset($this->fields[$name])) {
-            $this->fields[$name]->mergeWith($field);
-        } else {
-            $this->fields[$name] = $field;
+            $field = $this->fields[$name]->mergeWith($field);
         }
+        $this->fields[$name] = $field;
 
         if ($field->getType() === Mapping::TYPE_DATE) {
             $this->date_fields[$name] = $field;

tests/Alchemy/Tests/Phrasea/SearchEngine/FieldTest.php

@@ -0,0 +1,108 @@
+<?php
+
+namespace Alchemy\Tests\Phrasea\SearchEngine;
+
+use Alchemy\Phrasea\SearchEngine\Elastic\Mapping;
+use Alchemy\Phrasea\SearchEngine\Elastic\Thesaurus\Concept;
+use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field;
+
+/**
+ * @group unit
+ * @group structure
+ */
+class FieldTest extends \PHPUnit_Framework_TestCase
+{
+    public function testBasicMerge()
+    {
+        $field = new Field('foo', Mapping::TYPE_STRING);
+        $other = new Field('foo', Mapping::TYPE_STRING);
+        $merged = $field->mergeWith($other);
+        $this->assertInstanceOf(Field::class, $merged);
+        $this->assertNotSame($field, $merged);
+        $this->assertNotSame($other, $merged);
+        $this->assertEquals('foo', $merged->getName());
+        $this->assertEquals(Mapping::TYPE_STRING, $merged->getType());
+        $this->assertTrue($merged->isSearchable());
+        $this->assertFalse($merged->isPrivate());
+        $this->assertFalse($merged->isFacet());
+        $this->assertNull($merged->getThesaurusRoots());
+    }
+
+    /**
+     * @expectedException Alchemy\Phrasea\SearchEngine\Elastic\Exception\MergeException
+     * @expectedExceptionMessageRegExp #name#u
+     */
+    public function testConflictingNameMerge()
+    {
+        $field = new Field('foo', Mapping::TYPE_STRING);
+        $other = new Field('bar', Mapping::TYPE_STRING);
+        $field->mergeWith($other);
+    }
+
+    /**
+     * @expectedException Alchemy\Phrasea\SearchEngine\Elastic\Exception\MergeException
+     * @expectedExceptionMessageRegExp #type#u
+     */
+    public function testConflictingTypeMerge()
+    {
+        $field = new Field('foo', Mapping::TYPE_STRING);
+        $other = new Field('foo', Mapping::TYPE_DATE);
+        $field->mergeWith($other);
+    }
+
+    /**
+     * @expectedException Alchemy\Phrasea\SearchEngine\Elastic\Exception\MergeException
+     * @expectedExceptionMessageRegExp #search#u
+     */
+    public function testMixedSearchabilityMerge()
+    {
+        $field = new Field('foo', Mapping::TYPE_STRING, ['searchable' => true]);
+        $other = new Field('foo', Mapping::TYPE_STRING, ['searchable' => false]);
+        $field->mergeWith($other);
+    }
+
+    /**
+     * @expectedException Alchemy\Phrasea\SearchEngine\Elastic\Exception\MergeException
+     * @expectedExceptionMessageRegExp #private#u
+     */
+    public function testMixedPrivateAndPublicMerge()
+    {
+        $field = new Field('foo', Mapping::TYPE_STRING, ['private' => true]);
+        $other = new Field('foo', Mapping::TYPE_STRING, ['private' => false]);
+        $field->mergeWith($other);
+    }
+
+    /**
+     * @expectedException Alchemy\Phrasea\SearchEngine\Elastic\Exception\MergeException
+     * @expectedExceptionMessageRegExp #facet#u
+     */
+    public function testMixedFacetEligibilityMerge()
+    {
+        $field = new Field('foo', Mapping::TYPE_STRING, ['facet' => true]);
+        $other = new Field('foo', Mapping::TYPE_STRING, ['facet' => false]);
+        $field->mergeWith($other);
+    }
+
+    public function testMergeWithThesaurusRoots()
+    {
+        $foo = new Concept('/foo');
+        $bar = new Concept('/bar');
+        $field = new Field('foo', Mapping::TYPE_STRING);
+        $other = new Field('foo', Mapping::TYPE_STRING, [
+            'thesaurus_roots' => [$foo, $bar]
+        ]);
+        $merged = $field->mergeWith($other);
+        $this->assertEquals([$foo, $bar], $merged->getThesaurusRoots());
+
+        $foo = new Concept('/foo');
+        $bar = new Concept('/bar');
+        $field = new Field('foo', Mapping::TYPE_STRING, [
+            'thesaurus_roots' => [$foo]
+        ]);
+        $other = new Field('foo', Mapping::TYPE_STRING, [
+            'thesaurus_roots' => [$bar]
+        ]);
+        $merged = $field->mergeWith($other);
+        $this->assertEquals([$foo, $bar], $merged->getThesaurusRoots());
+    }
+}

tests/Alchemy/Tests/Phrasea/SearchEngine/StructureTest.php

@@ -3,27 +3,30 @@
 namespace Alchemy\Tests\Phrasea\SearchEngine;
 
 use Alchemy\Phrasea\SearchEngine\Elastic\Mapping;
+use Alchemy\Phrasea\SearchEngine\Elastic\Thesaurus\Concept;
 use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field;
 use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Structure;
 
+/**
+ * @group unit
+ * @group structure
+ */
 class StructureTest extends \PHPUnit_Framework_TestCase
 {
-    public function testFieldMerge()
+    public function testEmptiness()
     {
-        $field = new Field('foo', Mapping::TYPE_STRING);
-        $other = new Field('foo', Mapping::TYPE_STRING);
-        $field->mergeWith($other);
-        $this->assertEquals('foo', $field->getName());
-        $this->assertEquals(Mapping::TYPE_STRING, $field->getType());
-        $this->assertTrue($field->isSearchable());
-        $this->assertFalse($field->isPrivate());
-        $this->assertFalse($field->isFacet());
+        $structure = new Structure();
+        $this->assertEmpty($structure->getAllFields());
+        $this->assertEmpty($structure->getUnrestrictedFields());
+        $this->assertEmpty($structure->getPrivateFields());
+        $this->assertEmpty($structure->getFacetFields());
+        $this->assertEmpty($structure->getThesaurusEnabledFields());
+        $this->assertEmpty($structure->getDateFields());
     }
 
     public function testFieldAdd()
     {
         $structure = new Structure();
-        $this->assertEmpty($structure->getAllFields());
 
         $field = $this->prophesize(Field::class);
         $field->getName()->willReturn('foo');
@@ -49,12 +52,32 @@ class StructureTest extends \PHPUnit_Framework_TestCase
         $this->assertCount(1, $structure->getAllFields());
     }
 
+    public function testFieldMerge()
+    {
+        $field = $this->prophesize(Field::class);
+        $field->getName()->willReturn('foo');
+        $field->getType()->willReturn(Mapping::TYPE_STRING);
+        $field->isPrivate()->willReturn(false);
+        $field->isFacet()->willReturn(false);
+        $field->hasConceptInference()->willReturn(false);
+
+        $other = new Field('foo', Mapping::TYPE_STRING);
+
+        $merged = new Field('foo', Mapping::TYPE_STRING);
+        $field->mergeWith($other)->shouldBeCalled()->willReturn($merged);
+
+        $structure = new Structure();
+        $structure->add($field->reveal());
+        $structure->add($other);
+        $this->assertEquals($merged, $structure->get('foo'));
+    }
+
     public function testFieldsRestrictions()
     {
         $structure = new Structure();
-        $unrestricted_field = new Field('foo', Mapping::TYPE_STRING, true, false);
+        $unrestricted_field = new Field('foo', Mapping::TYPE_STRING, ['private' => false]);
         $structure->add($unrestricted_field);
-        $private_field = new Field('bar', Mapping::TYPE_STRING, true, true);
+        $private_field = new Field('bar', Mapping::TYPE_STRING, ['private' => true]);
         $structure->add($private_field);
         // All
         $all_fields = $structure->getAllFields();
@@ -70,6 +93,49 @@ class StructureTest extends \PHPUnit_Framework_TestCase
         $this->assertNotContains($unrestricted_field, $private_fields);
     }
 
+    public function testGetFacetFields()
+    {
+        $facet = new Field('foo', Mapping::TYPE_STRING, ['facet' => true]);
+        $not_facet = new Field('bar', Mapping::TYPE_STRING, ['facet' => false]);
+        $structure = new Structure();
+        $structure->add($facet);
+        $this->assertContains($facet, $structure->getFacetFields());
+        $structure->add($not_facet);
+        $facet_fields = $structure->getFacetFields();
+        $this->assertContains($facet, $facet_fields);
+        $this->assertNotContains($not_facet, $facet_fields);
+    }
+
+    public function testGetDateFields()
+    {
+        $string = new Field('foo', Mapping::TYPE_STRING);
+        $date = new Field('bar', Mapping::TYPE_DATE);
+        $structure = new Structure();
+        $structure->add($string);
+        $this->assertNotContains($string, $structure->getDateFields());
+        $structure->add($date);
+        $date_fields = $structure->getDateFields();
+        $this->assertContains($date, $date_fields);
+        $this->assertNotContains($string, $date_fields);
+    }
+
+    public function testGetThesaurusEnabledFields()
+    {
+        $not_enabled = new Field('foo', Mapping::TYPE_STRING, [
+            'thesaurus_roots' => null
+        ]);
+        $enabled = new Field('bar', Mapping::TYPE_STRING, [
+            'thesaurus_roots' => [new Concept('/foo')]
+        ]);
+        $structure = new Structure();
+        $structure->add($not_enabled);
+        $this->assertNotContains($not_enabled, $structure->getThesaurusEnabledFields());
+        $structure->add($enabled);
+        $enabled_fields = $structure->getThesaurusEnabledFields();
+        $this->assertContains($enabled, $enabled_fields);
+        $this->assertNotContains($not_enabled, $enabled_fields);
+    }
+
     public function testGet()
     {
         $structure = new Structure();
@@ -90,11 +156,11 @@ class StructureTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(Mapping::TYPE_DOUBLE, $structure->typeOf('baz'));
     }
 
-    public function testPrivateFieldCheck()
+    public function testPrivateCheck()
     {
         $structure = new Structure();
-        $structure->add(new Field('foo', Mapping::TYPE_STRING, true, false)); // Unrestricted field
-        $structure->add(new Field('bar', Mapping::TYPE_STRING, true, true)); // Private field
+        $structure->add(new Field('foo', Mapping::TYPE_STRING, ['private' => false]));
+        $structure->add(new Field('bar', Mapping::TYPE_STRING, ['private' => true]));
         $this->assertFalse($structure->isPrivate('foo'));
         $this->assertTrue($structure->isPrivate('bar'));
     }