jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/333995 )

Change subject: Refactor search fields: use ContentHandler instead of hooks
......................................................................


Refactor search fields: use ContentHandler instead of hooks

This makes existing fields indepenedent from Elastic and using
standard field definitions from SearchEngine.
Also removes usage of hooks since ContentHandler classes now
can perform this function.

NOTE: next patches will build on this one.

Change-Id: If9e945a7730a2d9797b8b57ce08dec6b882f588b
---
M repo/Wikibase.php
M repo/includes/Content/EntityHandler.php
M repo/includes/Content/ItemHandler.php
D repo/includes/Hooks/CirrusSearchHookHandlers.php
M repo/includes/Search/Elastic/Fields/LabelCountField.php
M repo/includes/Search/Elastic/Fields/SearchIndexField.php
M repo/includes/Search/Elastic/Fields/SiteLinkCountField.php
M repo/includes/Search/Elastic/Fields/StatementCountField.php
A repo/includes/Search/Elastic/Fields/WikibaseNumericField.php
M repo/tests/phpunit/includes/Content/EntityHandlerTest.php
M repo/tests/phpunit/includes/Content/ItemHandlerTest.php
M repo/tests/phpunit/includes/Content/PropertyHandlerTest.php
D repo/tests/phpunit/includes/Hooks/CirrusSearchHookHandlersTest.php
M repo/tests/phpunit/includes/Search/Elastic/Fields/LabelCountFieldTest.php
M repo/tests/phpunit/includes/Search/Elastic/Fields/SiteLinkCountFieldTest.php
M repo/tests/phpunit/includes/Search/Elastic/Fields/StatementCountFieldTest.php
A repo/tests/phpunit/includes/Search/Elastic/Fields/WikibaseNumericFieldTest.php
17 files changed, 217 insertions(+), 380 deletions(-)

Approvals:
  Aude: Looks good to me, approved
  jenkins-bot: Verified
  DCausse: Looks good to me, but someone else must approve



diff --git a/repo/Wikibase.php b/repo/Wikibase.php
index 5236193..8c91be0 100644
--- a/repo/Wikibase.php
+++ b/repo/Wikibase.php
@@ -879,10 +879,6 @@
        $wgHooks['BeforeDisplayNoArticleText'][] = 
'Wikibase\ViewEntityAction::onBeforeDisplayNoArticleText';
        $wgHooks['InfoAction'][] = '\Wikibase\RepoHooks::onInfoAction';
 
-       // CirrusSearch hooks
-       $wgHooks['CirrusSearchMappingConfig'][] = 
'Wikibase\Repo\Hooks\CirrusSearchHookHandlers::onCirrusSearchMappingConfig';
-       $wgHooks['CirrusSearchBuildDocumentParse'][] = 
'Wikibase\Repo\Hooks\CirrusSearchHookHandlers::onCirrusSearchBuildDocumentParse';
-
        // update hooks
        $wgHooks['LoadExtensionSchemaUpdates'][] = 
'\Wikibase\Repo\Store\Sql\ChangesSubscriptionSchemaUpdater::onSchemaUpdate';
 
diff --git a/repo/includes/Content/EntityHandler.php 
b/repo/includes/Content/EntityHandler.php
index ad872e0..1ae4711 100644
--- a/repo/includes/Content/EntityHandler.php
+++ b/repo/includes/Content/EntityHandler.php
@@ -13,8 +13,10 @@
 use MWContentSerializationException;
 use MWException;
 use ParserOptions;
+use ParserOutput;
 use RequestContext;
 use Revision;
+use SearchEngine;
 use Title;
 use User;
 use Wikibase\Content\DeferredDecodingEntityHolder;
@@ -28,12 +30,14 @@
 use Wikibase\EntityContent;
 use Wikibase\Lib\Store\EntityContentDataCodec;
 use Wikibase\Repo\Diff\EntityContentDiffView;
+use Wikibase\Repo\Search\Elastic\Fields\WikibaseFieldDefinitions;
 use Wikibase\Repo\Store\EntityPerPage;
 use Wikibase\Repo\Validators\EntityConstraintProvider;
 use Wikibase\Repo\Validators\EntityValidator;
 use Wikibase\Repo\Validators\ValidatorErrorLocalizer;
 use Wikibase\Repo\WikibaseRepo;
 use Wikibase\TermIndex;
+use WikiPage;
 
 /**
  * Base handler class for Entity content classes.
@@ -52,6 +56,7 @@
         * to parser output.
         */
        const PARSER_VERSION = 3;
+       protected $fieldDefinitions;
 
        /**
         * @var EntityPerPage
@@ -129,6 +134,8 @@
                $this->errorLocalizer = $errorLocalizer;
                $this->entityIdParser = $entityIdParser;
                $this->legacyExportFormatDetector = $legacyExportFormatDetector;
+               // FIXME: convert to DI, will be in the next patch
+               $this->fieldDefinitions = new WikibaseFieldDefinitions();
        }
 
        /**
@@ -332,7 +339,7 @@
         * @param string $blob
         * @param string|null $format
         *
-        * @return string|void
+        * @return string
         */
        public function exportTransform( $blob, $format = null ) {
                if ( !$this->legacyExportFormatDetector ) {
@@ -759,4 +766,41 @@
                return false;
        }
 
+       /**
+        * @param SearchEngine $engine
+        * @return \SearchIndexField[] List of fields this content handler can 
provide.
+        */
+       public function getFieldsForSearchIndex( SearchEngine $engine ) {
+               $fields = [];
+
+               foreach ( $this->fieldDefinitions->getFields() as $name => 
$field ) {
+                       $fields[$name] = $field->getMapping( $engine, $name );
+               }
+
+               return $fields;
+       }
+
+       /**
+        * @param WikiPage $page
+        * @param ParserOutput $output
+        * @param SearchEngine $engine
+        * @return array
+        */
+       public function getDataForSearchIndex( WikiPage $page, ParserOutput 
$output,
+                                              SearchEngine $engine ) {
+               $fieldsData = parent::getDataForSearchIndex( $page, $output, 
$engine );
+
+               $content = $page->getContent();
+               if ( ( $content instanceof EntityContent ) && 
!$content->isRedirect() ) {
+                       $entity = $content->getEntity();
+                       $fields = $this->fieldDefinitions->getFields();
+
+                       foreach ( $fields as $fieldName => $field ) {
+                               $fieldsData[$fieldName] = $field->getFieldData( 
$entity );
+                       }
+               }
+
+               return $fieldsData;
+       }
+
 }
diff --git a/repo/includes/Content/ItemHandler.php 
b/repo/includes/Content/ItemHandler.php
index e12dac2..97e4d44 100644
--- a/repo/includes/Content/ItemHandler.php
+++ b/repo/includes/Content/ItemHandler.php
@@ -6,6 +6,7 @@
 use IContextSource;
 use Page;
 use Title;
+use Wikibase\DataModel\Entity\EntityDocument;
 use Wikibase\DataModel\Entity\EntityId;
 use Wikibase\DataModel\Entity\Item;
 use Wikibase\DataModel\Entity\ItemId;
@@ -194,7 +195,7 @@
        /**
         * @see EntityHandler::makeEmptyEntity()
         *
-        * @return EntityContent
+        * @return EntityDocument
         */
        public function makeEmptyEntity() {
                return new Item();
diff --git a/repo/includes/Hooks/CirrusSearchHookHandlers.php 
b/repo/includes/Hooks/CirrusSearchHookHandlers.php
deleted file mode 100644
index a7910cf..0000000
--- a/repo/includes/Hooks/CirrusSearchHookHandlers.php
+++ /dev/null
@@ -1,113 +0,0 @@
-<?php
-
-namespace Wikibase\Repo\Hooks;
-
-use CirrusSearch\Connection;
-use CirrusSearch\Maintenance\MappingConfigBuilder;
-use Content;
-use Elastica\Document;
-use ParserOutput;
-use Title;
-use UnexpectedValueException;
-use Wikibase\EntityContent;
-use Wikibase\Repo\Search\Elastic\Fields\WikibaseFieldDefinitions;
-
-/**
- * @license GPL-2.0+
- * @author Katie Filbert < [email protected] >
- */
-class CirrusSearchHookHandlers {
-
-       /**
-        * @var WikibaseFieldDefinitions
-        */
-       private $fieldDefinitions;
-
-       /**
-        * @param Document $document
-        * @param Title $title
-        * @param Content $content
-        * @param ParserOutput $parserOutput
-        * @param Connection $connection
-        *
-        * @return bool
-        */
-       public static function onCirrusSearchBuildDocumentParse(
-               Document $document,
-               Title $title,
-               Content $content,
-               ParserOutput $parserOutput,
-               Connection $connection
-       ) {
-               $hookHandler = self::newFromGlobalState();
-               $hookHandler->indexExtraFields( $document, $content );
-
-               return true;
-       }
-
-       /**
-        * @param array &$config
-        * @param MappingConfigBuilder $mappingConfigBuilder
-        *
-        * @return bool
-        */
-       public static function onCirrusSearchMappingConfig(
-               array &$config,
-               MappingConfigBuilder $mappingConfigBuilder
-       ) {
-               $handler = self::newFromGlobalState();
-               $handler->addExtraFieldsToMappingConfig( $config );
-
-               return true;
-       }
-
-       /**
-        * @return self
-        */
-       public static function newFromGlobalState() {
-               return new self( new WikibaseFieldDefinitions() );
-       }
-
-       /**
-        * @param WikibaseFieldDefinitions $fieldDefinitions
-        */
-       public function __construct( WikibaseFieldDefinitions $fieldDefinitions 
) {
-               $this->fieldDefinitions = $fieldDefinitions;
-       }
-
-       /**
-        * @param Document $document
-        * @param Content $content
-        */
-       public function indexExtraFields( Document $document, Content $content 
) {
-               if ( !$content instanceof EntityContent || 
$content->isRedirect() === true ) {
-                       return;
-               }
-
-               $fields = $this->fieldDefinitions->getFields();
-               $entity = $content->getEntity();
-
-               foreach ( $fields as $fieldName => $field ) {
-                       $data = $field->getFieldData( $entity );
-                       $document->set( $fieldName, $data );
-               }
-       }
-
-       /**
-        * @param array &$config
-        *
-        * @throws UnexpectedValueException
-        */
-       public function addExtraFieldsToMappingConfig( array &$config ) {
-               $fields = $this->fieldDefinitions->getFields();
-
-               foreach ( $fields as $fieldName => $field ) {
-                       if ( array_key_exists( $fieldName, 
$config['page']['properties'] ) ) {
-                               throw new UnexpectedValueException( "$fieldName 
is already set in the mapping." );
-                       }
-
-                       $config['page']['properties'][$fieldName] = 
$field->getMapping();
-               }
-       }
-
-}
diff --git a/repo/includes/Search/Elastic/Fields/LabelCountField.php 
b/repo/includes/Search/Elastic/Fields/LabelCountField.php
index 12f8879..d717351 100644
--- a/repo/includes/Search/Elastic/Fields/LabelCountField.php
+++ b/repo/includes/Search/Elastic/Fields/LabelCountField.php
@@ -9,18 +9,7 @@
  * @license GPL-2.0+
  * @author Katie Filbert < [email protected] >
  */
-class LabelCountField implements SearchIndexField {
-
-       /**
-        * @see SearchIndexField::getMapping
-        *
-        * @return array
-        */
-       public function getMapping() {
-               return array(
-                       'type' => 'integer'
-               );
-       }
+class LabelCountField extends WikibaseNumericField {
 
        /**
         * @see SearchIndexField::getFieldData
diff --git a/repo/includes/Search/Elastic/Fields/SearchIndexField.php 
b/repo/includes/Search/Elastic/Fields/SearchIndexField.php
index f9dcaa1..fa3d9ab 100644
--- a/repo/includes/Search/Elastic/Fields/SearchIndexField.php
+++ b/repo/includes/Search/Elastic/Fields/SearchIndexField.php
@@ -2,6 +2,7 @@
 
 namespace Wikibase\Repo\Search\Elastic\Fields;
 
+use SearchEngine;
 use Wikibase\DataModel\Entity\EntityDocument;
 
 /**
@@ -18,14 +19,12 @@
 interface SearchIndexField {
 
        /**
-        * @return array The field mapping defines attributes of the field,
-        *               such as the field type (e.g. "string", "long", 
"nested")
-        *               and other attributes like "not_analyzed".
-        *
-        *               For detailed documentation about mapping of fields, 
see:
-        *               
https://www.elastic.co/guide/en/elasticsearch/guide/current/mapping-intro.html
+        * Produce specific field mapping
+        * @param SearchEngine $engine
+        * @param string $name
+        * @return \SearchIndexField
         */
-       public function getMapping();
+       public function getMapping( SearchEngine $engine, $name );
 
        /**
         * @param EntityDocument $entity
diff --git a/repo/includes/Search/Elastic/Fields/SiteLinkCountField.php 
b/repo/includes/Search/Elastic/Fields/SiteLinkCountField.php
index 5cc6a4b..4625e06 100644
--- a/repo/includes/Search/Elastic/Fields/SiteLinkCountField.php
+++ b/repo/includes/Search/Elastic/Fields/SiteLinkCountField.php
@@ -9,18 +9,7 @@
  * @license GPL-2.0+
  * @author Katie Filbert < [email protected] >
  */
-class SiteLinkCountField implements SearchIndexField {
-
-       /**
-        * @see SearchIndexField::getMapping
-        *
-        * @return array
-        */
-       public function getMapping() {
-               return array(
-                       'type' => 'integer'
-               );
-       }
+class SiteLinkCountField extends WikibaseNumericField {
 
        /**
         * @see SearchIndexField::getFieldData
diff --git a/repo/includes/Search/Elastic/Fields/StatementCountField.php 
b/repo/includes/Search/Elastic/Fields/StatementCountField.php
index 0ee5d01..308bc3e 100644
--- a/repo/includes/Search/Elastic/Fields/StatementCountField.php
+++ b/repo/includes/Search/Elastic/Fields/StatementCountField.php
@@ -9,18 +9,7 @@
  * @license GPL-2.0+
  * @author Katie Filbert < [email protected] >
  */
-class StatementCountField implements SearchIndexField {
-
-       /**
-        * @see SearchIndexField::getMapping
-        *
-        * @return array
-        */
-       public function getMapping() {
-               return array(
-                       'type' => 'integer'
-               );
-       }
+class StatementCountField extends WikibaseNumericField {
 
        /**
         * @see SearchIndexField::getFieldData
diff --git a/repo/includes/Search/Elastic/Fields/WikibaseNumericField.php 
b/repo/includes/Search/Elastic/Fields/WikibaseNumericField.php
new file mode 100644
index 0000000..4715153
--- /dev/null
+++ b/repo/includes/Search/Elastic/Fields/WikibaseNumericField.php
@@ -0,0 +1,24 @@
+<?php
+namespace Wikibase\Repo\Search\Elastic\Fields;
+
+use SearchEngine;
+
+/**
+ * Generic numeric field
+ * @package Wikibase\Repo\Search\Elastic\Fields
+ */
+abstract class WikibaseNumericField implements SearchIndexField {
+
+       /**
+        * @param SearchEngine $engine
+        * @param string       $name
+        * @return \SearchIndexField
+        */
+       public function getMapping( SearchEngine $engine, $name ) {
+               return $engine->makeSearchFieldMapping(
+                       $name,
+                       \SearchIndexField::INDEX_TYPE_INTEGER
+               );
+       }
+
+}
diff --git a/repo/tests/phpunit/includes/Content/EntityHandlerTest.php 
b/repo/tests/phpunit/includes/Content/EntityHandlerTest.php
index 1882772..73ce601 100644
--- a/repo/tests/phpunit/includes/Content/EntityHandlerTest.php
+++ b/repo/tests/phpunit/includes/Content/EntityHandlerTest.php
@@ -4,6 +4,7 @@
 
 use ContentHandler;
 use DataValues\Serializers\DataValueSerializer;
+use DummySearchIndexFieldDefinition;
 use FauxRequest;
 use InvalidArgumentException;
 use Language;
@@ -19,14 +20,17 @@
 use Wikibase\DataModel\SerializerFactory;
 use Wikibase\DataModel\Term\LabelsProvider;
 use Wikibase\EntityContent;
+use Wikibase\ItemContent;
 use Wikibase\Lib\DataTypeDefinitions;
 use Wikibase\Lib\EntityTypeDefinitions;
 use Wikibase\Lib\RepositoryDefinitions;
 use Wikibase\Repo\Content\EntityHandler;
+use Wikibase\Repo\Content\ItemHandler;
 use Wikibase\Repo\Validators\EntityValidator;
 use Wikibase\Repo\Validators\ValidatorErrorLocalizer;
 use Wikibase\Repo\WikibaseRepo;
 use Wikibase\SettingsArray;
+use WikiPage;
 use WikitextContent;
 
 /**
@@ -481,4 +485,44 @@
                $this->assertFalse( $this->getHandler()->supportsCategories() );
        }
 
+       public function testFieldsForSearchIndex() {
+               $handler = $this->getHandler();
+
+               $searchEngine = $this->getMockBuilder( 'SearchEngine' 
)->getMock();
+
+               $searchEngine->expects( $this->any() )
+                       ->method( 'makeSearchFieldMapping' )
+                       ->will( $this->returnCallback( function ( $name, $type 
) {
+                               return new DummySearchIndexFieldDefinition( 
$name, $type );
+                       } ) );
+
+               $fields = $handler->getFieldsForSearchIndex( $searchEngine );
+               $expectedFields = [ 'label_count', 'sitelink_count', 
'statement_count' ];
+               foreach ( $expectedFields as $expected ) {
+                       $this->assertInstanceOf( \SearchIndexField::class, 
$fields[$expected] );
+                       $mapping = $fields[$expected]->getMapping( 
$searchEngine );
+                       $this->assertEquals( $expected, $mapping['name'] );
+               }
+       }
+
+       abstract protected function getTestItemContent();
+
+       public function testDataForSearchIndex() {
+               $handler = $this->getHandler();
+               $engine = $this->getMock( \SearchEngine::class );
+
+               $page =
+                       $this->getMockBuilder( WikiPage::class )
+                               ->setConstructorArgs( [ Title::newFromText( 
'Q1' ) ] )
+                               ->getMock();
+               $page->method( 'getContent' )->willReturn( 
$this->getTestItemContent() );
+
+               $data = $handler->getDataForSearchIndex( $page, new 
\ParserOutput(), $engine );
+               $this->assertSame( 1, $data['label_count'], 'label_count' );
+               if ( $handler instanceof ItemHandler ) {
+                       $this->assertSame( 1, $data['sitelink_count'], 
'sitelink_count' );
+               }
+               $this->assertSame( 1, $data['statement_count'], 
'statement_count' );
+       }
+
 }
diff --git a/repo/tests/phpunit/includes/Content/ItemHandlerTest.php 
b/repo/tests/phpunit/includes/Content/ItemHandlerTest.php
index 70259b3..8f34b94 100644
--- a/repo/tests/phpunit/includes/Content/ItemHandlerTest.php
+++ b/repo/tests/phpunit/includes/Content/ItemHandlerTest.php
@@ -3,16 +3,21 @@
 namespace Wikibase\Repo\Tests\Content;
 
 use MWException;
+use Title;
 use Wikibase\Content\EntityInstanceHolder;
 use Wikibase\DataModel\Entity\EntityDocument;
 use Wikibase\DataModel\Entity\EntityId;
 use Wikibase\DataModel\Entity\EntityRedirect;
 use Wikibase\DataModel\Entity\Item;
 use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Snak\PropertyNoValueSnak;
 use Wikibase\EntityContent;
 use Wikibase\ItemContent;
 use Wikibase\Repo\Content\ItemHandler;
+use Wikibase\Repo\WikibaseRepo;
 use Wikibase\SettingsArray;
+use WikiPage;
 
 /**
  * @covers Wikibase\Repo\Content\ItemHandler
@@ -153,4 +158,15 @@
                $this->assertFalse( $handler->canCreateWithCustomId( $id ) );
        }
 
+       protected function getTestItemContent() {
+               $item = new Item();
+               $item->getFingerprint()->setLabel( 'en', 'Kitten' );
+               $item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Kitten' );
+               $item->getStatements()->addNewStatement(
+                       new PropertyNoValueSnak( new PropertyId( 'P1' ) )
+               );
+
+               return ItemContent::newFromItem( $item );
+       }
+
 }
diff --git a/repo/tests/phpunit/includes/Content/PropertyHandlerTest.php 
b/repo/tests/phpunit/includes/Content/PropertyHandlerTest.php
index 37019dd..c63d6ed 100644
--- a/repo/tests/phpunit/includes/Content/PropertyHandlerTest.php
+++ b/repo/tests/phpunit/includes/Content/PropertyHandlerTest.php
@@ -6,6 +6,7 @@
 use Wikibase\DataModel\Entity\EntityId;
 use Wikibase\DataModel\Entity\Property;
 use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Snak\PropertyNoValueSnak;
 use Wikibase\PropertyContent;
 use Wikibase\Repo\Content\PropertyHandler;
 use Wikibase\SettingsArray;
@@ -110,4 +111,14 @@
                $this->assertFalse( $handler->canCreateWithCustomId( $id ) );
        }
 
+       protected function getTestItemContent() {
+               $item = new Property( null, null, 'string' );
+               $item->getFingerprint()->setLabel( 'en', 'Kitten' );
+               $item->getStatements()->addNewStatement(
+                       new PropertyNoValueSnak( new PropertyId( 'P1' ) )
+               );
+
+               return PropertyContent::newFromProperty( $item );
+       }
+
 }
diff --git a/repo/tests/phpunit/includes/Hooks/CirrusSearchHookHandlersTest.php 
b/repo/tests/phpunit/includes/Hooks/CirrusSearchHookHandlersTest.php
deleted file mode 100644
index ded9389..0000000
--- a/repo/tests/phpunit/includes/Hooks/CirrusSearchHookHandlersTest.php
+++ /dev/null
@@ -1,161 +0,0 @@
-<?php
-
-namespace Wikibase\Repo\Tests\Hooks;
-
-use CirrusSearch\Connection;
-use CirrusSearch\Maintenance\MappingConfigBuilder;
-use CirrusSearch;
-use Elastica\Document;
-use ParserOutput;
-use PHPUnit_Framework_TestCase;
-use Title;
-use UnexpectedValueException;
-use Wikibase\DataModel\Entity\Item;
-use Wikibase\DataModel\Entity\PropertyId;
-use Wikibase\DataModel\Snak\PropertyNoValueSnak;
-use Wikibase\Repo\Search\Elastic\Fields\WikibaseFieldDefinitions;
-use Wikibase\Repo\Hooks\CirrusSearchHookHandlers;
-use Wikibase\Repo\WikibaseRepo;
-
-/**
- * @covers Wikibase\Repo\Hooks\CirrusSearchHookHandlers
- *
- * @group WikibaseElastic
- * @group Wikibase
- *
- * @license GPL-2.0+
- * @author Katie Filbert < [email protected] >
- */
-class CirrusSearchHookHandlersTest extends PHPUnit_Framework_TestCase {
-
-       protected function setUp() {
-               parent::setUp();
-
-               if ( !class_exists( CirrusSearch::class ) ) {
-                       $this->markTestSkipped( 'CirrusSearch is not available' 
);
-               }
-       }
-
-       public function testOnCirrusSearchBuildDocumentParse() {
-               $connection = $this->getMockBuilder( Connection::class )
-                       ->disableOriginalConstructor()
-                       ->getMock();
-
-               $document = new Document();
-
-               CirrusSearchHookHandlers::onCirrusSearchBuildDocumentParse(
-                       $document,
-                       Title::newFromText( 'Q1' ),
-                       $this->getContent(),
-                       new ParserOutput(),
-                       $connection
-               );
-
-               $this->assertSame( 1, $document->get( 'label_count' ), 
'label_count' );
-               $this->assertSame( 1, $document->get( 'sitelink_count' ), 
'sitelink_count' );
-               $this->assertSame( 1, $document->get( 'statement_count' ), 
'statement_count' );
-       }
-
-       public function testOnCirrusSearchMappingConfig() {
-               $mappingConfigBuilder = $this->getMockBuilder( 
MappingConfigBuilder::class )
-                       ->disableOriginalConstructor()
-                       ->getMock();
-
-               $config = array(
-                       'page' => array(
-                               'properties' => array()
-                       )
-               );
-
-               CirrusSearchHookHandlers::onCirrusSearchMappingConfig( $config, 
$mappingConfigBuilder );
-
-               $this->assertSame(
-                       array( 'label_count', 'sitelink_count', 
'statement_count' ),
-                       array_keys( $config['page']['properties'] )
-               );
-       }
-
-       public function testIndexExtraFields() {
-               $fieldDefinitions = $this->newFieldDefinitions();
-
-               $document = new Document();
-               $content = $this->getContent();
-
-               $hookHandlers = new CirrusSearchHookHandlers( $fieldDefinitions 
);
-               $hookHandlers->indexExtraFields( $document, $content );
-
-               $this->assertSame( 1, $document->get( 'label_count' ), 
'label_count' );
-               $this->assertSame( 1, $document->get( 'sitelink_count' ), 
'sitelink_count' );
-               $this->assertSame( 1, $document->get( 'statement_count' ), 
'statement_count' );
-       }
-
-       public function testAddExtraFieldsToMappingConfig() {
-               $fieldDefinitions = $this->newFieldDefinitions();
-
-               $config = array(
-                       'page' => array(
-                               'properties' => array()
-                       )
-               );
-
-               $hookHandlers = new CirrusSearchHookHandlers( $fieldDefinitions 
);
-               $hookHandlers->addExtraFieldsToMappingConfig( $config );
-
-               $expected = array(
-                       'page' => array(
-                               'properties' => array(
-                                       'label_count' => array(
-                                               'type' => 'integer'
-                                       ),
-                                       'sitelink_count' => array(
-                                               'type' => 'integer'
-                                       ),
-                                       'statement_count' => array(
-                                               'type' => 'integer'
-                                       )
-                               )
-                       )
-               );
-
-               $this->assertSame( $expected, $config );
-       }
-
-       public function 
testAddExtraFields_throwsExceptionIfFieldNameAlreadySet() {
-               $fieldDefinitions = $this->newFieldDefinitions();
-
-               $config = array(
-                       'page' => array(
-                               'properties' => array(
-                                       'sitelink_count' => array(
-                                               'type' => 'long'
-                                       )
-                               )
-                       )
-               );
-
-               $this->setExpectedException( UnexpectedValueException::class );
-
-               $hookHandlers = new CirrusSearchHookHandlers( $fieldDefinitions 
);
-               $hookHandlers->addExtraFieldsToMappingConfig( $config );
-       }
-
-       private function newFieldDefinitions() {
-               // when we add multilingual fields, then 
WikibaseFieldDefinitions
-               // will take WikibaseContentLanguages as an argument.
-               return new WikibaseFieldDefinitions();
-       }
-
-       private function getContent() {
-               $item = new Item();
-               $item->getFingerprint()->setLabel( 'en', 'Kitten' );
-               $item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Kitten' );
-               $item->getStatements()->addNewStatement(
-                       new PropertyNoValueSnak( new PropertyId( 'P1' ) )
-               );
-
-               $entityContentFactory = 
WikibaseRepo::getDefaultInstance()->getEntityContentFactory();
-
-               return $entityContentFactory->newFromEntity( $item );
-       }
-
-}
diff --git 
a/repo/tests/phpunit/includes/Search/Elastic/Fields/LabelCountFieldTest.php 
b/repo/tests/phpunit/includes/Search/Elastic/Fields/LabelCountFieldTest.php
index 815f825..c60a0fe 100644
--- a/repo/tests/phpunit/includes/Search/Elastic/Fields/LabelCountFieldTest.php
+++ b/repo/tests/phpunit/includes/Search/Elastic/Fields/LabelCountFieldTest.php
@@ -2,11 +2,10 @@
 
 namespace Wikibase\Repo\Tests\Search\Elastic\Fields;
 
-use PHPUnit_Framework_TestCase;
-use Wikibase\DataModel\Entity\EntityDocument;
 use Wikibase\DataModel\Entity\Item;
 use Wikibase\DataModel\Entity\Property;
 use Wikibase\Repo\Search\Elastic\Fields\LabelCountField;
+use Wikibase\Repo\Search\Elastic\Fields\WikibaseNumericField;
 
 /**
  * @covers Wikibase\Repo\Search\Elastic\Fields\LabelCountField
@@ -17,26 +16,7 @@
  * @license GPL-2.0+
  * @author Katie Filbert < [email protected] >
  */
-class LabelCountFieldTest extends PHPUnit_Framework_TestCase {
-
-       public function testGetMapping() {
-               $labelCountField = new LabelCountField();
-
-               $expected = array(
-                       'type' => 'integer'
-               );
-
-               $this->assertSame( $expected, $labelCountField->getMapping() );
-       }
-
-       /**
-        * @dataProvider getFieldDataProvider
-        */
-       public function testGetFieldData( $expected, EntityDocument $entity ) {
-               $labelCountField = new LabelCountField();
-
-               $this->assertSame( $expected, $labelCountField->getFieldData( 
$entity ) );
-       }
+class LabelCountFieldTest extends WikibaseNumericFieldTest {
 
        public function getFieldDataProvider() {
                $item = new Item();
@@ -48,4 +28,11 @@
                );
        }
 
+       /**
+        * @return WikibaseNumericField
+        */
+       protected function getFieldObject() {
+               return new LabelCountField();
+       }
+
 }
diff --git 
a/repo/tests/phpunit/includes/Search/Elastic/Fields/SiteLinkCountFieldTest.php 
b/repo/tests/phpunit/includes/Search/Elastic/Fields/SiteLinkCountFieldTest.php
index c7c1665..0de4ece 100644
--- 
a/repo/tests/phpunit/includes/Search/Elastic/Fields/SiteLinkCountFieldTest.php
+++ 
b/repo/tests/phpunit/includes/Search/Elastic/Fields/SiteLinkCountFieldTest.php
@@ -2,11 +2,10 @@
 
 namespace Wikibase\Repo\Tests\Search\Elastic\Fields;
 
-use PHPUnit_Framework_TestCase;
-use Wikibase\DataModel\Entity\EntityDocument;
 use Wikibase\DataModel\Entity\Item;
 use Wikibase\DataModel\Entity\Property;
 use Wikibase\Repo\Search\Elastic\Fields\SiteLinkCountField;
+use Wikibase\Repo\Search\Elastic\Fields\WikibaseNumericField;
 
 /**
  * @covers Wikibase\Repo\Search\Elastic\Fields\SiteLinkCountField
@@ -17,25 +16,13 @@
  * @license GPL-2.0+
  * @author Katie Filbert < [email protected] >
  */
-class SiteLinkCountFieldTest extends PHPUnit_Framework_TestCase {
-
-       public function testGetMapping() {
-               $siteLinkCountField = new SiteLinkCountField();
-
-               $expected = array(
-                       'type' => 'integer'
-               );
-
-               $this->assertSame( $expected, $siteLinkCountField->getMapping() 
);
-       }
+class SiteLinkCountFieldTest extends WikibaseNumericFieldTest {
 
        /**
-        * @dataProvider getFieldDataProvider
+        * @return WikibaseNumericField
         */
-       public function testGetFieldData( $expected, EntityDocument $entity ) {
-               $siteLinkCountField = new SiteLinkCountField();
-
-               $this->assertSame( $expected, 
$siteLinkCountField->getFieldData( $entity ) );
+       protected function getFieldObject() {
+               return new SiteLinkCountField();
        }
 
        public function getFieldDataProvider() {
diff --git 
a/repo/tests/phpunit/includes/Search/Elastic/Fields/StatementCountFieldTest.php 
b/repo/tests/phpunit/includes/Search/Elastic/Fields/StatementCountFieldTest.php
index f98ea38..49d6371 100644
--- 
a/repo/tests/phpunit/includes/Search/Elastic/Fields/StatementCountFieldTest.php
+++ 
b/repo/tests/phpunit/includes/Search/Elastic/Fields/StatementCountFieldTest.php
@@ -2,12 +2,12 @@
 
 namespace Wikibase\Repo\Tests\Search\Elastic\Fields;
 
-use PHPUnit_Framework_TestCase;
 use Wikibase\DataModel\Entity\EntityDocument;
 use Wikibase\DataModel\Entity\Item;
 use Wikibase\DataModel\Entity\Property;
 use Wikibase\DataModel\Snak\PropertyNoValueSnak;
 use Wikibase\Repo\Search\Elastic\Fields\StatementCountField;
+use Wikibase\Repo\Search\Elastic\Fields\WikibaseNumericField;
 
 /**
  * @covers Wikibase\Repo\Search\Elastic\Fields\StatementCountField
@@ -18,25 +18,13 @@
  * @license GPL-2.0+
  * @author Katie Filbert < [email protected] >
  */
-class StatementCountFieldTest extends PHPUnit_Framework_TestCase {
-
-       public function testGetMapping() {
-               $statementCountField = new StatementCountField();
-
-               $expected = array(
-                       'type' => 'integer'
-               );
-
-               $this->assertSame( $expected, 
$statementCountField->getMapping() );
-       }
+class StatementCountFieldTest extends WikibaseNumericFieldTest {
 
        /**
-        * @dataProvider getFieldDataProvider
+        * @return WikibaseNumericField
         */
-       public function testGetFieldData( $expected, EntityDocument $entity ) {
-               $statementCountField = new StatementCountField();
-
-               $this->assertSame( $expected, 
$statementCountField->getFieldData( $entity ) );
+       protected function getFieldObject() {
+               return new StatementCountField();
        }
 
        public function getFieldDataProvider() {
diff --git 
a/repo/tests/phpunit/includes/Search/Elastic/Fields/WikibaseNumericFieldTest.php
 
b/repo/tests/phpunit/includes/Search/Elastic/Fields/WikibaseNumericFieldTest.php
new file mode 100644
index 0000000..3293a15
--- /dev/null
+++ 
b/repo/tests/phpunit/includes/Search/Elastic/Fields/WikibaseNumericFieldTest.php
@@ -0,0 +1,47 @@
+<?php
+namespace Wikibase\Repo\Tests\Search\Elastic\Fields;
+
+use DummySearchIndexFieldDefinition;
+use PHPUnit_Framework_TestCase;
+use Wikibase\DataModel\Entity\EntityDocument;
+use Wikibase\Repo\Search\Elastic\Fields\WikibaseNumericField;
+
+/**
+ * Base class for testing numeric fields.
+ * @package Wikibase\Repo\Tests\Search\Elastic\Fields
+ */
+abstract class WikibaseNumericFieldTest extends PHPUnit_Framework_TestCase {
+
+       public function testGetMapping() {
+               $field = $this->getFieldObject();
+               $searchEngine = $this->getMockBuilder( 'SearchEngine' 
)->getMock();
+
+               $searchEngine->expects( $this->any() )
+                       ->method( 'makeSearchFieldMapping' )
+                       ->will( $this->returnCallback( function ( $name, $type 
) {
+                               return new DummySearchIndexFieldDefinition( 
$name, $type );
+                       } ) );
+
+               $mapping = $field->getMapping( $searchEngine, get_class( $field 
) )
+                       ->getMapping( $searchEngine );
+               $this->assertEquals( \SearchIndexField::INDEX_TYPE_INTEGER, 
$mapping['type'] );
+               $this->assertEquals( get_class( $field ), $mapping['name'] );
+       }
+
+       /**
+        * @dataProvider getFieldDataProvider
+        */
+       public function testGetFieldData( $expected, EntityDocument $entity ) {
+               $labelCountField = $this->getFieldObject();
+
+               $this->assertSame( $expected, $labelCountField->getFieldData( 
$entity ) );
+       }
+
+       abstract public function getFieldDataProvider();
+
+       /**
+        * @return WikibaseNumericField
+        */
+       abstract protected function getFieldObject();
+
+}

-- 
To view, visit https://gerrit.wikimedia.org/r/333995
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If9e945a7730a2d9797b8b57ce08dec6b882f588b
Gerrit-PatchSet: 22
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Smalyshev <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: DCausse <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: EBernhardson <[email protected]>
Gerrit-Reviewer: Smalyshev <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to