From a609ccd24b50c684ccc7e922eb04c4a32b5f9e85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Thu, 16 Apr 2020 15:27:36 +0200 Subject: [PATCH] HSEARCH-3776 Remove unnecessary hacks to support multiple data types (integer and boolean) in IndexWriterSetting --- .../backend/lucene/logging/impl/Log.java | 9 + .../logging/impl/LuceneLogCategories.java | 3 +- .../writer/impl/IndexWriterSetting.java | 179 ------------------ .../writer/impl/IndexWriterSettingValue.java | 67 +++++++ .../writer/impl/IndexWriterSettings.java | 143 ++++++++++++++ .../writer/impl/LuceneIndexingParameters.java | 80 +------- 6 files changed, 230 insertions(+), 251 deletions(-) delete mode 100644 backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSetting.java create mode 100644 backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSettingValue.java create mode 100644 backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSettings.java diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/Log.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/Log.java index 1e61bc12d41..2c89b9c3058 100644 --- a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/Log.java +++ b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/Log.java @@ -660,4 +660,13 @@ SearchException uncommittedOperationsBecauseOfFailure(String causeMessage, value = "Field '%1$s' is not contained in a nested object." + " Aggregation filters are only available if the field to aggregate on is contained in a nested object.") SearchException cannotFilterAggregationOnRootDocumentField(String absoluteFieldPath, @Param EventContext context); + + @Message(id = ID_OFFSET_2 + 123, + value = "IndexWriter setting '%1$s' cannot be set to '%2$s': %3$s") + SearchException illegalIndexWriterSetting(String settingName, Object settingValue, String message, @Cause Exception e); + + @Message(id = ID_OFFSET_2 + 124, + value = "Merge policy setting '%1$s' cannot be set to '%2$s': %3$s") + SearchException illegalMergePolicySetting(String settingName, Object settingValue, String message, @Cause Exception e); + } diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/LuceneLogCategories.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/LuceneLogCategories.java index aacf353d7ca..5272d49ba75 100644 --- a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/LuceneLogCategories.java +++ b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/LuceneLogCategories.java @@ -6,6 +6,7 @@ */ package org.hibernate.search.backend.lucene.logging.impl; +import org.hibernate.search.backend.lucene.lowlevel.writer.impl.IndexWriterSettings; import org.hibernate.search.util.common.logging.impl.LogCategory; import org.hibernate.search.util.common.logging.impl.LoggerFactory; @@ -25,8 +26,6 @@ private LuceneLogCategories() { * To enable the logger, the category needs to be enabled at TRACE level and configuration * property {@code org.hibernate.search.backend.configuration.impl.IndexWriterSetting#INFOSTREAM} * needs to be enabled on the index. - * - * @see org.hibernate.search.backend.lucene.lowlevel.writer.impl.IndexWriterSetting#INFOSTREAM */ public static final LogCategory INFOSTREAM_LOGGER_CATEGORY = new LogCategory( "org.hibernate.search.backend.lucene.infostream" ); diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSetting.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSetting.java deleted file mode 100644 index a4097344c7f..00000000000 --- a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSetting.java +++ /dev/null @@ -1,179 +0,0 @@ -/* - * Hibernate Search, full-text search for your domain model - * - * License: GNU Lesser General Public License (LGPL), version 2.1 or later - * See the lgpl.txt file in the root directory or . - */ -package org.hibernate.search.backend.lucene.lowlevel.writer.impl; - -import org.hibernate.search.engine.cfg.spi.ConvertUtils; -import org.hibernate.search.util.common.SearchException; - -import org.apache.lucene.index.IndexWriterConfig; -import org.apache.lucene.index.LogByteSizeMergePolicy; - -/** - * Represents possible options to be applied to an - * {@code org.apache.lucene.index.IndexWriter}. - * - * @author Sanne Grinovero - */ -public enum IndexWriterSetting { - - /** - * @see IndexWriterConfig#setMaxBufferedDocs(int) - */ - MAX_BUFFERED_DOCS( "max_buffered_docs" ) { - @Override - public void applySetting(IndexWriterConfig writerConfig, int value) { - writerConfig.setMaxBufferedDocs( value ); - } - }, - /** - * @see LogByteSizeMergePolicy#setMaxMergeDocs(int) - */ - MAX_MERGE_DOCS( "max_merge_docs" ) { - @Override - public void applySetting(LogByteSizeMergePolicy logByteSizeMergePolicy, int value) { - logByteSizeMergePolicy.setMaxMergeDocs( value ); - } - }, - /** - * @see LogByteSizeMergePolicy#setMergeFactor(int) - */ - MERGE_FACTOR( "merge_factor" ) { - @Override - public void applySetting(LogByteSizeMergePolicy logByteSizeMergePolicy, int value) { - logByteSizeMergePolicy.setMergeFactor( value ); - } - }, - /** - * @see LogByteSizeMergePolicy#setMinMergeMB(double) - */ - MERGE_MIN_SIZE( "merge_min_size" ) { - @Override - public void applySetting(LogByteSizeMergePolicy logByteSizeMergePolicy, int value) { - logByteSizeMergePolicy.setMinMergeMB( value ); - } - }, - /** - * @see LogByteSizeMergePolicy#setMaxMergeMB(double) - */ - MERGE_MAX_SIZE( "merge_max_size" ) { - @Override - public void applySetting(LogByteSizeMergePolicy logByteSizeMergePolicy, int value) { - logByteSizeMergePolicy.setMaxMergeMB( value ); - } - }, - /** - * @see LogByteSizeMergePolicy#setMaxMergeMBForForcedMerge(double) - */ - MERGE_MAX_OPTIMIZE_SIZE( "merge_max_optimize_size" ) { - @Override - public void applySetting(LogByteSizeMergePolicy logByteSizeMergePolicy, int value) { - logByteSizeMergePolicy.setMaxMergeMBForForcedMerge( value ); - } - }, - /** - * @see LogByteSizeMergePolicy#setCalibrateSizeByDeletes(boolean) - */ - MERGE_CALIBRATE_BY_DELETES( "merge_calibrate_by_deletes" ) { - @Override - public Integer parseVal(String value) { - return MERGE_CALIBRATE_BY_DELETES.parseBoolean( value ); - } - @Override - public void applySetting(LogByteSizeMergePolicy logByteSizeMergePolicy, int value) { - boolean calibrateByDeletes = intToBoolean( value ); - logByteSizeMergePolicy.setCalibrateSizeByDeletes( calibrateByDeletes ); - } - }, - /** - * @see IndexWriterConfig#setRAMBufferSizeMB(double) - */ - RAM_BUFFER_SIZE( "ram_buffer_size" ) { - @Override - public void applySetting(IndexWriterConfig writerConfig, int value) { - writerConfig.setRAMBufferSizeMB( value ); - } - }, - /** - * @see IndexWriterConfig#setInfoStream(org.apache.lucene.util.InfoStream) - */ - INFOSTREAM( "infostream" ) { - @Override - public Integer parseVal(String value) { - return INFOSTREAM.parseBoolean( value ); - } - @Override - public void applySetting(IndexWriterConfig writerConfig, int value) { - boolean enableInfoStream = intToBoolean( value ); - if ( enableInfoStream ) { - writerConfig.setInfoStream( new LoggerInfoStream() ); - } - } - }; - - private static final Integer TRUE = 1; - private static final Integer FALSE = 0; - - private final String cfgKey; - - IndexWriterSetting(String configurationKey) { - this.cfgKey = configurationKey; - } - - /** - * @param writerConfig the {@link IndexWriterConfig} - * @param value the value for the configuration - * @throws IllegalArgumentException when user selects an invalid value; should be wrapped. - */ - public void applySetting(IndexWriterConfig writerConfig, int value) { - // nothing to do unless overriden - } - /** - * @param logByteSizeMergePolicy the {@link LogByteSizeMergePolicy} - * @param value the value for the configuration - * @throws IllegalArgumentException when user selects an invalid value; should be wrapped. - */ - public void applySetting(LogByteSizeMergePolicy logByteSizeMergePolicy, int value) { - // nothing to do unless overriden - } - - /** - * @return The key used in configuration files to select an option. - */ - public String getKey() { - return cfgKey; - } - - /** - * Specific parameters may override to provide additional keywords support. - * - * @param value the string value as in configuration file - * @return the integer value going to be set as parameter - * @throws SearchException for unrecognized values - */ - public Integer parseVal(String value) { - try { - return ConvertUtils.convertInteger( value ); - } - catch (RuntimeException e) { - throw new SearchException( "Invalid value for " + cfgKey + ": " + value, e ); - } - } - - private Integer parseBoolean(String value) { - try { - boolean v = ConvertUtils.convertBoolean( value ); - return v ? TRUE : FALSE; - } - catch (RuntimeException e) { - throw new SearchException( "Invalid value for " + cfgKey + ": " + value, e ); - } - } - - private static boolean intToBoolean(int value) { - return value == TRUE; - } -} diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSettingValue.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSettingValue.java new file mode 100644 index 00000000000..278b9248af4 --- /dev/null +++ b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSettingValue.java @@ -0,0 +1,67 @@ +/* + * Hibernate Search, full-text search for your domain model + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.search.backend.lucene.lowlevel.writer.impl; + +import java.lang.invoke.MethodHandles; +import java.util.function.BiConsumer; + +import org.hibernate.search.backend.lucene.logging.impl.Log; +import org.hibernate.search.util.common.SearchException; +import org.hibernate.search.util.common.logging.impl.LoggerFactory; + +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.LogByteSizeMergePolicy; + +class IndexWriterSettingValue { + + private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() ); + + private final String settingName; + private final T value; + private final BiConsumer writerSettingApplier; + private final BiConsumer mergePolicySettingApplier; + + IndexWriterSettingValue(String settingName, T value, + BiConsumer writerSettingApplier, + BiConsumer mergePolicySettingApplier) { + this.settingName = settingName; + this.value = value; + this.writerSettingApplier = writerSettingApplier; + this.mergePolicySettingApplier = mergePolicySettingApplier; + } + + @Override + public String toString() { + return "<" + settingName + "=" + value + ">"; + } + + /** + * @param writerConfig the {@link IndexWriterConfig} + * @throws SearchException when user selects an invalid value. + */ + public void applySetting(IndexWriterConfig writerConfig) { + try { + writerSettingApplier.accept( writerConfig, value ); + } + catch (RuntimeException e) { + throw log.illegalIndexWriterSetting( settingName, value, e.getMessage(), e ); + } + } + + /** + * @param logByteSizeMergePolicy the {@link LogByteSizeMergePolicy} + * @throws SearchException when user selects an invalid value. + */ + public void applySetting(LogByteSizeMergePolicy logByteSizeMergePolicy) { + try { + mergePolicySettingApplier.accept( logByteSizeMergePolicy, value ); + } + catch (RuntimeException e) { + throw log.illegalMergePolicySetting( settingName, value, e.getMessage(), e ); + } + } +} diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSettings.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSettings.java new file mode 100644 index 00000000000..4288fbfa9f7 --- /dev/null +++ b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/IndexWriterSettings.java @@ -0,0 +1,143 @@ +/* + * Hibernate Search, full-text search for your domain model + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.search.backend.lucene.lowlevel.writer.impl; + +import java.io.Serializable; +import java.lang.invoke.MethodHandles; +import java.util.ArrayList; +import java.util.List; +import java.util.function.BiConsumer; +import java.util.function.Function; + +import org.hibernate.search.backend.lucene.logging.impl.Log; +import org.hibernate.search.engine.cfg.spi.ConfigurationProperty; +import org.hibernate.search.engine.cfg.spi.ConfigurationPropertySource; +import org.hibernate.search.engine.cfg.spi.OptionalConfigurationProperty; +import org.hibernate.search.util.common.logging.impl.LoggerFactory; + +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.LogByteSizeMergePolicy; + +/** + * Represents possible options to be applied to an + * {@code org.apache.lucene.index.IndexWriter}. + * + * @author Sanne Grinovero + */ +public final class IndexWriterSettings implements Serializable { + + private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() ); + + private IndexWriterSettings() { + } + + public static List> extractAll(ConfigurationPropertySource propertySource) { + List> result = new ArrayList<>(); + for ( Extractor extractor : EXTRACTORS ) { + IndexWriterSettingValue extracted = extractor.extractOrNull( propertySource ); + if ( extracted != null ) { + result.add( extracted ); + } + } + return result; + } + + private static final List> EXTRACTORS = new ArrayList<>(); + + static { + registerIntegerWriterSetting( "max_buffered_docs", IndexWriterConfig::setMaxBufferedDocs ); + registerIntegerWriterSetting( "ram_buffer_size", IndexWriterConfig::setRAMBufferSizeMB ); + + registerSetting( Extractor.fromBoolean( "infostream", enabled -> enabled ? new LoggerInfoStream() : null, + IndexWriterConfig::setInfoStream, (logByteSizeMergePolicy, integer) -> { } ) ); + + registerIntegerMergePolicySetting( "max_merge_docs", LogByteSizeMergePolicy::setMaxMergeDocs ); + registerIntegerMergePolicySetting( "merge_factor", LogByteSizeMergePolicy::setMergeFactor ); + registerIntegerMergePolicySetting( "merge_min_size", LogByteSizeMergePolicy::setMinMergeMB ); + registerIntegerMergePolicySetting( "merge_max_size", LogByteSizeMergePolicy::setMaxMergeMB ); + registerIntegerMergePolicySetting( "merge_max_optimize_size", LogByteSizeMergePolicy::setMaxMergeMBForForcedMerge ); + registerBooleanMergePolicySetting( "merge_calibrate_by_deletes", LogByteSizeMergePolicy::setCalibrateSizeByDeletes ); + } + + private static void registerIntegerWriterSetting(String propertyKey, + BiConsumer writerSettingApplier) { + EXTRACTORS.add( Extractor.fromInteger( propertyKey, passThrough -> passThrough, + writerSettingApplier, (logByteSizeMergePolicy, integer) -> { } ) ); + } + + private static void registerIntegerMergePolicySetting(String propertyKey, + BiConsumer mergePolicySettingApplier) { + EXTRACTORS.add( Extractor.fromInteger( propertyKey, passThrough -> passThrough, + (writer, integer) -> { }, mergePolicySettingApplier ) ); + } + + private static void registerBooleanMergePolicySetting(String propertyKey, + BiConsumer mergePolicySettingApplier) { + EXTRACTORS.add( Extractor.fromBoolean( propertyKey, passThrough -> passThrough, + (writer, integer) -> { }, mergePolicySettingApplier ) ); + } + + private static void registerSetting(Extractor extractor) { + EXTRACTORS.add( extractor ); + } + + private static final class Extractor { + + static Extractor fromInteger(String propertyKey, + Function processor, + BiConsumer writerSettingApplier, + BiConsumer mergePolicySettingApplier) { + OptionalConfigurationProperty property = ConfigurationProperty.forKey( propertyKey ) + .asInteger().build(); + return new Extractor<>( propertyKey, property, processor, writerSettingApplier, mergePolicySettingApplier ); + } + + static Extractor fromBoolean(String propertyKey, + Function processor, + BiConsumer writerSettingApplier, + BiConsumer mergePolicySettingApplier) { + OptionalConfigurationProperty property = ConfigurationProperty.forKey( propertyKey ) + .asBoolean().build(); + return new Extractor<>( propertyKey, property, processor, writerSettingApplier, mergePolicySettingApplier ); + } + + private final String settingName; + private final OptionalConfigurationProperty property; + private final Function processor; + private final BiConsumer writerSettingApplier; + private final BiConsumer mergePolicySettingApplier; + + private Extractor(String settingName, OptionalConfigurationProperty property, Function processor, + BiConsumer writerSettingApplier, + BiConsumer mergePolicySettingApplier) { + this.settingName = settingName; + this.property = property; + this.processor = processor; + this.writerSettingApplier = writerSettingApplier; + this.mergePolicySettingApplier = mergePolicySettingApplier; + } + + IndexWriterSettingValue extractOrNull(ConfigurationPropertySource source) { + return property.getAndMap( source, this::createValueOrNull ).orElse( null ); + } + + private IndexWriterSettingValue createValueOrNull(T value) { + if ( value == null ) { + return null; + } + if ( log.isDebugEnabled() ) { + //TODO add DirectoryProvider name when available to log message + log.debugf( "Set index writer parameter %s to value : %s", settingName, value ); + } + R processedValue = processor.apply( value ); + return new IndexWriterSettingValue<>( settingName, processedValue, + writerSettingApplier, mergePolicySettingApplier ); + } + + } + +} diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/LuceneIndexingParameters.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/LuceneIndexingParameters.java index c261ea43b16..0076bce91ef 100644 --- a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/LuceneIndexingParameters.java +++ b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/writer/impl/LuceneIndexingParameters.java @@ -7,14 +7,9 @@ package org.hibernate.search.backend.lucene.lowlevel.writer.impl; import java.io.Serializable; -import java.lang.invoke.MethodHandles; -import java.util.EnumMap; -import java.util.Map; +import java.util.List; -import org.hibernate.search.backend.lucene.logging.impl.Log; import org.hibernate.search.engine.cfg.spi.ConfigurationPropertySource; -import org.hibernate.search.util.common.SearchException; -import org.hibernate.search.util.common.logging.impl.LoggerFactory; import org.apache.lucene.index.IndexWriterConfig; import org.apache.lucene.index.LogByteSizeMergePolicy; @@ -30,10 +25,6 @@ */ public class LuceneIndexingParameters { - private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() ); - - // value keyword - public static final String EXPLICIT_DEFAULT_VALUE = "default"; // property path keywords public static final String PROP_GROUP = "indexwriter"; @@ -61,21 +52,10 @@ public static class ParameterSet implements Serializable { private static final long serialVersionUID = -6121723702279869524L; - final Map parameters = new EnumMap( IndexWriterSetting.class ); + private final List> values; public ParameterSet(ConfigurationPropertySource prop) { - //don't iterate on property entries as we know all the keys: - for ( IndexWriterSetting t : IndexWriterSetting.values() ) { - String key = t.getKey(); - Object value = prop.get( key ).orElse( null ); - if ( value instanceof String && !EXPLICIT_DEFAULT_VALUE.equalsIgnoreCase( (String) value ) ) { - if ( log.isDebugEnabled() ) { - //TODO add DirectoryProvider name when available to log message - log.debugf( "Set index writer parameter %s to value : %s", key, value ); - } - parameters.put( t, t.parseVal( (String) value ) ); - } - } + values = IndexWriterSettings.extractAll( prop ); } /** @@ -85,17 +65,8 @@ public ParameterSet(ConfigurationPropertySource prop) { * @param writerConfig the IndexWriter configuration whereto the parameters will be applied. */ public void applyToWriter(IndexWriterConfig writerConfig) { - for ( Map.Entry entry : parameters.entrySet() ) { - try { - entry.getKey().applySetting( writerConfig, entry.getValue() ); - } - catch (IllegalArgumentException e) { - //TODO if DirectoryProvider had getDirectoryName() exceptions could tell better - throw new SearchException( - "Illegal IndexWriter setting " - + entry.getKey().getKey() + " " + e.getMessage(), e - ); - } + for ( IndexWriterSettingValue value : values ) { + value.applySetting( writerConfig ); } } @@ -105,40 +76,17 @@ public void applyToWriter(IndexWriterConfig writerConfig) { */ public LogByteSizeMergePolicy getNewMergePolicy() { LogByteSizeMergePolicy logByteSizeMergePolicy = new LogByteSizeMergePolicy(); - for ( Map.Entry entry : parameters.entrySet() ) { - try { - entry.getKey().applySetting( logByteSizeMergePolicy, entry.getValue() ); - } - catch (IllegalArgumentException e) { - //TODO if DirectoryProvider had getDirectoryName() exceptions could tell better - throw new SearchException( - "Illegal IndexWriter setting " - + entry.getKey().getKey() + " " + e.getMessage(), e - ); - } + for ( IndexWriterSettingValue value : values ) { + value.applySetting( logByteSizeMergePolicy ); } return logByteSizeMergePolicy; } - public Integer getCurrentValueFor(IndexWriterSetting ws) { - return parameters.get( ws ); - } - - public void setCurrentValueFor(IndexWriterSetting ws, Integer newValue) { - if ( newValue == null ) { - parameters.remove( ws ); - } - else { - parameters.put( ws, newValue ); - } - } - @Override public int hashCode() { final int prime = 31; int result = 1; - result = prime * result - + ( ( parameters == null ) ? 0 : parameters.hashCode() ); + result = prime * result + values.hashCode(); return result; } @@ -154,22 +102,14 @@ public boolean equals(Object obj) { return false; } final ParameterSet other = (ParameterSet) obj; - if ( parameters == null ) { - if ( other.parameters != null ) { - return false; - } - } - else if ( !parameters.equals( other.parameters ) ) { - return false; - } - return true; + return values.equals( other.values ); } @Override public String toString() { final StringBuilder sb = new StringBuilder(); sb.append( "ParameterSet" ); - sb.append( "{parameters=" ).append( parameters ); + sb.append( "{values=" ).append( values ); sb.append( '}' ); return sb.toString(); }