Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add optional artefact index reader for startup scanning #16000
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
base: 8.0.x
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Add optional artefact index reader for startup scanning #16000
Changes from 1 commit
ccc1a3c20543427662a26File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
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.
Two operational concerns for production startup code:
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.
IOUtils.findRootResourcethrows IllegalStateException, not IOException, when the class resource can't be resolved (targetClass.getResource(...)returning null — e.g. an application class from a classloader that doesn't expose.classresources). That escapes this catch and would fail startup, where today the same situation just scans. Since the whole contract of this reader is "never make things worse than the fallback," this should catch that too (e.g.catch (IOException | RuntimeException)).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.
Rejecting the entire index on an empty line is stricter than it needs to be — a trailing blank line is the most common artifact of text-file generation and concatenation (the spec's own
writeIndexhas to.trim()to avoid it). Skipping blank lines (continue) keeps the strict-reject behavior for genuinely malformed content while tolerating the boring case. If strictness is intentional as a whole-file integrity signal, the javadoc should say the producer must not emit blank lines, including trailing ones.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.
Worth documenting the semantic differences from the
ClassPathScannerpath this replaces, since the future index producer has to compensate for them:grails.(defaultannotationFilter); the reader trusts every listed entry with no annotation check, so a hand-edited or buggy index can inject arbitrary classes into the artefact set.DEFAULT_IGNORED_ROOT_PACKAGES(com,org,net, …) even when explicitly passed as packageNames; the reader honors them.Both are fine if the producer mirrors scan semantics exactly, but that contract currently lives nowhere — a sentence in the class javadoc would pin it.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.