From 72bab5bde799901f9df6320b8220922acd192ac0 Mon Sep 17 00:00:00 2001 From: Aaron Whiteside Date: Thu, 17 Dec 2020 14:24:50 +1100 Subject: [PATCH] DATACOUCH-550 - Refactored the usage of CouchbasePersistentEntityIndexCreator. 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 --- 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 f6a6789f3..6c63dd454 100644 --- a/pom.xml +++ b/pom.xml @@ -150,9 +150,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 32cf783c2..dc0901270 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.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)} )} 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 2648a8e7e..5c44404fd 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 @@ 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 @@ * * @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 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); - } - } - } } 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 39ca60b7f..b4c09c986 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 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 @@ 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()); - } } 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 4ef36a4de..be3f83bc6 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.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, 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 @@ protected List createCompositeQueryIndexDefinitions(final } 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 27b3f45c5..7cb2e8723 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 6421dcd60..b30284c9d 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 @@ * * @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 @@ 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> 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; - } }