fix(generation): the glue templates get every reference they render - #6723
Merged
Conversation
Master went red on IntentEmissionCoverageIT with a 404 on a generated
controller. Two defects in the Java generation pipeline, both in generated glue
code, both reported far from their cause:
- GlueGenerator.bindGenerate never bound fromGenFolder or fromProjectName, so
Generate.java.template rendered `gen.${fromGenFolder}.data...` verbatim -
Velocity leaves an undefined reference in the output. The JavaScript this
replaced computed both (generateUtils.js: crossModelSource ? the source
model's folder : this project's).
- RollupAggregates tested `capacityField != null` where the JavaScript tested
truthiness. The intent glue writes "" for an unused optional field, and ""
is not null, so an unused capacity emitted `parent. == null ? ... : parent. ;`
- a member access with no member.
Either one breaks the WHOLE application: client Java compiles the registry in
one javac task, so a single unparseable generated file unregisters every
controller. The symptom was a 404 on AccountController, which has nothing to do
with roll-ups or create-from.
Nothing caught this because ModelGenerationIT - which replaced the parity
oracle when the JavaScript pipeline went - checked rendered PATHS for surviving
placeholders but never rendered CONTENT, and its glue fixture declared no
`generates` at all while omitting the optional roll-up keys instead of carrying
them as "" the way the intent actually writes them. The fixture exercised a
descriptor shape no real generation produces.
So, besides the two fixes:
- ModelGenerationIT now rejects an unresolved ${reference} and a dangling
member access in any generated .java (${JavaTask} excluded - that delegate
expression is a deliberate literal). Verified red on the unfixed code.
- orders.glue is faithful to the intent's output: `generates` with a local and
a cross-model source, the attachment family, businessKeyProperty,
criteriaExpression, roll-ups carrying "" optionals, and one populated
capacity/balance/status roll-up. Completing it surfaced three more unresolved
references immediately.
- RollupAggregatesTest pins empty vs populated vs absent optionals.
Verified: IntentEmissionCoverageIT green (73s), ModelGenerationIT green, 72
unit tests in ide-template green, formatter and release-javadoc clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Master is red:
IntentEmissionCoverageITfails on both DB legs since #6707 (run 31720936125, and every run after it). The reported failure is aConditionTimeoutonGET /services/java/emission-test/gen/emission/api/account/AccountControlleranswering 404 - which is nowhere near the cause.What actually broke
Two defects in the Java generation pipeline, both in generated glue code:
GlueGenerator.bindGeneratenever boundfromGenFolder/fromProjectName. Velocity renders an undefined reference verbatim, soGenerate.java.templateemitted a literalgen.${fromGenFolder}.data.... The JavaScript this replaced computed both (generateUtils.js:crossModelSource ? the source model's folder : this project's).RollupAggregatestestedcapacityField != nullwhere the JavaScript tested truthiness.GlueIntentGeneratorwrites""for an unused optional field, and""is notnull, so an unused capacity emittedparent. == null ? … : parent. ;- a member access with no member.Why that 404s an unrelated controller: client Java compiles the whole registry in one javac task. A single unparseable generated file unregisters every controller in the application, so the symptom lands on an endpoint that has nothing to do with roll-ups or create-from. Worth remembering the next time a generated app 404s for no reason.
Why the tests let it through
ModelGenerationITreplaced the parity oracle when the JavaScript pipeline was deleted, and it checked rendered paths for surviving placeholders - never rendered content. And itsorders.gluefixture declared nogeneratesblock at all, and omitted the optional roll-up keys rather than carrying them as""the way the intent really writes them. The fixture exercised a descriptor shape no generation produces.Changes
ModelGenerationITrejects an unresolved${reference}and a dangling member access in any generated.java.${JavaTask}is excluded - that Flowable delegate expression is a deliberate literal. Verified red on the unfixed code, naming${fromGenFolder}and${fromProjectName};orders.gluemade faithful to the intent's output:generateswith a local and a cross-model source, the attachment family,businessKeyProperty,criteriaExpression, roll-ups carrying""optionals, plus one populated capacity/balance/status roll-up. Completing it surfaced three more unresolved references immediately (all fixture gaps, now closed);RollupAggregatesTestpins empty vs populated vs absent optionals - red without the fix, 0.06s.Verification
formatter:validate and the release-profile javadoc check are clean.
Follow-up worth considering (not in this PR)
Velocity renders an undefined reference verbatim, which is how a forgotten context key becomes a Java syntax error instead of a missing value. Turning on
runtime.references.strictfor the model pipeline would make that fail loudly at generation time. It needs a sweep first - some templates may rely on an absent optional rendering as nothing - so it does not belong in a master-red fix.