Datacouch 550 autoindex not working in spring boot app #295

mikereiche · 2020-12-15T01:33:27Z

This needs some discussion as to whether or not we want to support this.
I created the branch to capture the debugging and changes I had made
earlier when Denis asked me to investigate this issue.
Mark Paluch has indicated this is maybe not a good idea.

Due to the interdependencies of beans, autoindexing does not work
in the startup of a normal spring-boot application. Processing of
entities occurs during the initialization of CouchbaseMappinContext,
which occurs before initialization of MappingCouchbaseConverter,
which occurs before the initialization of CouchbaseTemplate -
which creates the indexCreator listener. So when the
AbstractMappingContext (CouchbaseMappingContext) is publishing
MappingContextEvents - there is not yet any indexCreator.
And subsequent processing of entities finds the entities cached,
and therefore does not publish MappingContextEvents. This change
overrides the addPersistentEntity() and getPersistentEntity()
methods of AbstractMappingContext such that caching does not
preven the MappingContextEvents from being published. This change
also exposes indexCreator classesSeen map to CouchbaseMappingContext
so that once an entity has been processed by indexCreator, it is
not published again.

Autoindexing by SpringJUnitConfig did work prior to this change.

There is no test case as our automated testing does not create a spring-boot app.

You have read the Spring Data contribution guidelines.
There is a ticket in the bug tracker for the project in our JIRA.
You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
[-] You submit test cases (unit or integration tests) that back your changes.
You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

Due to the interdependency in bean creation, Autoindexing does not work in a normal spring-boot application. This change overrides the addPersistentEntity and getPersistentEntity of CouchbaseMappingContext to avoid entries cached before the indexCreator was listening from preventing MappingContextEvents from being published after the indexCreator is listening. It also exposes the indexCreators classesSeen map so that MappingEvents for an entity are not re-published after the indexCreator has seen them.

Due to the interdependencies of beans, autoindexing does not work in the startup of a normal spring-boot application. Processing of entities occurs during the initialization of CouchbaseMappinContext, which occurs before initialization of MappingCouchbaseConverter, which occurs before the initialization of CouchbaseTemplate - which creates the indexCreator listener. So when the AbstractMappingContext (CouchbaseMappingContext) is publishing MappingContextEvents - there is not yet any indexCreator. And subsequent processing of entities finds the entities cached, and therefore does not publish MappingContextEvents. This change overrides the addPersistentEntity() and getPersistentEntity() methods of AbstractMappingContext such that caching does not preven the MappingContextEvents from being published. This change also exposes indexCreator classesSeen map to CouchbaseMappingContext so that once an entity has been processed by indexCreator, it is not published again. Autoindexing by SpringJUnitConfig did work prior to this change.

aaronjwhiteside · 2020-12-16T22:24:41Z

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

@@ -166,6 +166,10 @@ private void prepareIndexCreator(final ApplicationContext context) {

 		if (context instanceof ConfigurableApplicationContext && indexCreator != null) {
 			((ConfigurableApplicationContext) context).addApplicationListener(indexCreator);
+			if (mappingContext instanceof CouchbaseMappingContext) {


would it not make sense to take this opportunity to move the glueing of beans out of the main code path and into a @configuration class?

aaronjwhiteside · 2020-12-16T22:34:14Z

@mikereiche I was too slow to catch you before this was merged, but I have a few thoughts..

This might solve the issue, but I think it's adding more complexity into the mix..

I think it might be cleaner if CouchbasePersistentEntityIndexCreator creation was moved completely out of the CouchbaseTemplate and into a @Configuration class.

I imagine it would take CouchbaseOperations and MappingContext as constructor arguments, and implement InitializingBean.

In afterPropertiesSet() you could simply call MappingContext#getPersistentEntities() and iterate over the results doing what it currently does.

No need to listen to any events, and no need to implement complex interactions/callbacks with ApplicationContextAware.

… boot app (#295)" This reverts commit 8ebb81b.

mikereiche · 2020-12-16T23:27:00Z

@aaronjwhiteside
"This might solve the issue, but I think it's adding more complexity into the mix.."
I don't have the background of why it was implemented the way it was - with events published to an application listener - I can only assume there were reasons - perhaps because autoindexing could take several minutes and holding up application initialization for that long is not acceptable (although the alternative of having the application initialized before the indexes are generated and that creates its own issues). Or perhaps because that is the point at which all persistent entities are processed. Probably both.
I just fixed the mechanism that was already there by ensuring that events would be sent to the listener after it was listening - which was the original intent.

aaronjwhiteside · 2020-12-16T23:37:21Z

@mikereiche events can continue to be produced as they are now, but you do not have to use the events to trigger the automate creation of the indices.

If something needs to be async, then that should be explicit, i.e. an Executor in the CouchbasePersistentEntityIndexCreator

But even that doesn't hold weight for me, like you said the indices should be available prior to the application starting.

And I'm just remember my own trouble with trying to parallelize index creation in Couchbase, the short answer is it's not supported.

Should we not strive to make things as simple as they can be, but no simpler? Right now we're adding workarounds for a bad design..

mikereiche · 2020-12-16T23:43:08Z

Make a pull request.

mikereiche · 2020-12-17T00:24:55Z

trying to parallelize index creation in Couchbase, the short answer is it's not supported.

the onApplicationEvent() calls are made serially. That's not an issue.

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 - no longer implements ApplicationListener - but instead calls CouchbaseMappingContext#getPersistentEntities to retrieve the list of entity candidates for index creation. - now implements InitializingBean, to trigger the creation of any needed indices. - 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 spring-projects#295

aaronjwhiteside · 2020-12-17T03:28:24Z

@mikereiche please see #298

…xCreator. 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 - no longer implements ApplicationListener - but instead calls CouchbaseMappingContext#getPersistentEntities to retrieve the list of entity candidates for index creation. - now implements InitializingBean, to trigger the creation of any needed indices. - 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 spring-projects#295

@configuration

…xCreator. 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 spring-projects#295

@configuration

…xCreator. 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 spring-projects#295

@configuration

…xCreator. 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 spring-projects#295

@configuration

…xCreator. (#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 <awhiteside@yapstone.com>

…ts#295) * DATACOUCH-550 Autoindexing only works from SpringJunitConfig Due to the interdependency in bean creation, Autoindexing does not work in a normal spring-boot application. This change overrides the addPersistentEntity and getPersistentEntity of CouchbaseMappingContext to avoid entries cached before the indexCreator was listening from preventing MappingContextEvents from being published after the indexCreator is listening. It also exposes the indexCreators classesSeen map so that MappingEvents for an entity are not re-published after the indexCreator has seen them. * DATACOUCH-550 - Autoindexing does not work spring-boot application. Due to the interdependencies of beans, autoindexing does not work in the startup of a normal spring-boot application. Processing of entities occurs during the initialization of CouchbaseMappinContext, which occurs before initialization of MappingCouchbaseConverter, which occurs before the initialization of CouchbaseTemplate - which creates the indexCreator listener. So when the AbstractMappingContext (CouchbaseMappingContext) is publishing MappingContextEvents - there is not yet any indexCreator. And subsequent processing of entities finds the entities cached, and therefore does not publish MappingContextEvents. This change overrides the addPersistentEntity() and getPersistentEntity() methods of AbstractMappingContext such that caching does not preven the MappingContextEvents from being published. This change also exposes indexCreator classesSeen map to CouchbaseMappingContext so that once an entity has been processed by indexCreator, it is not published again. Autoindexing by SpringJUnitConfig did work prior to this change. Co-authored-by: mikereiche <michael.reiche@couchbase.com>

… boot app (spring-projects#295)" This reverts commit 8ebb81b.

@configuration

…xCreator. (spring-projects#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 spring-projects#295 Co-authored-by: Aaron Whiteside <awhiteside@yapstone.com>

@configuration

…xCreator. (#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 <awhiteside@yapstone.com>

mikereiche added 2 commits December 14, 2020 15:31

daschl mentioned this pull request Dec 16, 2020

Spring Data Couchbase Auto Configuration prevents automatic index creation from occurring spring-projects/spring-boot#24525

Closed

mikereiche merged commit 8ebb81b into master Dec 16, 2020

aaronjwhiteside reviewed Dec 16, 2020

View reviewed changes

mikereiche added a commit that referenced this pull request Dec 16, 2020

DATACOUCH-677 - Revert "Datacouch 550 autoindex not working in spring…

6c6dc13

… boot app (#295)" This reverts commit 8ebb81b.

aaronjwhiteside mentioned this pull request Dec 17, 2020

DATACOUCH-550 - Refactored the usage of CouchbasePersistentEntityIndexCreator #298

Merged

spring-projects-issues mentioned this pull request Dec 31, 2020

AutoIndexing is not Working properly [DATACOUCH-550] #858

Closed

mikereiche deleted the datacouch_550_autoindex_not_working_in_spring_boot_app branch January 15, 2021 00:15

jorgerod pushed a commit to jorgerod/spring-data-couchbase that referenced this pull request Jan 15, 2021

DATACOUCH-677 - Revert "Datacouch 550 autoindex not working in spring…

b1e10c1

… boot app (spring-projects#295)" This reverts commit 8ebb81b.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Datacouch 550 autoindex not working in spring boot app #295

Datacouch 550 autoindex not working in spring boot app #295

Uh oh!

mikereiche commented Dec 15, 2020 •

edited

Loading

Uh oh!

aaronjwhiteside Dec 16, 2020

Uh oh!

aaronjwhiteside commented Dec 16, 2020 •

edited

Loading

Uh oh!

mikereiche commented Dec 16, 2020

Uh oh!

aaronjwhiteside commented Dec 16, 2020

Uh oh!

mikereiche commented Dec 16, 2020

Uh oh!

mikereiche commented Dec 17, 2020

Uh oh!

aaronjwhiteside commented Dec 17, 2020

Uh oh!

Uh oh!

Datacouch 550 autoindex not working in spring boot app #295

Datacouch 550 autoindex not working in spring boot app #295

Uh oh!

Conversation

mikereiche commented Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaronjwhiteside Dec 16, 2020

Choose a reason for hiding this comment

Uh oh!

aaronjwhiteside commented Dec 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikereiche commented Dec 16, 2020

Uh oh!

aaronjwhiteside commented Dec 16, 2020

Uh oh!

mikereiche commented Dec 16, 2020

Uh oh!

mikereiche commented Dec 17, 2020

Uh oh!

aaronjwhiteside commented Dec 17, 2020

Uh oh!

Uh oh!

mikereiche commented Dec 15, 2020 •

edited

Loading

aaronjwhiteside commented Dec 16, 2020 •

edited

Loading