Skip to content

Conversation

@JoseLion
Copy link
Contributor

@JoseLion JoseLion commented Nov 25, 2021

Closes #689

  • You have read the Spring Data contribution guidelines.
  • 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).

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 25, 2021
Comment on lines +464 to +465
@Value
@With
Copy link
Contributor Author

@JoseLion JoseLion Nov 25, 2021

Choose a reason for hiding this comment

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

Why did I change this?

This was the main reason for the test not catching the bug. The problem was that onBeforeConvert(..) was mutating the instance of the entity passed to the callback. This side-effect was spread through the code and reached the private method doUpdate(T entity, SqlIdentifier tableName), making the test think that it was using the value returned by the callback when it was actually using the mutated entity passed through the parameter.

I noticed that other parts of this test are already using immutability through @Value and @With, so I thought it'd make sense to use the same for Person with the minimum possible changes to preserve the behavior of the tests 🙂

@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 7, 2021
@mp911de mp911de self-assigned this Dec 7, 2021
@mp911de mp911de changed the title Fix entityToUse on non-versioned updates. Use BeforeConvert callback result on non-versioned updates Dec 7, 2021
@mp911de mp911de added this to the 1.3.8 (2021.0.8) milestone Dec 7, 2021
mp911de pushed a commit that referenced this pull request Dec 7, 2021
Previously, we used the original entity.

Closes #689
Original pull request: #690.
mp911de added a commit that referenced this pull request Dec 7, 2021
Remove unused imports. Use more explicit lambda parameter naming.

Original pull request: #690.
See #689
mp911de pushed a commit that referenced this pull request Dec 7, 2021
Previously, we used the original entity.

Closes #689
Original pull request: #690.
mp911de added a commit that referenced this pull request Dec 7, 2021
Remove unused imports. Use more explicit lambda parameter naming.

Also, adopt tests to changed Spring Data Relational SQL generation.

Original pull request: #690.
See #689
mp911de pushed a commit that referenced this pull request Dec 7, 2021
Previously, we used the original entity.

Closes #689
Original pull request: #690.
mp911de added a commit that referenced this pull request Dec 7, 2021
Remove unused imports. Use more explicit lambda parameter naming.

Original pull request: #690.
See #689
@mp911de
Copy link
Member

mp911de commented Dec 7, 2021

Thank you for your contribution. That's merged, polished, and backported now.

@mp911de mp911de closed this Dec 7, 2021
@JoseLion JoseLion deleted the issue/gh-689 branch December 8, 2021 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Before save callback not using transformed entity on update

3 participants