-
Notifications
You must be signed in to change notification settings - Fork 359
DATAJDBC-161: Share an SqlSession into a same transaction #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking a look and spending time on this. Very much appreciated.
I marked a couple of changes where I don't understand how they are related to DATAJDBC-161. Could you please clarify, or remove them from the commit?
Even just confirming if they are unrelated would be fine, I can take it from there.
@@ -24,9 +24,12 @@ | |||
@Alias("DummyEntity") | |||
class DummyEntity { | |||
|
|||
@Id final Long id; | |||
@Id final long id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems unrelated to DATAJDBC-161, possibly related to DATAJDBC-160
*/ | ||
@ContextConfiguration | ||
@Transactional | ||
public class MyBatisBatchHsqlIntegrationTests { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again I don't understand the connection to the issue this tries to fix.
@@ -12,11 +12,19 @@ | |||
<insert id="insert" parameterType="MyBatisContext" useGeneratedKeys="true" keyProperty="instance.id" keyColumn="ID"> | |||
INSERT INTO DummyEntity (id) VALUES (DEFAULT) | |||
</insert> | |||
<select id="findById" resultType="MyBatisContext" resultMap="dummyEntityMap"> | |||
<select id="findById" resultMap="dummyEntityMap"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again possibly unrelated to the issue at hand.
Took the essential part, polished and merged it. Thanks. |
@schauder Thanks for fixes! |
Postgres integration tests now run either with a locally installed and started database or if no such database can be found an instance gets started in a Docker container via Testcontainers. We prefer a database based on TestContainers. Only if this can't be obtained do we try to access a local database for Postgres. This minimizes the risk of accessing a database during tests that is not intended for that purpose. If the system property `spring.data.r2dbc.test.preferLocalDatabase` is set to "true" the local database is preferred. Original pull request: #51.
Add author tags. Add static factory method to create an unavailable database object instance. Simplify database selection. Javadoc. Original pull request: #51.
I've fixed the DATAJDBC-161.
Please review this.