codeconsole commented on PR #16134:
URL: https://github.com/apache/grails-core/pull/16134#issuecomment-5309175614
Thanks — most of this held up and is fixed. Three findings didn't reproduce:
**#6 — no NPE.** With a verified-null lookup and the guard removed,
`MissingMethodException` is thrown either way; the dynamic call on the null
receiver yields no tag library rather than throwing. I kept a guard so the null
case is stated rather than incidental, and `GroovyPageMethodMissingSpec` pins
the behaviour. Happy to drop it.
**#28 — the reflective spec already exists.** `TagDiscoveryRulesSpec` runs
each row through `AstTagMethodView` *and* `ReflectedTagMethodView` and asserts
they agree. The three specific rule changes weren't pinned, though — they're
now rows in that matrix, and the runtime-discovery change is in the description.
**#9 — `isMethod()` doesn't exist**; the accessor is `isBindable()`. The
point stood: nothing branches on `Kind`, and the javadoc claimed it decided
whether a call could be resolved, which is false. Corrected. `findTagNames`,
`getIncompleteNamespaces` and `getAmbiguousTagNames` are gone. I kept
`getTagNamesForClass` and `isClassDescribed` — spec-only, but each states an
invariant nothing else does. Say if you want them gone.
**#12** — added `--configuration-cache` to
`TagLibraryIndexWiringFunctionalSpec` rather than refactor on suspicion. The
build stores and reuses an entry, so no change needed.
**#15** — writing 32 tag libraries concurrently lost 29 manifest entries.
Now guarded by a monitor plus a file lock, with a spec that fails without
either.
Fixing it exposed a bug it had been masking. With the index complete,
`g:form` became resolvable and this compiled into a tag invocation:
```groovy
request.withFormat {
form multipartForm { redirect book }
}
```
`form` is a format in the DSL, resolved by the closure's delegate — not a
tag. It broke `POST /book/save` with `Tag [form] cannot be invoked without a
tag library lookup`. Unqualified calls inside closures are no longer rewritten;
namespaced ones still are, so tag bodies keep the fast path.
#1 was real and not hypothetical — `grails-fields` declares `f:with`, and
`with` is a DGM method on every object. Unqualified rewriting now reserves
every name the metaclass answers to.
On the blocking labels: #2 and #3 both degrade to dynamic dispatch, which is
what shipped, so they cost the optimisation and made the docs wrong rather than
changing behaviour. Both fixed or documented, with a spec pinning the
`@Artefact('Controller')` case.
Agreed on not splitting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
