DATACOUCH-550 - Refactored the usage of CouchbasePersistentEntityIndexCreator. (#298)

pom.xml
- updated CouchbaseMock dependency to the official release on maven central, which now includes the required commit.

CouchbaseTemplate.java
- remove creation of CouchbasePersistentEntityIndexCreator
- there can only be a single CouchbaseMappingContext and CouchbasePersistentEntityIndexCreator, not sure why there was code dealing with multiples of each, and there was no corresponding tests for these assumptions.
- no longer does the job of a @Configuation class by gluing beans together with their dependencies.

CouchbaseMappingContext.java
- removed references to auto index creation, that functionality should be a completely separate concern.

CouchbasePersistentEntityIndexCreator.java
- as well as implementing ApplicationListener, it now also implements InitializingBean
- when afterPropertiesSet() is called it uses CouchbaseMappingContext#getPersistentEntities to retrieve the list of entity candidates for index creation. This will handle the creation of indices for entities known at @configuration time.
- for entities not in the initialEntitySet of the MappingContext, the ApplicationListener will handle those entities at runtime.
- this is needed because the AbstractMappingContext will emit events when it is initialized via InitializingBean, and any beans that depend on MappingContext and also implement ApplicationListener expecting to receive those events won't.
 - in our case because Spring will have fully initialised the CouchbaseMappingContext instance before passing it to the CouchbasePersistentEntityIndexCreator instance we won't get those events.
- added a boolean flag to indicate if automatic index creation is to be enabled or now, passed in by the factory method that creates this bean.
- minor logging cleanup, use property placeholders where appropriate

AbstractCouchbaseConfiguration.java
- added a new factory method for the CouchbasePersistentEntityIndexCreator bean.
- CouchbaseMappingContext no longer has anything to do with index creation, other than supplying a candidate list of entities that may need indices.

Prompted by:
spring-projects/spring-boot#24525
and the discussion on #295

Co-authored-by: Aaron Whiteside <[email protected]>

Author: aaronjwhiteside

pom.xml

@@ -157,9 +157,9 @@
         </dependency>
 
         <dependency>
-            <groupId>com.github.Couchbase</groupId>
+            <groupId>com.couchbase.mock</groupId>
             <artifactId>CouchbaseMock</artifactId>
-            <version>73e493d259</version>
+            <version>1.5.25</version>
             <scope>test</scope>
         </dependency>
 

src/main/java/org/springframework/data/couchbase/config/AbstractCouchbaseConfiguration.java

@@ -37,6 +37,7 @@
 import org.springframework.data.couchbase.core.convert.MappingCouchbaseConverter;
 import org.springframework.data.couchbase.core.convert.translation.JacksonTranslationService;
 import org.springframework.data.couchbase.core.convert.translation.TranslationService;
+import org.springframework.data.couchbase.core.index.CouchbasePersistentEntityIndexCreator;
 import org.springframework.data.couchbase.core.mapping.CouchbaseMappingContext;
 import org.springframework.data.couchbase.core.mapping.Document;
 import org.springframework.data.couchbase.repository.config.ReactiveRepositoryOperationsMapping;
@@ -268,7 +269,6 @@ public CouchbaseMappingContext couchbaseMappingContext(CustomConversions customC
 		mappingContext.setInitialEntitySet(getInitialEntitySet());
 		mappingContext.setSimpleTypeHolder(customConversions.getSimpleTypeHolder());
 		mappingContext.setFieldNamingStrategy(fieldNamingStrategy());
-		mappingContext.setAutoIndexCreation(autoIndexCreation());
 
 		return mappingContext;
 	}
@@ -280,6 +280,18 @@ protected boolean autoIndexCreation() {
 		return false;
 	}
 
+	/**
+	 * Creates a {@link CouchbasePersistentEntityIndexCreator} bean that takes on the responsibility of automatically
+	 * creating indices.
+	 *
+	 * Does nothing if {@link #autoIndexCreation()} returns false.
+	 */
+	@Bean
+	public CouchbasePersistentEntityIndexCreator couchbasePersistentEntityIndexCreator(CouchbaseMappingContext couchbaseMappingContext,
+   			CouchbaseClientFactory clientFactory) {
+		return new CouchbasePersistentEntityIndexCreator(couchbaseMappingContext, clientFactory, typeKey(), autoIndexCreation());
+	}
+
 	/**
 	 * Register custom Converters in a {@link CustomConversions} object if required. These {@link CustomConversions} will
 	 * be registered with the {@link #mappingCouchbaseConverter(CouchbaseMappingContext, CouchbaseCustomConversions)} )}

src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java

@@ -19,15 +19,8 @@
 import org.springframework.beans.BeansException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
-import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.data.couchbase.CouchbaseClientFactory;
 import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
-import org.springframework.data.couchbase.core.index.CouchbasePersistentEntityIndexCreator;
-import org.springframework.data.couchbase.core.mapping.CouchbaseMappingContext;
-import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity;
-import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
-import org.springframework.data.mapping.context.MappingContext;
-import org.springframework.lang.Nullable;
 
 import com.couchbase.client.java.Collection;
 
@@ -36,30 +29,21 @@
  *
  * @author Michael Nitschinger
  * @author Michael Reiche
+ * @author Aaron Whiteside
  * @since 3.0
  */
 public class CouchbaseTemplate implements CouchbaseOperations, ApplicationContextAware {
 
 	private final CouchbaseClientFactory clientFactory;
 	private final CouchbaseConverter converter;
 	private final CouchbaseTemplateSupport templateSupport;
-	private final MappingContext<? extends CouchbasePersistentEntity<?>, CouchbasePersistentProperty> mappingContext;
 	private final ReactiveCouchbaseTemplate reactiveCouchbaseTemplate;
-	private @Nullable CouchbasePersistentEntityIndexCreator indexCreator;
 
 	public CouchbaseTemplate(final CouchbaseClientFactory clientFactory, final CouchbaseConverter converter) {
 		this.clientFactory = clientFactory;
 		this.converter = converter;
 		this.templateSupport = new CouchbaseTemplateSupport(converter);
 		this.reactiveCouchbaseTemplate = new ReactiveCouchbaseTemplate(clientFactory, converter);
-
-		this.mappingContext = this.converter.getMappingContext();
-		if (mappingContext instanceof CouchbaseMappingContext) {
-			CouchbaseMappingContext cmc = (CouchbaseMappingContext) mappingContext;
-			if (cmc.isAutoIndexCreation()) {
-				indexCreator = new CouchbasePersistentEntityIndexCreator(cmc, this);
-			}
-		}
 	}
 
 	@Override
@@ -148,28 +132,7 @@ public ReactiveCouchbaseTemplate reactive() {
 
 	@Override
 	public void setApplicationContext(final ApplicationContext applicationContext) throws BeansException {
-		prepareIndexCreator(applicationContext);
 		templateSupport.setApplicationContext(applicationContext);
 		reactiveCouchbaseTemplate.setApplicationContext(applicationContext);
 	}
-
-	private void prepareIndexCreator(final ApplicationContext context) {
-		String[] indexCreators = context.getBeanNamesForType(CouchbasePersistentEntityIndexCreator.class);
-
-		for (String creator : indexCreators) {
-			CouchbasePersistentEntityIndexCreator creatorBean = context.getBean(creator,
-					CouchbasePersistentEntityIndexCreator.class);
-			if (creatorBean.isIndexCreatorFor(mappingContext)) {
-				return;
-			}
-		}
-
-		if (context instanceof ConfigurableApplicationContext && indexCreator != null) {
-			((ConfigurableApplicationContext) context).addApplicationListener(indexCreator);
-			if (mappingContext instanceof CouchbaseMappingContext) {
-				CouchbaseMappingContext cmc = (CouchbaseMappingContext) mappingContext;
-				cmc.setIndexCreator(indexCreator);
-			}
-		}
-	}
 }

src/main/java/org/springframework/data/couchbase/core/index/CouchbasePersistentEntityIndexCreator.java

@@ -20,39 +20,56 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.InitializingBean;
 import org.springframework.context.ApplicationListener;
 import org.springframework.dao.DataIntegrityViolationException;
-import org.springframework.data.couchbase.core.CouchbaseOperations;
+import org.springframework.data.couchbase.CouchbaseClientFactory;
 import org.springframework.data.couchbase.core.index.CouchbasePersistentEntityIndexResolver.IndexDefinitionHolder;
 import org.springframework.data.couchbase.core.mapping.CouchbaseMappingContext;
 import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity;
 import org.springframework.data.couchbase.core.mapping.Document;
-import org.springframework.data.mapping.PersistentEntity;
-import org.springframework.data.mapping.context.MappingContext;
-import org.springframework.data.mapping.context.MappingContextEvent;
 
 import com.couchbase.client.core.error.IndexExistsException;
 import com.couchbase.client.java.Cluster;
+import org.springframework.data.mapping.PersistentEntity;
+import org.springframework.data.mapping.context.MappingContextEvent;
 
-public class CouchbasePersistentEntityIndexCreator implements ApplicationListener<MappingContextEvent<?, ?>> {
+/**
+ * Encapsulates the logic of creating indices.
+ *
+ * @author Michael Nitschinger
+ * @author Aaron Whiteside
+ */
+public class CouchbasePersistentEntityIndexCreator implements InitializingBean, ApplicationListener<MappingContextEvent<?, ?>> {
 
 	private static final Logger LOGGER = LoggerFactory.getLogger(CouchbasePersistentEntityIndexCreator.class);
 
 	private final Map<Class<?>, Boolean> classesSeen = new ConcurrentHashMap<>();
 	private final CouchbaseMappingContext mappingContext;
 	private final QueryIndexResolver indexResolver;
-	private final CouchbaseOperations couchbaseOperations;
+	private final CouchbaseClientFactory clientFactory;
+	private final boolean enabled;
 
 	public CouchbasePersistentEntityIndexCreator(final CouchbaseMappingContext mappingContext,
-			final CouchbaseOperations operations) {
+			final CouchbaseClientFactory clientFactory, final String typeKey, final boolean enabled) {
 		this.mappingContext = mappingContext;
-		this.couchbaseOperations = operations;
-		this.indexResolver = QueryIndexResolver.create(mappingContext, operations);
+		this.clientFactory = clientFactory;
+		this.enabled = enabled;
+		this.indexResolver = QueryIndexResolver.create(mappingContext, typeKey);
+	}
+
+	@Override
+	public void afterPropertiesSet() throws Exception {
+		if (enabled) {
+			mappingContext.getPersistentEntities().forEach(this::checkForIndexes);
+		} else {
+			LOGGER.debug("Automatic index creation not enabled.");
+		}
 	}
 
 	@Override
 	public void onApplicationEvent(final MappingContextEvent<?, ?> event) {
-		if (!event.wasEmittedBy(mappingContext)) {
+		if (!enabled || !event.wasEmittedBy(mappingContext)) {
 			return;
 		}
 
@@ -71,7 +88,7 @@ private void checkForIndexes(final CouchbasePersistentEntity<?> entity) {
 			this.classesSeen.put(type, Boolean.TRUE);
 
 			if (LOGGER.isDebugEnabled()) {
-				LOGGER.debug("Analyzing class " + type + " for index information.");
+				LOGGER.debug("Analyzing class {} for index information.", type);
 			}
 
 			checkForAndCreateIndexes(entity);
@@ -93,10 +110,10 @@ private void checkForAndCreateIndexes(final CouchbasePersistentEntity<?> entity)
 	}
 
 	private void createIndex(final IndexDefinitionHolder indexToCreate) {
-		Cluster cluster = couchbaseOperations.getCouchbaseClientFactory().getCluster();
+		Cluster cluster = clientFactory.getCluster();
 
 		StringBuilder statement = new StringBuilder("CREATE INDEX ").append(indexToCreate.getIndexName()).append(" ON `")
-				.append(couchbaseOperations.getBucketName()).append("` (")
+				.append(clientFactory.getBucket().name()).append("` (")
 				.append(String.join(",", indexToCreate.getIndexFields())).append(")");
 
 		if (indexToCreate.getIndexPredicate() != null && !indexToCreate.getIndexPredicate().isEmpty()) {
@@ -107,21 +124,10 @@ private void createIndex(final IndexDefinitionHolder indexToCreate) {
 			cluster.query(statement.toString());
 		} catch (IndexExistsException ex) {
 			// ignored on purpose, rest is propagated
-			LOGGER.debug("Index \"" + indexToCreate.getIndexName() + "\" already exists, ignoring.");
+			LOGGER.debug("Index \"{}\" already exists, ignoring.", indexToCreate.getIndexName());
 		} catch (Exception ex) {
 			throw new DataIntegrityViolationException("Could not auto-create index with statement: " + statement.toString(),
 					ex);
 		}
 	}
-
-	/**
-	 * Returns whether the current index creator was registered for the given {@link MappingContext}.
-	 */
-	public boolean isIndexCreatorFor(final MappingContext<?, ?> context) {
-		return this.mappingContext.equals(context);
-	}
-
-	public boolean hasSeen(CouchbasePersistentEntity<?> entity) {
-		return classesSeen.containsKey(entity.getType());
-	}
 }

src/main/java/org/springframework/data/couchbase/core/index/CouchbasePersistentEntityIndexResolver.java

@@ -20,7 +20,6 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
-import org.springframework.data.couchbase.core.CouchbaseOperations;
 import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity;
 import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
 import org.springframework.data.couchbase.core.mapping.Document;
@@ -35,13 +34,13 @@
 public class CouchbasePersistentEntityIndexResolver implements QueryIndexResolver {
 
 	private final MappingContext<? extends CouchbasePersistentEntity<?>, CouchbasePersistentProperty> mappingContext;
-	private final CouchbaseOperations operations;
+	private final String typeKey;
 
 	public CouchbasePersistentEntityIndexResolver(
 			final MappingContext<? extends CouchbasePersistentEntity<?>, CouchbasePersistentProperty> mappingContext,
-			CouchbaseOperations operations) {
+			final String typeKey) {
 		this.mappingContext = mappingContext;
-		this.operations = operations;
+		this.typeKey = typeKey;
 	}
 
 	@Override
@@ -135,7 +134,6 @@ protected List<IndexDefinitionHolder> createCompositeQueryIndexDefinitions(final
 	}
 
 	private String getPredicate(final MappingCouchbaseEntityInformation<?, Object> entityInfo) {
-		String typeKey = operations.getConverter().getTypeKey();
 		String typeValue = entityInfo.getJavaType().getName();
 		return "`" + typeKey + "` = \"" + typeValue + "\"";
 	}

src/main/java/org/springframework/data/couchbase/core/index/QueryIndexResolver.java

@@ -15,7 +15,6 @@
  */
 package org.springframework.data.couchbase.core.index;
 
-import org.springframework.data.couchbase.core.CouchbaseOperations;
 import org.springframework.data.couchbase.core.mapping.CouchbaseMappingContext;
 import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity;
 import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
@@ -43,9 +42,9 @@ public interface QueryIndexResolver {
 	 */
 	static QueryIndexResolver create(
 			MappingContext<? extends CouchbasePersistentEntity<?>, CouchbasePersistentProperty> mappingContext,
-			CouchbaseOperations operations) {
+			String typeKey) {
 		Assert.notNull(mappingContext, "CouchbaseMappingContext must not be null!");
-		return new CouchbasePersistentEntityIndexResolver(mappingContext, operations);
+		return new CouchbasePersistentEntityIndexResolver(mappingContext, typeKey);
 	}
 
 	/**

src/main/java/org/springframework/data/couchbase/core/mapping/CouchbaseMappingContext.java

@@ -16,15 +16,10 @@
 
 package org.springframework.data.couchbase.core.mapping;
 
-import java.util.Optional;
-
 import org.springframework.beans.BeansException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
-import org.springframework.context.ApplicationEventPublisher;
-import org.springframework.data.couchbase.core.index.CouchbasePersistentEntityIndexCreator;
 import org.springframework.data.mapping.context.AbstractMappingContext;
-import org.springframework.data.mapping.context.MappingContextEvent;
 import org.springframework.data.mapping.model.FieldNamingStrategy;
 import org.springframework.data.mapping.model.Property;
 import org.springframework.data.mapping.model.PropertyNameFieldNamingStrategy;
@@ -37,6 +32,7 @@
  *
  * @author Michael Nitschinger
  * @author Michael Reiche
+ * @author Aaron Whiteside
  */
 public class CouchbaseMappingContext
 		extends AbstractMappingContext<BasicCouchbasePersistentEntity<?>, CouchbasePersistentProperty>
@@ -55,10 +51,6 @@ public class CouchbaseMappingContext
 	 */
 	private FieldNamingStrategy fieldNamingStrategy = DEFAULT_NAMING_STRATEGY;
 
-	private boolean autoIndexCreation = true;
-	private ApplicationEventPublisher eventPublisher;
-	private CouchbasePersistentEntityIndexCreator indexCreator = null;
-
 	/**
 	 * Configures the {@link FieldNamingStrategy} to be used to determine the field name if no manual mapping is applied.
 	 * Defaults to a strategy using the plain property name.
@@ -108,65 +100,8 @@ protected CouchbasePersistentProperty createPersistentProperty(Property property
 	 */
 	@Override
 	public void setApplicationContext(final ApplicationContext applicationContext) throws BeansException {
-		context = applicationContext;
 		super.setApplicationContext(applicationContext);
+		context = applicationContext;
 	}
 
-	@Override
-	public void setApplicationEventPublisher(ApplicationEventPublisher applicationEventPublisher) {
-		eventPublisher = applicationEventPublisher;
-		if (this.eventPublisher == null) {
-			this.eventPublisher = context;
-		}
-	}
-
-	public boolean isAutoIndexCreation() {
-		return autoIndexCreation;
-	}
-
-	public void setAutoIndexCreation(boolean autoCreateIndexes) {
-		this.autoIndexCreation = autoCreateIndexes;
-	}
-
-	/**
-	 * override method from AbstractMappingContext as that method will not publishEvent() if it finds the entity has
-	 * already been cached
-	 * 
-	 * @param typeInformation - entity type
-	 */
-	@Override
-	protected Optional<BasicCouchbasePersistentEntity<?>> addPersistentEntity(TypeInformation<?> typeInformation) {
-		Optional<BasicCouchbasePersistentEntity<?>> entity = super.addPersistentEntity(typeInformation);
-
-		if (this.eventPublisher != null && entity.isPresent()) {
-			if (this.indexCreator != null) {
-				if (!indexCreator.hasSeen(entity.get())) {
-					this.eventPublisher.publishEvent(new MappingContextEvent(this, entity.get()));
-				}
-			}
-		}
-		return entity;
-	}
-
-	/**
-	 * override method from AbstractMappingContext as that method will not publishEvent() if it finds the entity has
-	 * already been cached. Instead, user our own addPersistEntity that will.
-	 * 
-	 * @param typeInformation - entity type
-	 */
-	@Override
-	public BasicCouchbasePersistentEntity<?> getPersistentEntity(TypeInformation<?> typeInformation) {
-		Optional<BasicCouchbasePersistentEntity<?>> entity = addPersistentEntity(typeInformation);
-		return entity.isPresent() ? entity.get() : null;
-	}
-
-	/**
-	 * capture the indexCreator when it has been added as a listener. only publishEvent() if the indexCreator hasn't
-	 * already seen the class.
-	 * 
-	 * @param indexCreator
-	 */
-	public void setIndexCreator(CouchbasePersistentEntityIndexCreator indexCreator) {
-		this.indexCreator = indexCreator;
-	}
 }