Skip to content
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

Fix mutated metadata being cleared before it was written in Groovy classes #6749

Merged
merged 5 commits into from Jan 13, 2022

Conversation

jameskleeh
Copy link
Contributor

@dstepanov Please give this a try with data

@dstepanov
Copy link
Contributor

It fixes runtime errors but there are still some compile problems with getRequiredValue not returning default values. You can reproduce them by reverting modified Java files.

@jameskleeh
Copy link
Contributor Author

Right because the code to retrieve the default values only looks at the runtime source of that information. If we should not leak the compile time data to runtime, which was the cause of the original issue, then either default value data will not be available at compile time or the methods to get the defaults needs to look at the compile time defaults.

@dstepanov
Copy link
Contributor

We have code that relies on compile-time default values so we need to fix it somehow 🤔

@graemerocher
Copy link
Contributor

@jameskleeh can you link to the original issue, the original PR that broke everything to solve that issue and so on. None of the PRs that tried to mitigate this issue include any references to the issues they solve so it is hard to trace back the history of the issue

@dstepanov
Copy link
Contributor

The problem in 3.1.4 was caused by #6455 and the fix that broke more stuff #6734

@jameskleeh
Copy link
Contributor Author

@graemerocher The original issue is #6433

The fix I created for that was #6455 which basically reused the runtime cache at compile time. Because there was a singular cache the data from Groovy was leaking to Java. Groovy is creating proxies for annotation instances and the Java annotation metadata stuff is not designed to handle them.

I tried to solve that issue by clearing the cache after compilation but that caused lots of issues with compiling code at runtime in our tests, so it was evident the runtime cache could not be cleared.

The next step I took was to separate the compile time from runtime default value caches so the compile time cache could be cleared after compilation preventing one languages values leaking to another. #6734

Unfortunately I mistakenly cleared the mutated metadata too early for Groovy so that caused runtime issues of mutated metadata being missing. It also had the side effect of annotation default values not being available at compile time because the metadata only looks at the runtime cache and we aren't populating it

@dstepanov
Copy link
Contributor

I can confirm these changes fix the problems in Micronaut Data

@jameskleeh jameskleeh merged commit 61a9533 into 3.2.x Jan 13, 2022
@jameskleeh jameskleeh deleted the mutated_cache branch January 13, 2022 17:33
@jameskleeh jameskleeh linked an issue Jan 18, 2022 that may be closed by this pull request
yawkat pushed a commit that referenced this pull request Jan 21, 2022
…asses (#6749)

* Fix mutated metadata being cleared before it was written in Groovy classes

* Add test for mutated metadata being used in bean definitions

* Read defaults from compile time information for mutable metadata

* Reuse existing test instead

* Fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error compiling Data JDBC interface
3 participants