Matthias Mullie has uploaded a new change for review.
https://gerrit.wikimedia.org/r/176285
Change subject: Move validateMapping code into separate class
......................................................................
Move validateMapping code into separate class
Meanwhile removed the now unused checkConfig methods
Change-Id: I0c7192c59198a92d9ba3e22bf1b44db3522b2c24
---
M CirrusSearch.php
A includes/Maintenance/Validators/MappingValidator.php
M maintenance/updateOneSearchIndexConfig.php
3 files changed, 131 insertions(+), 120 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CirrusSearch
refs/changes/85/176285/1
diff --git a/CirrusSearch.php b/CirrusSearch.php
index 7f1b6ff..55a81ae 100644
--- a/CirrusSearch.php
+++ b/CirrusSearch.php
@@ -583,6 +583,7 @@
$wgAutoloadClasses['CirrusSearch\Maintenance\Validators\NumberOfShardsValidator']
= $maintenanceDir . '/Validators/NumberOfShardsValidator.php';
$wgAutoloadClasses['CirrusSearch\Maintenance\Validators\ReplicaRangeValidator']
= $maintenanceDir . '/Validators/ReplicaRangeValidator.php';
$wgAutoloadClasses['CirrusSearch\Maintenance\Validators\AnalyzersValidator'] =
$maintenanceDir . '/Validators/AnalyzersValidator.php';
+$wgAutoloadClasses['CirrusSearch\Maintenance\Validators\MappingValidator'] =
$maintenanceDir . '/Validators/MappingValidator.php';
$wgAutoloadClasses['CirrusSearch\Maintenance\UpdateVersionIndex'] = __DIR__ .
'/maintenance/updateVersionIndex.php';
$wgAutoloadClasses['CirrusSearch\NearMatchPicker'] = $includes .
'NearMatchPicker.php';
$wgAutoloadClasses['CirrusSearch\OtherIndexes'] = $includes .
'OtherIndexes.php';
diff --git a/includes/Maintenance/Validators/MappingValidator.php
b/includes/Maintenance/Validators/MappingValidator.php
new file mode 100644
index 0000000..32a1fc0
--- /dev/null
+++ b/includes/Maintenance/Validators/MappingValidator.php
@@ -0,0 +1,119 @@
+<?php
+
+namespace CirrusSearch\Maintenance\Validators;
+
+use CirrusSearch\ElasticsearchIntermediary;
+use CirrusSearch\Maintenance\Maintenance;
+use Elastica\Exception\ExceptionInterface;
+use Elastica\Index;
+use Elastica\Type;
+use Elastica\Type\Mapping;
+
+class MappingValidator extends Validator {
+ /**
+ * @var Index
+ */
+ private $index;
+
+ /**
+ * @var bool
+ */
+ private $optimizeIndexForExperimentalHighlighter;
+
+ /**
+ * @var array
+ */
+ private $availablePlugins;
+
+ /**
+ * @var array
+ */
+ private $mappingConfig;
+
+ /**
+ * @var Type
+ */
+ private $pageType;
+
+ /**
+ * @var Type
+ */
+ private $namespaceType;
+
+ /**
+ * @todo: this constructor takes way too much arguments - refactor
+ *
+ * @param Index $index
+ * @param bool $optimizeIndexForExperimentalHighlighter
+ * @param array $availablePlugins
+ * @param array $mappingConfig
+ * @param Type $pageType
+ * @param Type $namespaceType
+ * @param Maintenance $out
+ */
+ public function __construct( Index $index,
$optimizeIndexForExperimentalHighlighter, array $availablePlugins, array
$mappingConfig, Type $pageType, Type $namespaceType, Maintenance $out = null ) {
+ parent::__construct( $out );
+
+ $this->index = $index;
+ $this->optimizeIndexForExperimentalHighlighter =
$optimizeIndexForExperimentalHighlighter;
+ $this->availablePlugins = $availablePlugins;
+ $this->mappingConfig = $mappingConfig;
+ $this->pageType = $pageType;
+ $this->namespaceType = $namespaceType;
+ }
+
+ public function validate() {
+ $this->outputIndented( "Validating mappings..." );
+ if ( $this->optimizeIndexForExperimentalHighlighter &&
+ !in_array( 'experimental highlighter',
$this->availablePlugins ) ) {
+ $this->output( "impossible!\n" );
+ $this->error(
"wgCirrusSearchOptimizeIndexForExperimentalHighlighter is set to true but the "
.
+ "'experimental highlighter' plugin is not
installed on all hosts.", 1 );
+ return false;
+ }
+
+ $requiredMappings = $this->mappingConfig;
+ if ( !$this->checkMapping( $requiredMappings ) ) {
+ // TODO Conflict resolution here might leave old
portions of mappings
+ $pageAction = new Mapping( $this->pageType );
+ foreach ( $requiredMappings[ 'page' ] as $key => $value
) {
+ $pageAction->setParam( $key, $value );
+ }
+ $namespaceAction = new Mapping( $this->namespaceType );
+ foreach ( $requiredMappings[ 'namespace' ] as $key =>
$value ) {
+ $namespaceAction->setParam( $key, $value );
+ }
+ try {
+ $pageAction->send();
+ $namespaceAction->send();
+ $this->output( "corrected\n" );
+ } catch ( ExceptionInterface $e ) {
+ $this->output( "failed!\n" );
+ $message =
ElasticsearchIntermediary::extractMessage( $e );
+ $this->error( "Couldn't update mappings. Here
is elasticsearch's error message: $message\n", 1 );
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ /**
+ * Check that the mapping returned from Elasticsearch is as we want it.
+ *
+ * @param array $requiredMappings the mappings we want
+ * @return bool is the mapping good enough for us?
+ */
+ private function checkMapping( $requiredMappings ) {
+ $actualMappings = $this->index->getMapping();
+ $this->output( "\n" );
+ $this->outputIndented( "\tValidating mapping..." );
+ if ( $this->checkConfig( $actualMappings, $requiredMappings ) )
{
+ $this->output( "ok\n" );
+ return true;
+ } else {
+ $this->output( "different..." );
+ return false;
+ }
+ }
+}
diff --git a/maintenance/updateOneSearchIndexConfig.php
b/maintenance/updateOneSearchIndexConfig.php
index ed7fad4..fb954f9 100644
--- a/maintenance/updateOneSearchIndexConfig.php
+++ b/maintenance/updateOneSearchIndexConfig.php
@@ -366,126 +366,17 @@
}
private function validateMapping() {
- $this->outputIndented( "Validating mappings..." );
- if ( $this->optimizeIndexForExperimentalHighlighter &&
- !in_array( 'experimental highlighter',
$this->availablePlugins ) ) {
- $this->output( "impossible!\n" );
- $this->error(
"wgCirrusSearchOptimizeIndexForExperimentalHighlighter is set to true but the "
.
- "'experimental highlighter' plugin is not
installed on all hosts.", 1 );
- }
-
- $requiredMappings = $this->getMappingConfig();
- if ( !$this->checkMapping( $requiredMappings ) ) {
- // TODO Conflict resolution here might leave old
portions of mappings
- $pageAction = new \Elastica\Type\Mapping(
$this->getPageType() );
- foreach ( $requiredMappings[ 'page' ] as $key => $value
) {
- $pageAction->setParam( $key, $value );
- }
- $namespaceAction = new \Elastica\Type\Mapping(
$this->getNamespaceType() );
- foreach ( $requiredMappings[ 'namespace' ] as $key =>
$value ) {
- $namespaceAction->setParam( $key, $value );
- }
- try {
- $pageAction->send();
- $namespaceAction->send();
- $this->output( "corrected\n" );
- } catch ( \Elastica\Exception\ExceptionInterface $e ) {
- $this->output( "failed!\n" );
- $message =
ElasticsearchIntermediary::extractMessage( $e );
- $this->error( "Couldn't update mappings. Here
is elasticsearch's error message: $message\n", 1 );
- }
- }
- }
-
- /**
- * Check that the mapping returned from Elasticsearch is as we want it.
- * @param array $requiredMappings the mappings we want
- * @return bool is the mapping good enough for us?
- */
- private function checkMapping( $requiredMappings ) {
- $actualMappings = $this->getIndex()->getMapping();
- $this->output( "\n" );
- $this->outputIndented( "\tValidating mapping..." );
- if ( $this->checkConfig( $actualMappings, $requiredMappings ) )
{
- $this->output( "ok\n" );
- return true;
- } else {
- $this->output( "different..." );
- return false;
- }
- }
-
- /**
- * @param $actual
- * @param $required array
- * @return bool
- *
- * @deprecated Duplicate of
CirrusSearch\Maintenance\Validators\Validator - once all callers here have been
moved, this'll be removed
- */
- private function checkConfig( $actual, $required, $indent = null ) {
- foreach( $required as $key => $value ) {
- $this->debugCheckConfig( "\n$indent$key: " );
- if ( !array_key_exists( $key, $actual ) ) {
- $this->debugCheckConfig( "not found..." );
- if ( $key === '_all' ) {
- // The _all field never comes back so
we just have to assume it
- // is set correctly.
- $this->debugCheckConfig( "was the all
field so skipping..." );
- continue;
- }
- return false;
- }
- if ( is_array( $value ) ) {
- $this->debugCheckConfig( "descend..." );
- if ( !is_array( $actual[ $key ] ) ) {
- $this->debugCheckConfig( "other not
array..." );
- return false;
- }
- if ( !$this->checkConfig( $actual[ $key ],
$value, $indent . "\t" ) ) {
- return false;
- }
- continue;
- }
-
- $actual[ $key ] = $this->normalizeConfigValue( $actual[
$key ] );
- $value = $this->normalizeConfigValue( $value );
- $this->debugCheckConfig( $actual[ $key ] . " ??
$value..." );
- // Note that I really mean !=, not !==. Coercion is
cool here.
- // print $actual[ $key ] . " $value\n";
- if ( $actual[ $key ] != $value ) {
- $this->debugCheckConfig( 'different...' );
- return false;
- }
- }
- return true;
- }
-
- /**
- * Normalize a config value for comparison. Elasticsearch will accept
all kinds
- * of config values but it tends to through back 'true' for true and
'false' for
- * false so we normalize everything. Sometimes, oddly, it'll through
back false
- * for false....
- * @param mixed $value config value
- * @return mixes value normalized
- *
- * @deprecated Duplicate of
CirrusSearch\Maintenance\Validators\Validator - once all callers here have been
moved, this'll be removed
- */
- private function normalizeConfigValue( $value ) {
- if ( $value === true ) {
- return 'true';
- } else if ( $value === false ) {
- return 'false';
- }
- return $value;
- }
-
- /**
- * @deprecated Duplicate of
CirrusSearch\Maintenance\Validators\Validator - once all callers here have been
moved, this'll be removed
- */
- private function debugCheckConfig( $string ) {
- if ( $this->printDebugCheckConfig ) {
- $this->output( $string );
- }
+ $validator = new
\CirrusSearch\Maintenance\Validators\MappingValidator(
+ $this->getIndex(),
+ $this->optimizeIndexForExperimentalHighlighter,
+ $this->availablePlugins,
+ $this->getMappingConfig(),
+ $this->getPageType(),
+ $this->getNamespaceType(),
+ $this
+ );
+ $validator->printDebugCheckConfig( $this->printDebugCheckConfig
);
+ $validator->validate();
}
private function validateAlias() {
--
To view, visit https://gerrit.wikimedia.org/r/176285
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c7192c59198a92d9ba3e22bf1b44db3522b2c24
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
