fix(reports): report generation does not cause recursive update #368
Conversation
andrewazores
force-pushed
the
report-generation-fail-test
branch
from
c89fc24 to
936cc76
Compare
|
/build_test |
|
Workflow started at 4/15/2024, 1:28:45 PM. View Actions Run. |
|
No OpenAPI schema changes detected. |
|
CI build and push: All tests pass ✅ (JDK17) |
andrewazores
force-pushed
the
report-generation-fail-test
branch
from
936cc76 to
4ad6270
Compare
|
/build_test |
|
Workflow started at 4/15/2024, 2:50:02 PM. View Actions Run. |
|
No OpenAPI schema changes detected. |
|
CI build and push: All tests pass ✅ (JDK17) |
andrewazores
force-pushed
the
report-generation-fail-test
branch
from
4ad6270 to
2d5775e
Compare
|
/build_test |
|
Workflow started at 4/18/2024, 5:20:22 PM. View Actions Run. |
|
No OpenAPI schema changes detected. |
|
CI build and push: All tests pass ✅ (JDK17) |
andrewazores
force-pushed
the
report-generation-fail-test
branch
3 times, most recently
from
ff7e2b3 to
c748807
Compare
andrewazores
force-pushed
the
report-generation-fail-test
branch
from
c748807 to
aee6e18
Compare
|
/build_test |
|
Workflow started at 4/24/2024, 11:16:54 AM. View Actions Run. |
|
No OpenAPI schema changes detected. |
|
No GraphQL schema changes detected. |
|
CI build and push: All tests pass ✅ (JDK17) |
Welcome to Cryostat3! 👋
Before contributing, make sure you have:
mainbranch[chore, ci, docs, feat, fix, test]To recreate commits with GPG signature
git fetch upstream && git rebase --force --gpg-sign upstream/mainFixes: #367
Description of the change:
See the linked bug. There is a Quarkus change that causes the
defaultmethod implementations in theReportsServiceto break the dependency injection system such that the memory caching layer decorator receives an instance of itself for injection, rather than the storage cache layer as a delegate. This causes it to recursively call itself, causing the original bug. Lifting the default method implementations out of the interface and into the concrete implementations fixes the injection bug.Also renamed some constants for clarity, and re-enabled (largely reimplemented) the report generation test case (related to #42). Funnily enough, after reverting the Quarkus upgrade to the previous working version, fixing up this test suite, and then rolling the upgrade forward again, the bug was not caught by the tests... because the tests disable caching on purpose to ensure things are actually generated as expected and tests are independent of each other. But disabling the cache also fixes the bug inadvertently, while reducing performance.
For now the only enabled report generation tests are for active recordings, but enabling them for archived recordings as well would be good. I have other higher priority things to spend my time on right now, however.
How to manually test: