From 80bae4bf67c8deb78fa9cef5c685335b62f16efd Mon Sep 17 00:00:00 2001 From: Aaron Whiteside Date: Fri, 15 Jan 2021 06:42:44 +1100 Subject: [PATCH] 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: https://github.com/spring-projects/spring-boot/issues/24525 and the discussion on https://github.com/spring-projects/spring-data-couchbase/pull/295 Co-authored-by: Aaron Whiteside --- pom.xml | 4 +- .../AbstractCouchbaseConfiguration.java | 14 +++- .../couchbase/core/CouchbaseTemplate.java | 39 +---------- ...CouchbasePersistentEntityIndexCreator.java | 56 ++++++++------- ...ouchbasePersistentEntityIndexResolver.java | 8 +-- .../core/index/QueryIndexResolver.java | 5 +- .../core/mapping/CouchbaseMappingContext.java | 69 +------------------ 7 files changed, 54 insertions(+), 141 deletions(-) diff --git a/pom.xml b/pom.xml index 0c2a65af..f3ea757a 100644 --- a/pom.xml +++ b/pom.xml @@ -157,9 +157,9 @@ - com.github.Couchbase + com.couchbase.mock CouchbaseMock - 73e493d259 + 1.5.25 test diff --git a/src/main/java/org/springframework/data/couchbase/config/AbstractCouchbaseConfiguration.java b/src/main/java/org/springframework/data/couchbase/config/AbstractCouchbaseConfiguration.java index 32cf783c..dc090127 100644 --- a/src/main/java/org/springframework/data/couchbase/config/AbstractCouchbaseConfiguration.java +++ b/src/main/java/org/springframework/data/couchbase/config/AbstractCouchbaseConfiguration.java @@ -37,6 +37,7 @@ import org.springframework.data.couchbase.core.convert.CouchbaseCustomConversion 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 abstract class AbstractCouchbaseConfiguration { mappingContext.setInitialEntitySet(getInitialEntitySet()); mappingContext.setSimpleTypeHolder(customConversions.getSimpleTypeHolder()); mappingContext.setFieldNamingStrategy(fieldNamingStrategy()); - mappingContext.setAutoIndexCreation(autoIndexCreation()); return mappingContext; } @@ -280,6 +280,18 @@ public abstract class AbstractCouchbaseConfiguration { 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)} )} diff --git a/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java b/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java index 2648a8e7..5c44404f 100644 --- a/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java +++ b/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java @@ -19,15 +19,8 @@ package org.springframework.data.couchbase.core; 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,6 +29,7 @@ import com.couchbase.client.java.Collection; * * @author Michael Nitschinger * @author Michael Reiche + * @author Aaron Whiteside * @since 3.0 */ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationContextAware { @@ -43,23 +37,13 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationContex private final CouchbaseClientFactory clientFactory; private final CouchbaseConverter converter; private final CouchbaseTemplateSupport templateSupport; - private final MappingContext, 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 class CouchbaseTemplate implements CouchbaseOperations, ApplicationContex @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); - } - } - } } diff --git a/src/main/java/org/springframework/data/couchbase/core/index/CouchbasePersistentEntityIndexCreator.java b/src/main/java/org/springframework/data/couchbase/core/index/CouchbasePersistentEntityIndexCreator.java index 39ca60b7..b4c09c98 100644 --- a/src/main/java/org/springframework/data/couchbase/core/index/CouchbasePersistentEntityIndexCreator.java +++ b/src/main/java/org/springframework/data/couchbase/core/index/CouchbasePersistentEntityIndexCreator.java @@ -20,39 +20,56 @@ import java.util.concurrent.ConcurrentHashMap; 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> { +/** + * Encapsulates the logic of creating indices. + * + * @author Michael Nitschinger + * @author Aaron Whiteside + */ +public class CouchbasePersistentEntityIndexCreator implements InitializingBean, ApplicationListener> { private static final Logger LOGGER = LoggerFactory.getLogger(CouchbasePersistentEntityIndexCreator.class); private final Map, 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 @@ public class CouchbasePersistentEntityIndexCreator implements ApplicationListene 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 @@ public class CouchbasePersistentEntityIndexCreator implements ApplicationListene } 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 @@ public class CouchbasePersistentEntityIndexCreator implements ApplicationListene 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()); - } } diff --git a/src/main/java/org/springframework/data/couchbase/core/index/CouchbasePersistentEntityIndexResolver.java b/src/main/java/org/springframework/data/couchbase/core/index/CouchbasePersistentEntityIndexResolver.java index 4ef36a4d..be3f83bc 100644 --- a/src/main/java/org/springframework/data/couchbase/core/index/CouchbasePersistentEntityIndexResolver.java +++ b/src/main/java/org/springframework/data/couchbase/core/index/CouchbasePersistentEntityIndexResolver.java @@ -20,7 +20,6 @@ import java.util.Arrays; 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 @@ import org.springframework.util.StringUtils; public class CouchbasePersistentEntityIndexResolver implements QueryIndexResolver { private final MappingContext, CouchbasePersistentProperty> mappingContext; - private final CouchbaseOperations operations; + private final String typeKey; public CouchbasePersistentEntityIndexResolver( final MappingContext, CouchbasePersistentProperty> mappingContext, - CouchbaseOperations operations) { + final String typeKey) { this.mappingContext = mappingContext; - this.operations = operations; + this.typeKey = typeKey; } @Override @@ -135,7 +134,6 @@ public class CouchbasePersistentEntityIndexResolver implements QueryIndexResolve } private String getPredicate(final MappingCouchbaseEntityInformation entityInfo) { - String typeKey = operations.getConverter().getTypeKey(); String typeValue = entityInfo.getJavaType().getName(); return "`" + typeKey + "` = \"" + typeValue + "\""; } diff --git a/src/main/java/org/springframework/data/couchbase/core/index/QueryIndexResolver.java b/src/main/java/org/springframework/data/couchbase/core/index/QueryIndexResolver.java index 27b3f45c..7cb2e872 100644 --- a/src/main/java/org/springframework/data/couchbase/core/index/QueryIndexResolver.java +++ b/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, CouchbasePersistentProperty> mappingContext, - CouchbaseOperations operations) { + String typeKey) { Assert.notNull(mappingContext, "CouchbaseMappingContext must not be null!"); - return new CouchbasePersistentEntityIndexResolver(mappingContext, operations); + return new CouchbasePersistentEntityIndexResolver(mappingContext, typeKey); } /** diff --git a/src/main/java/org/springframework/data/couchbase/core/mapping/CouchbaseMappingContext.java b/src/main/java/org/springframework/data/couchbase/core/mapping/CouchbaseMappingContext.java index 6421dcd6..b30284c9 100644 --- a/src/main/java/org/springframework/data/couchbase/core/mapping/CouchbaseMappingContext.java +++ b/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 @@ import org.springframework.data.util.TypeInformation; * * @author Michael Nitschinger * @author Michael Reiche + * @author Aaron Whiteside */ public class CouchbaseMappingContext extends AbstractMappingContext, 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 @@ public class CouchbaseMappingContext */ @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> addPersistentEntity(TypeInformation typeInformation) { - Optional> 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> 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; - } }