HSEARCH-4214 Evaluate hibernate.search.backend.version_check.enabled …

…(and version) on backend startup
hibernate · Apr 20, 2021 · 9f2e80d · 9f2e80d
1 parent 74ef63c
commit 9f2e80d
Show file tree

Hide file tree

Showing 5 changed files with 153 additions and 49 deletions.
diff --git a/...ain/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchBackendFactory.java b/...ain/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<ElasticsearchVersion> VERSION =
-			ConfigurationProperty.forKey( ElasticsearchBackendSettings.VERSION )
-					.as( ElasticsearchVersion.class, ElasticsearchVersion::of )
-					.build();
-
-	private static final ConfigurationProperty<Boolean> VERSION_CHECK_ENABLED =
-			ConfigurationProperty.forKey( ElasticsearchBackendSettings.VERSION_CHECK_ENABLED )
-					.asBoolean()
-					.withDefault( ElasticsearchBackendSettings.Defaults.VERSION_CHECK_ENABLED )
-					.build();
-
 	private static final ConfigurationProperty<MultiTenancyStrategyName> 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<ElasticsearchVersion> configuredVersion = VERSION.get( propertySource );
-		boolean versionCheckEnabled = getVersionCheckEnabled( propertySource );
+		Optional<ElasticsearchVersion> configuredVersion = ElasticsearchLinkImpl.VERSION.get( propertySource );
 
 		BeanResolver beanResolver = buildContext.beanResolver();
 		BeanHolder<? extends ElasticsearchClientFactory> 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/.../src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchLinkImpl.java b/.../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<ElasticsearchVersion> VERSION =
+			ConfigurationProperty.forKey( ElasticsearchBackendSettings.VERSION )
+					.as( ElasticsearchVersion.class, ElasticsearchVersion::of )
+					.build();
+
+	private static final ConfigurationProperty<Boolean> VERSION_CHECK_ENABLED =
+			ConfigurationProperty.forKey( ElasticsearchBackendSettings.VERSION_CHECK_ENABLED )
+					.asBoolean()
+					.withDefault( ElasticsearchBackendSettings.Defaults.VERSION_CHECK_ENABLED )
+					.build();
+
 	private static final ConfigurationProperty<Integer> 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<ElasticsearchVersion> configuredVersionOptional;
-	private final boolean versionCheckEnabled;
+	private final Optional<ElasticsearchVersion> configuredVersionOnBackendCreationOptional;
 
 	private ElasticsearchClientImplementor clientImplementor;
 	private ElasticsearchVersion elasticsearchVersion;
@@ -63,15 +74,13 @@ class ElasticsearchLinkImpl implements ElasticsearchLink {
 	ElasticsearchLinkImpl(BeanHolder<? extends ElasticsearchClientFactory> clientFactoryHolder,
 			BackendThreads threads, GsonProvider defaultGsonProvider, boolean logPrettyPrinting,
 			ElasticsearchDialectFactory dialectFactory,
-			Optional<ElasticsearchVersion> configuredVersionOptional,
-			boolean versionCheckEnabled) {
+			Optional<ElasticsearchVersion> 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<ElasticsearchVersion> configuredVersionOptional = VERSION.getAndTransform( propertySource, configuredVersionOnStartOptional -> {
+				Optional<ElasticsearchVersion> 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/...sticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java b/...sticsearch/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/...nate/search/integrationtest/backend/elasticsearch/bootstrap/ElasticsearchBootstrapIT.java b/...nate/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<String, Object> 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<String, Object> 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<String, Object> 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/...hibernate/search/integrationtest/backend/tck/testsupport/util/rule/SearchSetupHelper.java b/...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);
 
 	}
 }