[wip/4.2] Implemented Reactive versions of PostAction, PreAction and JdbcSelectWithActions #2732
Conversation
hibernate-github-bot
bot
changed the title
hibernate-github-bot
bot
added
the
wip/4.2
| public String name; | ||
|
|
||
| @ManyToOne | ||
| @ManyToOne(fetch = FetchType.LAZY) |
There was a problem hiding this comment.
I don't think you should change the mapping of an existing test. ALso, does it matter if you use lazy here?
There was a problem hiding this comment.
With the new Lock implementation in ORM, it affects the number of queries executed.
I'm going to revert this change and modify the testFindUpgradeNoWait assertions accrdingly
…ction` and `JdbcSelectWithActions`
dreab8
force-pushed
the
issue#2651_4.2
branch
3 times, most recently
from
4189a5f to
229542e
Compare
dreab8
force-pushed
the
issue#2651_4.2
branch
from
229542e to
a48c3b8
Compare
DavideD
force-pushed
the
wip/4.2
branch
2 times, most recently
from
b0fdbef to
b543eb0
Compare
DavideD
added
the
waiting
|
We need to wait for the next ORM 7.2 release |
| * Reactive version of {@link ConnectionLockTimeoutStrategy} | ||
| */ | ||
| public interface ReactiveConnectionLockTimeoutStrategy extends ConnectionLockTimeoutStrategy { | ||
| Log LOG = LoggerFactory.make( Log.class, MethodHandles.lookup() ); |
There was a problem hiding this comment.
I know that we use this pattern in some places, but
I think it's better if we don't put the LOG in the interface and override the method in the implemetations instead, even if it means we have some duplication.
|
|
||
| @FunctionalInterface | ||
| public interface TimeoutExtractor { | ||
| CompletionStage<Timeout> extractFrom(ReactiveConnection.Result resultSet); |
There was a problem hiding this comment.
Are you sure this needs to return a CompletionStage? All the implementations I see return a constant.
There was a problem hiding this comment.
I did a little refactoring and it seems you can just return a Timeout object: DavideD@f76290d
Feel free to squash the commit in your PR, if you agree with the changes
| return LockHelper.getLockTimeout( | ||
| "show lock_timeout", | ||
| (resultSet) -> { | ||
| // see https://dev.mysql.com/doc/refman/8.4/en/innodb-parameters.html#sysvar_innodb_lock_wait_timeout |
There was a problem hiding this comment.
I think you copied and paste the link: it links to the MySQL docs but it's in the code for the Cockroach strategy
The same error happens in all the other implementations
| Timeout timeout, | ||
| ReactiveConnection connection, | ||
| SessionFactoryImplementor factory) { | ||
| return LockHelper.setLockTimeout( |
There was a problem hiding this comment.
This code is the same as the one in Postgres. If Cockroach calls the one in Postgres, we remove some duplicated code.
| * Reactive version of {@link TableLock} | ||
| */ | ||
| public class ReactiveTableLock extends TableLock { | ||
| Log LOG = LoggerFactory.make( Log.class, MethodHandles.lookup() ); |
There was a problem hiding this comment.
this LOG should be private static final
| ReactiveListResultsConsumer.UniqueSemantic.ALLOW | ||
| ).thenCompose( results -> { | ||
| if ( isEmpty( results ) ) { | ||
| throw new AssertionFailure( "Expecting results" ); |
There was a problem hiding this comment.
Can't we add anything else to the error message? What should have the query returned? Something related to a lock I imagine.
| reference = null; | ||
| } | ||
| applyLoadedState( entityDetails, statePosition, reference ); | ||
| applyModelState( entityDetails, statePosition, reference ); |
| } | ||
|
|
||
| @Override | ||
| public CompletionStage<Void> reactiveApplyResult( |
There was a problem hiding this comment.
Unless I'm missing something, nothing in this method seems reactive. Do you need to return a CompletionStage?
| if ( stateValue == null ) { | ||
| if ( !toOne.isNullable() ) { | ||
| throw new IllegalStateException( "Retrieved key was null, but to-one is not nullable : " + toOne.getNavigableRole() | ||
| .getFullPath() ); |
There was a problem hiding this comment.
Shouldn't this have a error code in the Log?
2 participants
Fix #2651
The PR implements the Reactive version of
JdbcSelectWithActionsand of the newPostActionandPreAction(ReactiveLockTimeoutHandlerandReactiveLockTimeoutHandler).The PR does not implement the Reactive version of
CollectionLockingActionbut in order to use it a Reactive version oforg.hibernate.query.SelectionQuery#setLockScope(Locking.Scope lockScope);method need to be added.