Support transactions manager with spring-data #1145
Labels
type: enhancement
A general enhancement
Comments
spring-projects-issues
added
the
status: waiting-for-triage
mikereiche
added
type: enhancement
mikereiche
added a commit
that referenced
this issue
Oct 8, 2021
5 tasks
mikereiche
added a commit
that referenced
this issue
Oct 8, 2021
mikereiche
added a commit
that referenced
this issue
Oct 8, 2021
mikereiche
added a commit
that referenced
this issue
Oct 16, 2021
mikereiche
added a commit
that referenced
this issue
Oct 19, 2021
mikereiche
added a commit
that referenced
this issue
Oct 22, 2021
The fluent operations are common for options that are common to operations with and without transactions. Once there is a transaction(ctx), or an option specific to without-transactions (such as scanConsistency), the interfaces are bifurcated, so that an non-transaction option cannot be applied to a transaction operation, and a transaction(ctx) cannot be applied where a non-transaction option has already been applied. Closes #1145.
mikereiche
added a commit
that referenced
this issue
Mar 15, 2022
The fluent operations are common for options that are common to operations with and without transactions. Once there is a transaction(ctx), or an option specific to without-transactions (such as scanConsistency), the interfaces are bifurcated, so that an non-transaction option cannot be applied to a transaction operation, and a transaction(ctx) cannot be applied where a non-transaction option has already been applied. Closes #1145.
mikereiche
added a commit
that referenced
this issue
Jun 24, 2022
The fluent operations are common for options that are common to operations with and without transactions. Once there is a transaction(ctx), or an option specific to without-transactions (such as scanConsistency), the interfaces are bifurcated, so that an non-transaction option cannot be applied to a transaction operation, and a transaction(ctx) cannot be applied where a non-transaction option has already been applied. Closes #1145. Support for @transactional for blocking and reactive Transactionmanager. Closes 1145. Transaction Support. Transaction Support. Porting to SDK-integrated version of transactions The transactions logic exists in the Java SDK as of 3.3.0, with a slightly different API. This is the first effort at the port, which literally just compiles. It will not run as crucial code has been commented and todo-ed. There is work remaining to figure out how to complete the port, as some crucial parts (such as ctx.commit() and ctx.rollback()) have been intentionally removed. Continuing work to get the ExtSDKIntegration port working Trying to transition to CallbackPreferring manager. Added CouchbaseSimpleCallbackTransactionManager, the simplest possible implementation of CallbackPreferringTransactionManager, combined with a simpler approach to ThreadLocal storage in ReactiveInsertByIdSupport. Test 'commitShouldPersistTxEntriesOfTxAnnotatedMethod' is now passing. Adding WIP get-and-replace @transactional support (Not yet working as CAS/version field in Person is not populated correctly.) Datacouch 1145 transaction support (#1423) * Porting to SDK-integrated version of transactions The transactions logic exists in the Java SDK as of 3.3.0, with a slightly different API. This is the first effort at the port, which literally just compiles. It will not run as crucial code has been commented and todo-ed. There is work remaining to figure out how to complete the port, as some crucial parts (such as ctx.commit() and ctx.rollback()) have been intentionally removed. * Continuing work to get the ExtSDKIntegration port working Trying to transition to CallbackPreferring manager. * Added CouchbaseSimpleCallbackTransactionManager, the simplest possible implementation of CallbackPreferringTransactionManager, combined with a simpler approach to ThreadLocal storage in ReactiveInsertByIdSupport. Test 'commitShouldPersistTxEntriesOfTxAnnotatedMethod' is now passing. * Adding WIP get-and-replace @transactional support (Not yet working as CAS/version field in Person is not populated correctly.) Commit before pulling Graham's changes. Merge branch 'datacouch_1145_transaction_support' of github.com:spring-projects/spring-data-couchbase into datacouch_1145_transaction_support Transitioning to use CoreTransactionAttemptContext. Tests may fail. Removing AttemptContextReactiveAccessor Don't think we need this, as we can pass around CoreTransactionAttemptContext instead, which gives access to a lot of internals. Removing TransactionsReactive Would prefer not to C&P a huge class out of the transaction internals, and don't think we need it. Removing some files not currently used To reduce & simplify the amount of code to look at. Some don't seem to be used in any branch, some just aren't used in this branch. Removing CouchbaseTransactionInterceptor As per offline discussion, CallbackPreferringPlatformTransactionManager is perhaps the optimal solution. Copying @transactional tests out into separate class Tidyup Tidyup test names Verify GenericSupport is on same thread before and after transactional operation Refactoring CouchbaseSimpleCallbackTransactionManager ThreadLocalStorage management Using latest java-client ReactiveReplaceByIdSupport - Fixing use of CAS now have CoreTransactionAttemptContext. Removing unused code. ReactiveInsertByIdSupport - fixing use of reactive vs non-reactive, and CAS Merging upstream Remove incorrect thread check (.doOnNext could execute on a different thread) Get scope and collection from pseudoArgs and some cleanup. Completing merge from upstream Removing unused classes Give GenericSupport a better name Reject at runtime options that aren't supported in a transaction Fixing some small todos, partly by removing unused coe Fix runtime option checks Simplifying CouchbaseSimpleCallbackTransactionManager ThreadLocalStorage Standardising on ReactiveCouchbaseResourceHolder rather than holding CoreTransactionAttemptContext too Removing version from CouchbaseDocument Can't recall why I added this, and tests pass without it Improving CouchbaseTransactionalIntegrationTests and adding more tests Reject operations that aren't allowed in a transaction (upsertById etc.) Improve handling of CAS mismatch By calling CoreTransactionAttemptContext.operationFailed, it ensures that internal state is set. So even if the user catches the exception, the transaction still behaves as it should. Removing a now-redundant non-transactional check on upsertById I missed this when adding TransactionalSupport.verifyNotInTransaction here. Support @transactional options timeout and isolation level Add ReactiveTransactionWrapper/TransactionWrapper and a bunch of cleanup. Merge branch 'datacouch_1145_transaction_support' of https://github.com/programmatix/spring-data-couchbase into programmatix-datacouch_1145_transaction_support Fixed up merge issues. Datacouch 1145 transaction support (#1447) * Move CouchbaseTransactionalOperator to use SLF4J, same as rest of the code. * Handle all propagation levels * Adding new tests for repository calls inside @transactional One test is failure due to what looks like a bug elsewhere. * Rename CouchbaseTransactionalIntegrationTests, and check after each test that we're not in a transaction. Remove unnecessary methods From ReactiveCouchbaseClientFactory. Also rationalized naming of methods and other changes. Cleanup of test classes. Datacouch 1145 transaction support (#1448) * Move CouchbaseTransactionalOperator to use SLF4J, same as rest of the code. * Handle all propagation levels * Adding new tests for repository calls inside @transactional One test is failure due to what looks like a bug elsewhere. * Rename CouchbaseTransactionalIntegrationTests, and check after each test that we're not in a transaction. * Remove unnecessary methods From ReactiveCouchbaseClientFactory. Also rationalized naming of methods and other changes. * Cleanup of test classes. * Removing unused classes (Reducing the cognitive burden) * Removing version from CouchbaseDocument This change was done previously - it must have slipped back in a merge. * Adding and removing TODOs * Adding and removing TODOs * DRYing CouchbaseTransactionalPropagationIntegrationTests * Check propagation tests retry as expected * Tidy up PersonWithoutVersion to the minimum required * Removing unused code This should all be non-destructive. Just removing code IntelliJ declares unused. Intent is to make it easier to figure out how the CoreTransactionAttemptContext TLS is working. * Adding tests for @transactional removeByQuery and findByQuery Failing as they aren't being executed transactionally - investigating why. Co-authored-by: Michael Reiche <[email protected]> change references to resource holder change refs to resource holder Merge branch 'programmatix-datacouch_1145_transaction_support' of github.com:spring-projects/spring-data-couchbase into programmatix-datacouch_1145_transaction_support Removing core transaction attempt context bound couchbase client factory rebased (#1449) * Move ReactiveTransactionsWrapper tests into a new file * (Temporarily?) disabling tests using CouchbaseTransactionOperation or TransactionalOperator As I feel we should be removing/not-supporting these, and on this branch I've broken them. * Make all transaction tests call assertNotInTransaction * Removing unused code * Instead of binding the transaction AttemptContext to a CouchbaseClientFactory, fetch it from ThreadLocalStorage (or the reactive context) instead. This allows a lot of simplifying: * The non-trivial ReactiveCouchbaseClientUtils can be removed * As can CoreTransactionAttemptContextBoundCouchbaseClientFactory Also removing TransactionalSupport.one as it wasn't providing as much DRY utility as I thought it would - only used in two places. This change won't compile on its own. To reduce the complexity of this patchset, the Reactive*OperationSupport changes will go into a separate commit. * Reactive*OperationSupport changes to support the previous commit. * Fixing ReactiveRemoveByQuerySupport. Both to support the changes to TransactionalSupport. And to fix the TODO where the query resuls were not being handled. * Disabling a test * Adding CouchbaseTransactionsWrapperTemplateIntegrationTests * Another advantage of removing CoreTransactionAttemptContextBoundCouchbaseClientFactory is we can remove Cluster and ClusterInterface. * Adding CouchbaseReactiveTransactionsWrapperTemplateIntegrationTests Some of these tests are currently failing - tracking down where the issue is. Merge branch 'programmatix-datacouch_1145_transaction_support' of github.com:spring-projects/spring-data-couchbase into programmatix-datacouch_1145_transaction_support manual merges for PR manual merges for PR Fix a bunch of test cases and remove unused bits. Removing CouchbaseTransactionInterceptor As discussed on Slack. Reenabling some tests that are passing (Unclear why these were disabled?) Verified that CallbackPreferringPlatformTransactionManager getTransaction/commit/rollback are never called Adding tests for TransactionTemplate, which works fine with CouchbaseSimpleCallbackTransactionManager Whether we actually document this support is another matter - it's yet another way of doing transactions. Small fixes to support TransactionTemplate * Handle if the user has set isRollbackOnly on the TransactionStatus (which is only available - I think - when using TransactionTemplate) * Supply a `transaction` object to CouchbaseTransactionStatus so that status.isNewTransaction() correctly returns true. (This method requires that transaction to be set.) Clarifying that direct use of PlatformTransactionManager is not supported Adding further TransactionTemplateIntegrationTests tests Fixing removeByQuery queryOptions creation Removing now-fixed TODO (no longer key off Cluster) Adding overload to CouchbaseSimpleCallbackTransactionManager to allow it to be constructed without a TransactionOptions. Adding CouchbaseSimpleTransactionalOperator, the simplest possible implementation of TransactionalOperator. Adding retry tests Just making sure that error handling and retries are done correctly throughout. Updating and adding some TODOs Have CouchbaseTransactionManager support CouchbaseResourceHolder.class binding Adding more tests for CouchbaseTransactionManager. These tests fail, and are known to fail. I'm adding them as a solid demonstration of why I don't feel we can have this CouchbaseTransactionManager: it doesn't provide the crucial 'core loop' functionality, including error handling and retries. We should standardise on CouchbaseSimpleCallbackTransactionManager instead. CouchbaseSimpleCallbackTransactionManager.executeNewReactiveTransaction now buffers results rather than trying to stream a Flux from out of a completed lambda (which I doubt is even possible.) Removing comment that has been resolved. (As per Slack, we will live with this limitation.) Adding CouchbaseSimpleTransactionInterceptor, a very simple TransactionInterceptor implemention that defers to CouchbaseSimpleCallbackTransactionManager if that is the provided TransactionManager, and otherwise just calls super. This allows reactive @transactional - though all @transactional methods including blocking will now flow through it. There are two rather divergent approaches in the code currently: 1. CouchbaseTransactionManager, ReactiveTransactionManager, CouchbaseTransactionalOperator, CouchbaseTransactionInterceptor 2. CouchbaseSimpleCallbackTransactionManager, CouchbaseSimpleTransactionalOperator, CouchbaseSimpleTransactionInterceptor I know the intent is to remove some aspects of (1), but until that's done it's proving tricky to have tests for both concurrently - I've hit several issues on adding CouchbaseSimpleTransactionInterceptor, with 'multiple transaction manager beans in config' being common. So, temporarily moving some beans from AbstractCouchbaseConfiguration into the test Config class, renaming it, and having two separately TransactionsConfig classes for the two approaches. Once we've aligned the approaches more, can move what beans survive back into AbstractCouchbaseConfiguration. Safety check in CouchbaseSimpleCallbackTransactionManager that the blocking run is not accidentally running a reactive @transactional somehow. Adding tests for reactive @transactional, which now works (including error handling and retries) as of the CouchbaseSimpleTransactionInterceptor. With TransactionsConfigCouchbaseSimpleTransactionManager change, can now simplify @transactional(transactionManager = ...) to just @transactional. Tidying TODOs I saw in a PR comment that getResources no longer uses TransactionOptions Removing configureTransactions from config As per PR discussion Removing some code that has now been refactored into 3.3.1 SDK Removing TODOs that are TODONE already Removing transactionsOptions() bean from AbstractCouchbaseConfiguration. As per comment, this feels unnecessary: any options you'd configure at this config level, you'd surely provide at the global (Cluster) level instead? Reinstating some commented-out code This looks pretty crucial - can't recall why I commented it in first place Removing TransactionResult This was from a now-abandoned idea of storing transactional metadata in the entity class. Fix recent removal of TransactionOptions bean Switch some IllegalStateException for more accurate UnsupportedOperationException Tidying tests to remove old GenericApplicationContext method Tidying some TODOs (todo gp == in code that I think we should remove) (todo gpx == needs looking at) Tidying TransactionsConfigCouchbaseSimpleTransactionManager Provide SpringTransactionAttemptContext and ReactiveSpringTransactionAttemptContext wrappers For use by TransactionsWrapper and reactive equivalent. Pro: it's an abstraction layer. It lets us add Spring-specific functionality, or hide functionality that for whatever reason doesn't work with Spring (which is why it's composition rather than inheritance, beyond that being a best practice anyway). Con: any new API added will also have to be added to these wrappers. But that's a small amount of work and API is added very infrequently. Cleanup tests mostly. Temporary fix for CouchbaseSimpleCallbackTransactionManager. Merge branch 'programmatix-datacouch_1145_transaction_support' of github.com:spring-projects/spring-data-couchbase into programmatix-datacouch_1145_transaction_support Tidying up test cases. Fixes to over-zealous manual merging. - keep CouchbaseTransactionManager as Graham is still using it. - fix tests that were expecting SimulateFailure to be nested. Add ReactiveTransactionWrapper and enable tests that use it. More tidying. Removed CouchbaseTransactionManager. Only wrap exceptions in CouchbaseSimpleTransactionInterceptor if they are not RuntimeExceptions. Removed CouchbaseTransactionalOperator and ReactiveCouchbaseTransactionalOperator. Fixed non-transactions regression introduced in previous commit. Removing the two transactions config classes Now we've landed on a single agreed approach there's no need for this separation any more, and any beans can be moved back into AbstractCouchbaseConfiguration. Rename CouchbaseSimpleTransactionInterceptor The "Simple" moniker is no longer useful since we now only have one of these. Rename CouchbaseSimpleCallbackTransactionManager The "Simple" moniker is no longer useful since we now only have one of these. Rename CouchbaseSimpleTransactionalOperator The "Simple" moniker is no longer useful since we now only have one of these. Change CouchbaseTransactionalOperator construction to static As per Slack discussion. Remove comments related to another database Removing a test comment that doesn't seem to apply anymore Test passes for me at least Simplify TransactionsWrapper and AttemptContextReactiveAccessor newCoreTranactionAttemptContext is reimplementing some code from core plus has some config bugs to resolve. I think it's simpler to remove it and replace TransactionsWrapper (the only code still using this method) with the simple code seen now. (Note this is similar to how I had it before 1701183 - not sure if that commit intentionally reverted things?) This change also petmits a lot of tidyup & simplification throughout the codebase. We can just create CouchbaseResourceHolders directly now. Tidying a test No longer needs retryWhen now using the new approaches Remove ReactiveCouchbaseClientFactory. It was added to support getting the transaction from the TransactionSynchronizationManager.forCurrent() and providing a template with a session containing the transaction. Fixing some code warnings Mostly removing unused code Mark internal classes @Stability.Internal Removing some tests that have already been previously moved into another file. Removing the transaction wrappers Requires JVMCBC-1105 and 3.3.2 Add more tests for native SDK transactions Move all tests related to native SDK transactions into their own package It makes it easier to test just that functionality while iterating. Removing CouchbaseTransactionManagerTransactionalTemplateIntegrationTests This test is now redundant. It was created to show why the original CouchbaseTransactionManager couldn't work (no retries). Now that has been replaced, this test is just duplicating others. Adding some minimal JavaDocs and comments. Tidying up after moving ThreadLocalStorage into SDK Deleting CouchbaseTemplateTransactionIntegrationTests As this relies on Spring test @transactional, which we do not support as that Spring logic is not aware of CallbackPreferringTransactionManager. Removed some redundant bean names Tidying up tests and comments Removing some now-unused reflection code Starting with mapping TransactionFailedException and TransactionCommitAmbiguousException, into new errors TransactionSystemUnambiguousException and TransactionSystemAmbiguousException. These will be raised from an @transactional transaction. E.g. to do error handling the user would do: ``` try { service.transactionalMethod(); } catch (TransactionSystemAmbiguousException ex) { // app-specific handling } catch (TransactionSystemUnambiguousException ex) { // app-specific handling } class Service { @transactional void transactionalMethod() { // ... } } ``` Mapping TransactionOperationFailedException, which is an opaque signal raised from transaction operations, to new exception UncategorizedTransactionDataAccessException. This depends on some new functionality added into Java SDK 3.3.2, WrappedTransactionOperationFailedException. Minor tidyuo Improving tests for correct operation-level errors
mikereiche
added a commit
that referenced
this issue
Jun 25, 2022
The fluent operations are common for options that are common to operations with and without transactions. Once there is a transaction(ctx), or an option specific to without-transactions (such as scanConsistency), the interfaces are bifurcated, so that an non-transaction option cannot be applied to a transaction operation, and a transaction(ctx) cannot be applied where a non-transaction option has already been applied. Closes #1145. Support for @transactional for blocking and reactive Transactionmanager. Closes 1145. Transaction Support. Transaction Support. Porting to SDK-integrated version of transactions The transactions logic exists in the Java SDK as of 3.3.0, with a slightly different API. This is the first effort at the port, which literally just compiles. It will not run as crucial code has been commented and todo-ed. There is work remaining to figure out how to complete the port, as some crucial parts (such as ctx.commit() and ctx.rollback()) have been intentionally removed. Continuing work to get the ExtSDKIntegration port working Trying to transition to CallbackPreferring manager. Added CouchbaseSimpleCallbackTransactionManager, the simplest possible implementation of CallbackPreferringTransactionManager, combined with a simpler approach to ThreadLocal storage in ReactiveInsertByIdSupport. Test 'commitShouldPersistTxEntriesOfTxAnnotatedMethod' is now passing. Adding WIP get-and-replace @transactional support (Not yet working as CAS/version field in Person is not populated correctly.) Datacouch 1145 transaction support (#1423) * Porting to SDK-integrated version of transactions The transactions logic exists in the Java SDK as of 3.3.0, with a slightly different API. This is the first effort at the port, which literally just compiles. It will not run as crucial code has been commented and todo-ed. There is work remaining to figure out how to complete the port, as some crucial parts (such as ctx.commit() and ctx.rollback()) have been intentionally removed. * Continuing work to get the ExtSDKIntegration port working Trying to transition to CallbackPreferring manager. * Added CouchbaseSimpleCallbackTransactionManager, the simplest possible implementation of CallbackPreferringTransactionManager, combined with a simpler approach to ThreadLocal storage in ReactiveInsertByIdSupport. Test 'commitShouldPersistTxEntriesOfTxAnnotatedMethod' is now passing. * Adding WIP get-and-replace @transactional support (Not yet working as CAS/version field in Person is not populated correctly.) Commit before pulling Graham's changes. Merge branch 'datacouch_1145_transaction_support' of github.com:spring-projects/spring-data-couchbase into datacouch_1145_transaction_support Transitioning to use CoreTransactionAttemptContext. Tests may fail. Removing AttemptContextReactiveAccessor Don't think we need this, as we can pass around CoreTransactionAttemptContext instead, which gives access to a lot of internals. Removing TransactionsReactive Would prefer not to C&P a huge class out of the transaction internals, and don't think we need it. Removing some files not currently used To reduce & simplify the amount of code to look at. Some don't seem to be used in any branch, some just aren't used in this branch. Removing CouchbaseTransactionInterceptor As per offline discussion, CallbackPreferringPlatformTransactionManager is perhaps the optimal solution. Copying @transactional tests out into separate class Tidyup Tidyup test names Verify GenericSupport is on same thread before and after transactional operation Refactoring CouchbaseSimpleCallbackTransactionManager ThreadLocalStorage management Using latest java-client ReactiveReplaceByIdSupport - Fixing use of CAS now have CoreTransactionAttemptContext. Removing unused code. ReactiveInsertByIdSupport - fixing use of reactive vs non-reactive, and CAS Merging upstream Remove incorrect thread check (.doOnNext could execute on a different thread) Get scope and collection from pseudoArgs and some cleanup. Completing merge from upstream Removing unused classes Give GenericSupport a better name Reject at runtime options that aren't supported in a transaction Fixing some small todos, partly by removing unused coe Fix runtime option checks Simplifying CouchbaseSimpleCallbackTransactionManager ThreadLocalStorage Standardising on ReactiveCouchbaseResourceHolder rather than holding CoreTransactionAttemptContext too Removing version from CouchbaseDocument Can't recall why I added this, and tests pass without it Improving CouchbaseTransactionalIntegrationTests and adding more tests Reject operations that aren't allowed in a transaction (upsertById etc.) Improve handling of CAS mismatch By calling CoreTransactionAttemptContext.operationFailed, it ensures that internal state is set. So even if the user catches the exception, the transaction still behaves as it should. Removing a now-redundant non-transactional check on upsertById I missed this when adding TransactionalSupport.verifyNotInTransaction here. Support @transactional options timeout and isolation level Add ReactiveTransactionWrapper/TransactionWrapper and a bunch of cleanup. Merge branch 'datacouch_1145_transaction_support' of https://github.com/programmatix/spring-data-couchbase into programmatix-datacouch_1145_transaction_support Fixed up merge issues. Datacouch 1145 transaction support (#1447) * Move CouchbaseTransactionalOperator to use SLF4J, same as rest of the code. * Handle all propagation levels * Adding new tests for repository calls inside @transactional One test is failure due to what looks like a bug elsewhere. * Rename CouchbaseTransactionalIntegrationTests, and check after each test that we're not in a transaction. Remove unnecessary methods From ReactiveCouchbaseClientFactory. Also rationalized naming of methods and other changes. Cleanup of test classes. Datacouch 1145 transaction support (#1448) * Move CouchbaseTransactionalOperator to use SLF4J, same as rest of the code. * Handle all propagation levels * Adding new tests for repository calls inside @transactional One test is failure due to what looks like a bug elsewhere. * Rename CouchbaseTransactionalIntegrationTests, and check after each test that we're not in a transaction. * Remove unnecessary methods From ReactiveCouchbaseClientFactory. Also rationalized naming of methods and other changes. * Cleanup of test classes. * Removing unused classes (Reducing the cognitive burden) * Removing version from CouchbaseDocument This change was done previously - it must have slipped back in a merge. * Adding and removing TODOs * Adding and removing TODOs * DRYing CouchbaseTransactionalPropagationIntegrationTests * Check propagation tests retry as expected * Tidy up PersonWithoutVersion to the minimum required * Removing unused code This should all be non-destructive. Just removing code IntelliJ declares unused. Intent is to make it easier to figure out how the CoreTransactionAttemptContext TLS is working. * Adding tests for @transactional removeByQuery and findByQuery Failing as they aren't being executed transactionally - investigating why. Co-authored-by: Michael Reiche <[email protected]> change references to resource holder change refs to resource holder Merge branch 'programmatix-datacouch_1145_transaction_support' of github.com:spring-projects/spring-data-couchbase into programmatix-datacouch_1145_transaction_support Removing core transaction attempt context bound couchbase client factory rebased (#1449) * Move ReactiveTransactionsWrapper tests into a new file * (Temporarily?) disabling tests using CouchbaseTransactionOperation or TransactionalOperator As I feel we should be removing/not-supporting these, and on this branch I've broken them. * Make all transaction tests call assertNotInTransaction * Removing unused code * Instead of binding the transaction AttemptContext to a CouchbaseClientFactory, fetch it from ThreadLocalStorage (or the reactive context) instead. This allows a lot of simplifying: * The non-trivial ReactiveCouchbaseClientUtils can be removed * As can CoreTransactionAttemptContextBoundCouchbaseClientFactory Also removing TransactionalSupport.one as it wasn't providing as much DRY utility as I thought it would - only used in two places. This change won't compile on its own. To reduce the complexity of this patchset, the Reactive*OperationSupport changes will go into a separate commit. * Reactive*OperationSupport changes to support the previous commit. * Fixing ReactiveRemoveByQuerySupport. Both to support the changes to TransactionalSupport. And to fix the TODO where the query resuls were not being handled. * Disabling a test * Adding CouchbaseTransactionsWrapperTemplateIntegrationTests * Another advantage of removing CoreTransactionAttemptContextBoundCouchbaseClientFactory is we can remove Cluster and ClusterInterface. * Adding CouchbaseReactiveTransactionsWrapperTemplateIntegrationTests Some of these tests are currently failing - tracking down where the issue is. Merge branch 'programmatix-datacouch_1145_transaction_support' of github.com:spring-projects/spring-data-couchbase into programmatix-datacouch_1145_transaction_support manual merges for PR manual merges for PR Fix a bunch of test cases and remove unused bits. Removing CouchbaseTransactionInterceptor As discussed on Slack. Reenabling some tests that are passing (Unclear why these were disabled?) Verified that CallbackPreferringPlatformTransactionManager getTransaction/commit/rollback are never called Adding tests for TransactionTemplate, which works fine with CouchbaseSimpleCallbackTransactionManager Whether we actually document this support is another matter - it's yet another way of doing transactions. Small fixes to support TransactionTemplate * Handle if the user has set isRollbackOnly on the TransactionStatus (which is only available - I think - when using TransactionTemplate) * Supply a `transaction` object to CouchbaseTransactionStatus so that status.isNewTransaction() correctly returns true. (This method requires that transaction to be set.) Clarifying that direct use of PlatformTransactionManager is not supported Adding further TransactionTemplateIntegrationTests tests Fixing removeByQuery queryOptions creation Removing now-fixed TODO (no longer key off Cluster) Adding overload to CouchbaseSimpleCallbackTransactionManager to allow it to be constructed without a TransactionOptions. Adding CouchbaseSimpleTransactionalOperator, the simplest possible implementation of TransactionalOperator. Adding retry tests Just making sure that error handling and retries are done correctly throughout. Updating and adding some TODOs Have CouchbaseTransactionManager support CouchbaseResourceHolder.class binding Adding more tests for CouchbaseTransactionManager. These tests fail, and are known to fail. I'm adding them as a solid demonstration of why I don't feel we can have this CouchbaseTransactionManager: it doesn't provide the crucial 'core loop' functionality, including error handling and retries. We should standardise on CouchbaseSimpleCallbackTransactionManager instead. CouchbaseSimpleCallbackTransactionManager.executeNewReactiveTransaction now buffers results rather than trying to stream a Flux from out of a completed lambda (which I doubt is even possible.) Removing comment that has been resolved. (As per Slack, we will live with this limitation.) Adding CouchbaseSimpleTransactionInterceptor, a very simple TransactionInterceptor implemention that defers to CouchbaseSimpleCallbackTransactionManager if that is the provided TransactionManager, and otherwise just calls super. This allows reactive @transactional - though all @transactional methods including blocking will now flow through it. There are two rather divergent approaches in the code currently: 1. CouchbaseTransactionManager, ReactiveTransactionManager, CouchbaseTransactionalOperator, CouchbaseTransactionInterceptor 2. CouchbaseSimpleCallbackTransactionManager, CouchbaseSimpleTransactionalOperator, CouchbaseSimpleTransactionInterceptor I know the intent is to remove some aspects of (1), but until that's done it's proving tricky to have tests for both concurrently - I've hit several issues on adding CouchbaseSimpleTransactionInterceptor, with 'multiple transaction manager beans in config' being common. So, temporarily moving some beans from AbstractCouchbaseConfiguration into the test Config class, renaming it, and having two separately TransactionsConfig classes for the two approaches. Once we've aligned the approaches more, can move what beans survive back into AbstractCouchbaseConfiguration. Safety check in CouchbaseSimpleCallbackTransactionManager that the blocking run is not accidentally running a reactive @transactional somehow. Adding tests for reactive @transactional, which now works (including error handling and retries) as of the CouchbaseSimpleTransactionInterceptor. With TransactionsConfigCouchbaseSimpleTransactionManager change, can now simplify @transactional(transactionManager = ...) to just @transactional. Tidying TODOs I saw in a PR comment that getResources no longer uses TransactionOptions Removing configureTransactions from config As per PR discussion Removing some code that has now been refactored into 3.3.1 SDK Removing TODOs that are TODONE already Removing transactionsOptions() bean from AbstractCouchbaseConfiguration. As per comment, this feels unnecessary: any options you'd configure at this config level, you'd surely provide at the global (Cluster) level instead? Reinstating some commented-out code This looks pretty crucial - can't recall why I commented it in first place Removing TransactionResult This was from a now-abandoned idea of storing transactional metadata in the entity class. Fix recent removal of TransactionOptions bean Switch some IllegalStateException for more accurate UnsupportedOperationException Tidying tests to remove old GenericApplicationContext method Tidying some TODOs (todo gp == in code that I think we should remove) (todo gpx == needs looking at) Tidying TransactionsConfigCouchbaseSimpleTransactionManager Provide SpringTransactionAttemptContext and ReactiveSpringTransactionAttemptContext wrappers For use by TransactionsWrapper and reactive equivalent. Pro: it's an abstraction layer. It lets us add Spring-specific functionality, or hide functionality that for whatever reason doesn't work with Spring (which is why it's composition rather than inheritance, beyond that being a best practice anyway). Con: any new API added will also have to be added to these wrappers. But that's a small amount of work and API is added very infrequently. Cleanup tests mostly. Temporary fix for CouchbaseSimpleCallbackTransactionManager. Merge branch 'programmatix-datacouch_1145_transaction_support' of github.com:spring-projects/spring-data-couchbase into programmatix-datacouch_1145_transaction_support Tidying up test cases. Fixes to over-zealous manual merging. - keep CouchbaseTransactionManager as Graham is still using it. - fix tests that were expecting SimulateFailure to be nested. Add ReactiveTransactionWrapper and enable tests that use it. More tidying. Removed CouchbaseTransactionManager. Only wrap exceptions in CouchbaseSimpleTransactionInterceptor if they are not RuntimeExceptions. Removed CouchbaseTransactionalOperator and ReactiveCouchbaseTransactionalOperator. Fixed non-transactions regression introduced in previous commit. Removing the two transactions config classes Now we've landed on a single agreed approach there's no need for this separation any more, and any beans can be moved back into AbstractCouchbaseConfiguration. Rename CouchbaseSimpleTransactionInterceptor The "Simple" moniker is no longer useful since we now only have one of these. Rename CouchbaseSimpleCallbackTransactionManager The "Simple" moniker is no longer useful since we now only have one of these. Rename CouchbaseSimpleTransactionalOperator The "Simple" moniker is no longer useful since we now only have one of these. Change CouchbaseTransactionalOperator construction to static As per Slack discussion. Remove comments related to another database Removing a test comment that doesn't seem to apply anymore Test passes for me at least Simplify TransactionsWrapper and AttemptContextReactiveAccessor newCoreTranactionAttemptContext is reimplementing some code from core plus has some config bugs to resolve. I think it's simpler to remove it and replace TransactionsWrapper (the only code still using this method) with the simple code seen now. (Note this is similar to how I had it before 1701183 - not sure if that commit intentionally reverted things?) This change also petmits a lot of tidyup & simplification throughout the codebase. We can just create CouchbaseResourceHolders directly now. Tidying a test No longer needs retryWhen now using the new approaches Remove ReactiveCouchbaseClientFactory. It was added to support getting the transaction from the TransactionSynchronizationManager.forCurrent() and providing a template with a session containing the transaction. Fixing some code warnings Mostly removing unused code Mark internal classes @Stability.Internal Removing some tests that have already been previously moved into another file. Removing the transaction wrappers Requires JVMCBC-1105 and 3.3.2 Add more tests for native SDK transactions Move all tests related to native SDK transactions into their own package It makes it easier to test just that functionality while iterating. Removing CouchbaseTransactionManagerTransactionalTemplateIntegrationTests This test is now redundant. It was created to show why the original CouchbaseTransactionManager couldn't work (no retries). Now that has been replaced, this test is just duplicating others. Adding some minimal JavaDocs and comments. Tidying up after moving ThreadLocalStorage into SDK Deleting CouchbaseTemplateTransactionIntegrationTests As this relies on Spring test @transactional, which we do not support as that Spring logic is not aware of CallbackPreferringTransactionManager. Removed some redundant bean names Tidying up tests and comments Removing some now-unused reflection code Starting with mapping TransactionFailedException and TransactionCommitAmbiguousException, into new errors TransactionSystemUnambiguousException and TransactionSystemAmbiguousException. These will be raised from an @transactional transaction. E.g. to do error handling the user would do: ``` try { service.transactionalMethod(); } catch (TransactionSystemAmbiguousException ex) { // app-specific handling } catch (TransactionSystemUnambiguousException ex) { // app-specific handling } class Service { @transactional void transactionalMethod() { // ... } } ``` Mapping TransactionOperationFailedException, which is an opaque signal raised from transaction operations, to new exception UncategorizedTransactionDataAccessException. This depends on some new functionality added into Java SDK 3.3.2, WrappedTransactionOperationFailedException. Minor tidyuo Improving tests for correct operation-level errors
mikereiche
added a commit
that referenced
this issue
Jun 28, 2022
Documentation to follow. Closes #1145.
5 tasks
mikereiche
added a commit
that referenced
this issue
Jun 28, 2022
Documentation to follow. Closes #1145.
mikereiche
added a commit
that referenced
this issue
Jun 28, 2022
Documentation to follow. Closes #1145.
mikereiche
added a commit
that referenced
this issue
Jun 28, 2022
In addition to transaction support, some inconsitencies in fluent APIs were remedied. Documentation to follow. Closes #1145.
mikereiche
added a commit
that referenced
this issue
Jun 28, 2022
In addition to transaction support, some inconsitencies in fluent APIs were remedied. Documentation to follow. Closes #1145.
This was referenced Jun 28, 2022
mikereiche
added a commit
that referenced
this issue
Jun 29, 2022
In addition to transaction support, some inconsitencies in fluent APIs were remedied. Documentation to follow. Closes #1145.
mikereiche
added a commit
that referenced
this issue
Jun 29, 2022
In addition to transaction support, some inconsitencies in fluent APIs were remedied. Documentation to follow. Closes #1145.
mikereiche
added a commit
that referenced
this issue
Jun 29, 2022
In addition to transaction support, some inconsitencies in fluent APIs were remedied. Documentation to follow. Closes #1145.
mikereiche
added a commit
that referenced
this issue
Jun 30, 2022
In addition to transaction support, some inconsitencies in fluent APIs were remedied. Documentation to follow. Closes #1145.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Today transaction support in spring-data-couchbase is provided by native Java SDK.
Enhance this support to provide annotation based transaction support.
references:
https://stackoverflow.com/questions/58498795/add-transaction-manager-for-a-couchbase-app-in-a-springboot-2-app-for-junit-tes
https://spring.io/guides/gs/managing-transactions/
The text was updated successfully, but these errors were encountered: