oscerd commented on PR #3087:
URL: https://github.com/apache/camel-kamelets/pull/3087#issuecomment-5998042501
Checked the two mechanisms this rests on, because both are the kind that can
look right and silently do nothing. Both work. Details below, plus one
documentation suggestion and one caveat about what I did **not** test.
## Verified: `precondition: true` really does drop the guard when the cap is
unset
Reproduced the pattern in a throwaway action Kamelet and ran all three cases:
| cap | body | branch in route? | outcome |
|---|---|---|---|
| 5 | 10 bytes | yes | `IllegalArgumentException: too big (10 > 5)` |
| 50 | 10 bytes | yes | passes through |
| unset (0) | 10 bytes | **no** | passes through |
In the third case the branch's log statement never fires at all, so the
guard is genuinely absent from the built route rather than
evaluated-and-skipped. That is what the description claims and it holds.
**One thing worth a YAML comment, because it surprised me.** The
precondition uses `${properties:maxDocumentSize:0}`, and that expression
resolves *differently* depending on when it runs:
```
ENTER len=10 maxProp=[${properties:maxDocumentSize:0}] -> 0
placeholder=[{{maxDocumentSize}}] -> 50
```
At **route-template instantiation**, which is when `precondition: true`
evaluates it, `${properties:...}` sees the template parameter and returns 50 —
which is why the guard is correctly included. At **runtime** the same
expression returns 0, because the template-local binding is no longer in scope
as a property. It does not matter here, since the precondition has already done
its job and the inner comparison uses `{{maxDocumentSize}}`, but the asymmetry
is invisible from the YAML. A future editor could reasonably "tidy" the
precondition to use the runtime-correct form, or move the check out of the
precondition, and silently disable the cap for everyone. Worth a line saying
the precondition is build-time-only.
## Verified: `length()` does not consume the payload
The claim that the guard "adds no copy of the payload" would be worth little
if `${length()}` drained the body before the parse. It does not — file source,
cap 50, 10-byte document:
```
BEFORE type=org.apache.camel.component.file.GenericFile
ENTER len=10
PASSED
AFTER type=org.apache.camel.component.file.GenericFile value=[0123456789]
```
Body intact afterwards.
**Caveat, stated because the test does not cover it:** that exercises a
`GenericFile` body, not a raw `InputStream`. Camel JBang has stream caching on
by default, so the realistic file-source path is fine, but a deployment with
stream caching disabled and a genuine non-cached `InputStream` body is the case
I have not tested. If that is reachable through these actions it would be worth
one more test; if it is not, worth saying so.
## Smaller notes
- Not trusting `CamelFileLength` is the right call — it is caller-supplied
by the time it reaches an action, and a guard that believes it is not a guard.
- `0` as "no limit" rather than absent-means-no-limit keeps the property
always resolvable, which is what makes the `${properties:...}` precondition
work at all. Consistent with the `maxDocumentSize` already on
`langchain4j-ingest-sink`.
- Throwing `IllegalArgumentException` naming the document id matches how the
sink reports an oversized document, so the two read the same way in a log.
- All checks green, including the 28-minute integration run.
Nothing blocking from me. The precondition comment is the only thing I would
ask for, and it is a comment rather than a behaviour change.
---
_Claude Code on behalf of Andrea Cosentino_
--
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]
