diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchBackendFactory.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchBackendFactory.java index a26b46fddcc..26ec8af4877 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchBackendFactory.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchBackendFactory.java @@ -6,7 +6,6 @@ */ package org.hibernate.search.backend.elasticsearch.impl; -import java.lang.invoke.MethodHandles; import java.util.Locale; import java.util.Optional; @@ -18,7 +17,6 @@ import org.hibernate.search.backend.elasticsearch.dialect.model.impl.ElasticsearchModelDialect; import org.hibernate.search.backend.elasticsearch.gson.spi.GsonProvider; import org.hibernate.search.backend.elasticsearch.index.layout.IndexLayoutStrategy; -import org.hibernate.search.backend.elasticsearch.logging.impl.Log; import org.hibernate.search.backend.elasticsearch.mapping.TypeNameMappingStrategyName; import org.hibernate.search.backend.elasticsearch.mapping.impl.DiscriminatorTypeNameMapping; import org.hibernate.search.backend.elasticsearch.mapping.impl.IndexNameTypeNameMapping; @@ -34,13 +32,11 @@ import org.hibernate.search.engine.backend.spi.BackendImplementor; 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.engine.environment.bean.BeanHolder; import org.hibernate.search.engine.environment.bean.BeanReference; import org.hibernate.search.engine.environment.bean.BeanResolver; import org.hibernate.search.util.common.AssertionFailure; import org.hibernate.search.util.common.impl.SuppressingCloser; -import org.hibernate.search.util.common.logging.impl.LoggerFactory; import org.hibernate.search.util.common.reporting.EventContext; import com.google.gson.Gson; @@ -49,19 +45,6 @@ public class ElasticsearchBackendFactory implements BackendFactory { - private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() ); - - private static final OptionalConfigurationProperty VERSION = - ConfigurationProperty.forKey( ElasticsearchBackendSettings.VERSION ) - .as( ElasticsearchVersion.class, ElasticsearchVersion::of ) - .build(); - - private static final ConfigurationProperty VERSION_CHECK_ENABLED = - ConfigurationProperty.forKey( ElasticsearchBackendSettings.VERSION_CHECK_ENABLED ) - .asBoolean() - .withDefault( ElasticsearchBackendSettings.Defaults.VERSION_CHECK_ENABLED ) - .build(); - private static final ConfigurationProperty MULTI_TENANCY_STRATEGY = ConfigurationProperty.forKey( ElasticsearchBackendSettings.MULTI_TENANCY_STRATEGY ) .as( MultiTenancyStrategyName.class, MultiTenancyStrategyName::of ) @@ -103,8 +86,7 @@ public BackendImplementor create(EventContext eventContext, BackendBuildContext */ GsonProvider defaultGsonProvider = GsonProvider.create( GsonBuilder::new, logPrettyPrinting ); - Optional configuredVersion = VERSION.get( propertySource ); - boolean versionCheckEnabled = getVersionCheckEnabled( propertySource ); + Optional configuredVersion = ElasticsearchLinkImpl.VERSION.get( propertySource ); BeanResolver beanResolver = buildContext.beanResolver(); BeanHolder clientFactoryHolder = null; @@ -119,7 +101,7 @@ public BackendImplementor create(EventContext eventContext, BackendBuildContext ElasticsearchDialectFactory dialectFactory = new ElasticsearchDialectFactory(); link = new ElasticsearchLinkImpl( clientFactoryHolder, threads, defaultGsonProvider, logPrettyPrinting, - dialectFactory, configuredVersion, versionCheckEnabled + dialectFactory, configuredVersion ); ElasticsearchModelDialect dialect; @@ -165,20 +147,6 @@ public BackendImplementor create(EventContext eventContext, BackendBuildContext } } - private boolean getVersionCheckEnabled(ConfigurationPropertySource propertySource) { - boolean versionCheckEnabled = VERSION_CHECK_ENABLED.get( propertySource ); - if ( !versionCheckEnabled ) { - VERSION.getAndTransform( propertySource, optionalValue -> { - if ( !optionalValue.isPresent() || optionalValue.isPresent() && !optionalValue.get().minor().isPresent() ) { - throw log.impreciseElasticsearchVersionWhenNoVersionCheck( - VERSION_CHECK_ENABLED.resolveOrRaw( propertySource ) ); - } - return optionalValue; - } ); - } - return versionCheckEnabled; - } - private MultiTenancyStrategy getMultiTenancyStrategy(ConfigurationPropertySource propertySource) { MultiTenancyStrategyName multiTenancyStrategyName = MULTI_TENANCY_STRATEGY.get( propertySource ); diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchLinkImpl.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchLinkImpl.java index 73ebe8060b4..5f7ff0c36c1 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchLinkImpl.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchLinkImpl.java @@ -25,6 +25,7 @@ import org.hibernate.search.backend.elasticsearch.work.builder.factory.impl.ElasticsearchWorkBuilderFactory; 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.engine.environment.bean.BeanHolder; import org.hibernate.search.engine.environment.bean.BeanResolver; import org.hibernate.search.util.common.AssertionFailure; @@ -37,6 +38,17 @@ class ElasticsearchLinkImpl implements ElasticsearchLink { private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() ); + static final OptionalConfigurationProperty VERSION = + ConfigurationProperty.forKey( ElasticsearchBackendSettings.VERSION ) + .as( ElasticsearchVersion.class, ElasticsearchVersion::of ) + .build(); + + private static final ConfigurationProperty VERSION_CHECK_ENABLED = + ConfigurationProperty.forKey( ElasticsearchBackendSettings.VERSION_CHECK_ENABLED ) + .asBoolean() + .withDefault( ElasticsearchBackendSettings.Defaults.VERSION_CHECK_ENABLED ) + .build(); + private static final ConfigurationProperty SCROLL_TIMEOUT = ConfigurationProperty.forKey( ElasticsearchBackendSettings.SCROLL_TIMEOUT ) .asInteger() @@ -48,8 +60,7 @@ class ElasticsearchLinkImpl implements ElasticsearchLink { private final GsonProvider defaultGsonProvider; private final boolean logPrettyPrinting; private final ElasticsearchDialectFactory dialectFactory; - private final Optional configuredVersionOptional; - private final boolean versionCheckEnabled; + private final Optional configuredVersionOnBackendCreationOptional; private ElasticsearchClientImplementor clientImplementor; private ElasticsearchVersion elasticsearchVersion; @@ -63,15 +74,13 @@ class ElasticsearchLinkImpl implements ElasticsearchLink { ElasticsearchLinkImpl(BeanHolder clientFactoryHolder, BackendThreads threads, GsonProvider defaultGsonProvider, boolean logPrettyPrinting, ElasticsearchDialectFactory dialectFactory, - Optional configuredVersionOptional, - boolean versionCheckEnabled) { + Optional configuredVersionOnBackendCreationOptional) { this.clientFactoryHolder = clientFactoryHolder; this.threads = threads; this.defaultGsonProvider = defaultGsonProvider; this.logPrettyPrinting = logPrettyPrinting; this.dialectFactory = dialectFactory; - this.configuredVersionOptional = configuredVersionOptional; - this.versionCheckEnabled = versionCheckEnabled; + this.configuredVersionOnBackendCreationOptional = configuredVersionOnBackendCreationOptional; } @Override @@ -129,6 +138,32 @@ void onStart(BeanResolver beanResolver, ConfigurationPropertySource propertySour ); clientFactoryHolder.close(); // We won't need it anymore + boolean versionCheckEnabled = VERSION_CHECK_ENABLED.get( propertySource ); + Optional configuredVersionOptional = VERSION.getAndTransform( propertySource, configuredVersionOnStartOptional -> { + Optional resultOptional; + if ( configuredVersionOnStartOptional.isPresent() ) { + // Allow overriding the version on start, + // but expect it to match the version configured on backend creation (if any) + if ( configuredVersionOnBackendCreationOptional.isPresent() + && !configuredVersionOnBackendCreationOptional.get() + .matches( configuredVersionOnStartOptional.get() ) ) { + throw log.incompatibleElasticsearchVersionOnStart( + configuredVersionOnBackendCreationOptional.get(), + configuredVersionOnStartOptional.get() ); + } + resultOptional = configuredVersionOnStartOptional; + } + else { + // Default to the version configured when the backend was created + resultOptional = configuredVersionOnBackendCreationOptional; + } + if ( !versionCheckEnabled && !resultOptional.isPresent() || !resultOptional.get().minor().isPresent() ) { + throw log.impreciseElasticsearchVersionWhenNoVersionCheck( + VERSION_CHECK_ENABLED.resolveOrRaw( propertySource ) ); + } + return resultOptional; + } ); + if ( versionCheckEnabled ) { elasticsearchVersion = ElasticsearchClientUtils.getElasticsearchVersion( clientImplementor ); if ( configuredVersionOptional.isPresent() ) { @@ -139,7 +174,8 @@ void onStart(BeanResolver beanResolver, ConfigurationPropertySource propertySour } } else { - configuredVersionOptional.ifPresent( version -> elasticsearchVersion = version ); + // In this case know the optional is non-empty, see above. + elasticsearchVersion = configuredVersionOptional.get(); } ElasticsearchProtocolDialect protocolDialect = dialectFactory.createProtocolDialect( elasticsearchVersion ); diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java index dd1a14cb3ba..5016c200e09 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java @@ -704,4 +704,13 @@ SearchException cannotUseQueryElementForObjectFieldBecauseCreationException(Stri @Message(id = ID_OFFSET + 130, value = "Invalid target hosts configuration: the list of URIs must not be empty.") SearchException emptyListOfUris(); + + @Message(id = ID_OFFSET + 141, + value = "Incompatible Elasticsearch version:" + + " version '%2$s' does not match version '%1$s' that was provided " + + " when the backend was created." + + " You can provide a more precise version on startup," + + " but you cannot override the version that was provided when the backend was created.") + SearchException incompatibleElasticsearchVersionOnStart(ElasticsearchVersion versionOnCreation, + ElasticsearchVersion versionOnStart); } diff --git a/integrationtest/backend/elasticsearch/src/test/java/org/hibernate/search/integrationtest/backend/elasticsearch/bootstrap/ElasticsearchBootstrapIT.java b/integrationtest/backend/elasticsearch/src/test/java/org/hibernate/search/integrationtest/backend/elasticsearch/bootstrap/ElasticsearchBootstrapIT.java index cd4c964a8b1..1dfd2b7eba5 100644 --- a/integrationtest/backend/elasticsearch/src/test/java/org/hibernate/search/integrationtest/backend/elasticsearch/bootstrap/ElasticsearchBootstrapIT.java +++ b/integrationtest/backend/elasticsearch/src/test/java/org/hibernate/search/integrationtest/backend/elasticsearch/bootstrap/ElasticsearchBootstrapIT.java @@ -10,10 +10,15 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.hibernate.search.util.impl.integrationtest.common.assertion.SearchResultAssert.assertThatQuery; +import java.util.HashMap; +import java.util.Map; + import org.hibernate.search.backend.elasticsearch.ElasticsearchVersion; import org.hibernate.search.backend.elasticsearch.cfg.ElasticsearchBackendSettings; import org.hibernate.search.backend.elasticsearch.cfg.spi.ElasticsearchBackendSpiSettings; import org.hibernate.search.backend.elasticsearch.client.spi.ElasticsearchRequest; +import org.hibernate.search.engine.cfg.BackendSettings; +import org.hibernate.search.engine.cfg.spi.ConfigurationPropertySource; import org.hibernate.search.integrationtest.backend.elasticsearch.testsupport.util.ElasticsearchClientSpy; import org.hibernate.search.integrationtest.backend.elasticsearch.testsupport.util.ElasticsearchRequestAssertionMode; import org.hibernate.search.integrationtest.backend.tck.testsupport.util.rule.SearchSetupHelper; @@ -125,7 +130,7 @@ public void noVersionCheck_incompleteVersion() { ) .withSchemaManagement( StubMappingSchemaManagementStrategy.DROP_ON_SHUTDOWN_ONLY ) .withIndex( StubMappedIndex.withoutFields() ) - .setupFirstPhaseOnly(), + .setup(), "NO version check with partial version number" ) .isInstanceOf( SearchException.class ) @@ -151,10 +156,43 @@ public void noVersionCheck_completeVersion() { SearchSetupHelper.PartialSetup partialSetup = setupHelper.start() .withBackendProperty( - ElasticsearchBackendSettings.VERSION_CHECK_ENABLED, false + ElasticsearchBackendSettings.VERSION, versionWithMajorAndMinorOnly ) .withBackendProperty( - ElasticsearchBackendSettings.VERSION, versionWithMajorAndMinorOnly + ElasticsearchBackendSpiSettings.CLIENT_FACTORY, + elasticsearchClientSpy.factoryReference() + ) + .withSchemaManagement( StubMappingSchemaManagementStrategy.DROP_ON_SHUTDOWN_ONLY ) + .withIndex( index ) + .setupFirstPhaseOnly(); + // We do not expect the client to be created in the first phase + assertThat( elasticsearchClientSpy.getCreatedClientCount() ).isZero(); + + Map runtimeProperties = new HashMap<>(); + runtimeProperties.put( BackendSettings.backendKey( ElasticsearchBackendSettings.VERSION_CHECK_ENABLED ), false ); + partialSetup.doSecondPhase( ConfigurationPropertySource.fromMap( runtimeProperties ) ); + // We do not expect any request, since the version check is disabled + assertThat( elasticsearchClientSpy.getRequestCount() ).isZero(); + + checkBackendWorks(); + + assertThat( elasticsearchClientSpy.getRequestCount() ).isNotZero(); + } + + /** + * Check that an exception is thrown when version_check.enabled is false + * and specifying a version on backend creation, and a different one on backend start. + */ + @Test + @TestForIssue(jiraKey = "HSEARCH-4214") + public void noVersionCheck_versionOverrideOnStart_incompatibleVersion() { + ElasticsearchVersion clusterVersion = ElasticsearchVersion.of( ElasticsearchTestDialect.getClusterVersion() ); + String versionWithMajorOnly = String.valueOf( clusterVersion.major() ); + String incompatibleVersionWithMajorAndMinorOnly = "2." + clusterVersion.minor().getAsInt(); + + SearchSetupHelper.PartialSetup partialSetup = setupHelper.start() + .withBackendProperty( + ElasticsearchBackendSettings.VERSION, versionWithMajorOnly ) .withBackendProperty( ElasticsearchBackendSpiSettings.CLIENT_FACTORY, @@ -166,7 +204,56 @@ public void noVersionCheck_completeVersion() { // We do not expect the client to be created in the first phase assertThat( elasticsearchClientSpy.getCreatedClientCount() ).isZero(); - partialSetup.doSecondPhase(); + Map runtimeProperties = new HashMap<>(); + runtimeProperties.put( BackendSettings.backendKey( ElasticsearchBackendSettings.VERSION_CHECK_ENABLED ), false ); + runtimeProperties.put( BackendSettings.backendKey( ElasticsearchBackendSettings.VERSION ), incompatibleVersionWithMajorAndMinorOnly ); + assertThatThrownBy( + () -> partialSetup.doSecondPhase( ConfigurationPropertySource.fromMap( runtimeProperties ) ) ) + .isInstanceOf( SearchException.class ) + .hasMessageMatching( FailureReportUtils.buildFailureReportPattern() + .defaultBackendContext() + .failure( + "Invalid value for configuration property 'hibernate.search.backend.version': '" + + incompatibleVersionWithMajorAndMinorOnly + "'", + "Incompatible Elasticsearch version:" + + " version '" + incompatibleVersionWithMajorAndMinorOnly + + "' does not match version '" + versionWithMajorOnly + "' that was provided " + + " when the backend was created.", + "You can provide a more precise version on startup," + + " but you cannot override the version that was provided when the backend was created." ) + .build() + ); + } + + /** + * Check everything works fine when version_check.enabled is false + * and specifying a version on backend creation, and a more precise one on backend start. + */ + @Test + @TestForIssue(jiraKey = "HSEARCH-4214") + public void noVersionCheck_versionOverrideOnStart_compatibleVersion() { + ElasticsearchVersion clusterVersion = ElasticsearchVersion.of( ElasticsearchTestDialect.getClusterVersion() ); + String versionWithMajorOnly = String.valueOf( clusterVersion.major() ); + String versionWithMajorAndMinorOnly = clusterVersion.major() + "." + clusterVersion.minor().getAsInt(); + + SearchSetupHelper.PartialSetup partialSetup = setupHelper.start() + .withBackendProperty( + ElasticsearchBackendSettings.VERSION, versionWithMajorOnly + ) + .withBackendProperty( + ElasticsearchBackendSpiSettings.CLIENT_FACTORY, + elasticsearchClientSpy.factoryReference() + ) + .withSchemaManagement( StubMappingSchemaManagementStrategy.DROP_ON_SHUTDOWN_ONLY ) + .withIndex( index ) + .setupFirstPhaseOnly(); + // We do not expect the client to be created in the first phase + assertThat( elasticsearchClientSpy.getCreatedClientCount() ).isZero(); + + Map runtimeProperties = new HashMap<>(); + runtimeProperties.put( BackendSettings.backendKey( ElasticsearchBackendSettings.VERSION_CHECK_ENABLED ), false ); + runtimeProperties.put( BackendSettings.backendKey( ElasticsearchBackendSettings.VERSION ), versionWithMajorAndMinorOnly ); + partialSetup.doSecondPhase( ConfigurationPropertySource.fromMap( runtimeProperties ) ); // We do not expect any request, since the version check is disabled assertThat( elasticsearchClientSpy.getRequestCount() ).isZero(); diff --git a/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/testsupport/util/rule/SearchSetupHelper.java b/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/testsupport/util/rule/SearchSetupHelper.java index a9c1be25594..a2d5467ea68 100644 --- a/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/testsupport/util/rule/SearchSetupHelper.java +++ b/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/testsupport/util/rule/SearchSetupHelper.java @@ -20,8 +20,8 @@ import org.hibernate.search.engine.cfg.BackendSettings; import org.hibernate.search.engine.cfg.EngineSettings; import org.hibernate.search.engine.cfg.spi.AllAwareConfigurationPropertySource; -import org.hibernate.search.engine.cfg.spi.ConfigurationPropertyChecker; import org.hibernate.search.engine.cfg.spi.ConfigurationPropertySource; +import org.hibernate.search.engine.cfg.spi.ConfigurationPropertyChecker; import org.hibernate.search.engine.common.spi.SearchIntegration; import org.hibernate.search.engine.common.spi.SearchIntegrationBuilder; import org.hibernate.search.engine.common.spi.SearchIntegrationFinalizer; @@ -241,9 +241,9 @@ public PartialSetup setupFirstPhaseOnly() { SearchIntegrationPartialBuildState integrationPartialBuildState = integrationBuilder.prepareBuild(); integrationPartialBuildStates.add( integrationPartialBuildState ); - return () -> { + return overrides -> { SearchIntegrationFinalizer finalizer = - integrationPartialBuildState.finalizer( propertySource, unusedPropertyChecker ); + integrationPartialBuildState.finalizer( propertySource.withOverride( overrides ), unusedPropertyChecker ); StubMapping mapping = finalizer.finalizeMapping( mappingKey, (context, partialMapping) -> partialMapping.finalizeMapping( schemaManagementStrategy ) @@ -265,7 +265,11 @@ public SearchIntegration setup() { public interface PartialSetup { - SearchIntegration doSecondPhase(); + default SearchIntegration doSecondPhase() { + return doSecondPhase( ConfigurationPropertySource.empty() ); + } + + SearchIntegration doSecondPhase(ConfigurationPropertySource overrides); } }