Skip to content

Fix collections on transaction query. #1571

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

Merged

Conversation

mikereiche
Copy link
Collaborator

Closes #1569.

  • 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).

@mikereiche mikereiche added this to the 5.0 RC1 (2022.0.0) milestone Oct 3, 2022
@mikereiche mikereiche requested review from daschl and dnault October 3, 2022 19:37
Copy link
Contributor

@daschl daschl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should @programmatix also review this?

Copy link
Collaborator Author

@mikereiche mikereiche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, @programmatix could review it as well. For some reason I can't select him as a reviewer.

Copy link
Collaborator Author

@mikereiche mikereiche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to merge this without review from @programmatix as he is feeling under the weather today.

@mikereiche mikereiche merged commit deb6e81 into main Oct 4, 2022
return (AttemptContextReactiveAccessor.createReactiveTransactionAttemptContext(s.get().getCore(),
clientFactory.getCluster().environment().jsonSerializer())).query(statement, opts);
JsonSerializer jSer = clientFactory.getCluster().environment().jsonSerializer();
return AttemptContextReactiveAccessor.createReactiveTransactionAttemptContext(s.get().getCore(), jSer)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TransactionAttemptContext.query overload that takes a Scope doesn't explicitly mark that as being @nullable (and in fact arguably should be checking it's not-null), so it would be less brittle if this code followed the specced API like so:

if (rs.name.equals(DEFAULT_SCOPE)) {
  ctx.query(statement, opts);
}
else {
  ctx.query(rs, statement, opts);
}

But it's ok. I'll just have to be careful to never change my interface to require scope to be non-null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants