HSEARCH-4214 hibernate.search.backend.version_check.enabled should be evaluated on backend startup

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.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 );
 

backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchLinkImpl.java

@@ -6,7 +6,9 @@
  */
 package org.hibernate.search.backend.elasticsearch.impl;
 
-import com.google.gson.GsonBuilder;
+import java.lang.invoke.MethodHandles;
+import java.util.Optional;
+
 import org.hibernate.search.backend.elasticsearch.ElasticsearchVersion;
 import org.hibernate.search.backend.elasticsearch.cfg.ElasticsearchBackendSettings;
 import org.hibernate.search.backend.elasticsearch.client.impl.ElasticsearchClientUtils;
@@ -23,20 +25,31 @@
 import org.hibernate.search.backend.elasticsearch.resources.impl.BackendThreads;
 import org.hibernate.search.backend.elasticsearch.search.query.impl.ElasticsearchSearchResultExtractorFactory;
 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.ConfigurationPropertySource;
+import org.hibernate.search.engine.cfg.spi.ConfigurationProperty;
+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;
 import org.hibernate.search.util.common.impl.Closer;
 import org.hibernate.search.util.common.logging.impl.LoggerFactory;
 
-import java.lang.invoke.MethodHandles;
-import java.util.Optional;
+import com.google.gson.GsonBuilder;
 
 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 +61,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 +75,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,18 +139,7 @@ void onStart(BeanResolver beanResolver, ConfigurationPropertySource propertySour
 			);
 			clientFactoryHolder.close(); // We won't need it anymore
 
-			if ( versionCheckEnabled ) {
-				elasticsearchVersion = ElasticsearchClientUtils.getElasticsearchVersion( clientImplementor );
-				if ( configuredVersionOptional.isPresent() ) {
-					ElasticsearchVersion configuredVersion = configuredVersionOptional.get();
-					if ( !configuredVersion.matches( elasticsearchVersion ) ) {
-						throw log.unexpectedElasticsearchVersion( configuredVersion, elasticsearchVersion );
-					}
-				}
-			}
-			else {
-				configuredVersionOptional.ifPresent( version -> elasticsearchVersion = version );
-			}
+			elasticsearchVersion = initVersion( propertySource );
 
 			ElasticsearchProtocolDialect protocolDialect = dialectFactory.createProtocolDialect( elasticsearchVersion );
 			gsonProvider = GsonProvider.create( GsonBuilder::new, logPrettyPrinting );
@@ -166,4 +165,50 @@ private void checkStarted() {
 			);
 		}
 	}
+
+	private ElasticsearchVersion initVersion(ConfigurationPropertySource propertySource) {
+		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 versionFromCluster =
+					ElasticsearchClientUtils.getElasticsearchVersion( clientImplementor );
+			if ( configuredVersionOptional.isPresent() ) {
+				ElasticsearchVersion configuredVersion = configuredVersionOptional.get();
+				if ( !configuredVersion.matches( versionFromCluster ) ) {
+					throw log.unexpectedElasticsearchVersion( configuredVersion, versionFromCluster );
+				}
+			}
+			return versionFromCluster;
+		}
+		else {
+			// In this case we know the optional is non-empty, see above.
+			return configuredVersionOptional.get();
+		}
+	}
 }

backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java

@@ -740,4 +740,13 @@ SearchException customIndexSettingsJsonSyntaxErrors(String filePath, @Cause Exce
 			" hits. Refer to Elasticsearch's 'max_result_window_size' setting for more information." )
 	void defaultedLimitedHits(Integer defaultLimit, long hitCount);
 
+	@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);
+
 }