[
https://issues.apache.org/jira/browse/CAMEL-24986?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18118740#comment-18118740
]
Claus Ibsen commented on CAMEL-24986:
-------------------------------------
The first attempt (PR #26831) is reverted in main: 7aec6a1a2b9b.
It failed {{RestProducerPathTest.testMissingHeader}} in *camel-core*, which I
had not run - I ran camel-rest and camel-rest-openapi, and the test lives
elsewhere. The behaviour it breaks is deliberate and documented in two places:
* {{testMissingHeader}}: "Backward compatibility: if one of the params is
resolved", the same wording as the comment in {{RestProducer}} - a partly
resolved template keeps the rest of the placeholders and is sent;
* {{testNoHeaders}}: a template where nothing resolved is not an error either,
it simply sets no {{REST_HTTP_URI}}.
The second of those is exactly the shape this issue is about
({{/api/stock/\{sku\}/reserve}} with no value for {{sku}}), so failing the
exchange contradicts a documented behaviour rather than fixing an oversight.
h3. What to do instead
Warn rather than fail. When the prepared uri still holds a {{\{name\}}}, log a
WARN naming the parameter and where its value comes from, and send the request
as before. Both behaviours above are kept, nothing breaks, and the reason
appears next to the 404 instead of nowhere. In the benchmark the log is read
constantly ({{camel_get_log}} was called 315 times over 360 steps), so a
warning does reach the reader.
The second half of this issue is unchanged: the YAML validator can say it
before the route runs, from the OpenAPI specification it already reads for
CAMEL-24844.
> camel-rest: a REST producer sends the request when a path parameter has no
> value, with the placeholder still in the uri
> -----------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-24986
> URL: https://issues.apache.org/jira/browse/CAMEL-24986
> Project: Camel
> Issue Type: Improvement
> Components: camel-rest-openapi, rest
> Reporter: Claus Ibsen
> Priority: Major
>
> A REST producer sends the request even when a path parameter has no value, so
> the call goes out with the placeholder still in it:
> {noformat}
> org.apache.camel.http.base.HttpOperationFailedException: HTTP operation
> failed invoking
> http://localhost:8080/api/stock/%7Bsku%7D/reserve with statusCode: 404
> {noformat}
> {{%7Bsku%7D}} is {{\{sku\}}}. Nothing says the parameter was not resolved;
> the caller sees a 404 from the server and starts looking at the server.
> h3. Where it comes from
> {{RestProducer.prepareExchange}} resolves each path segment with
> {{resolvePlaceholders}}, which reads a header and falls back to an exchange
> variable:
> {code:java}
> String value = exchange.getMessage().getHeader(key, String.class);
> if (value == null) {
> value = exchange.getVariable(key, String.class);
> }
> if (value != null) {
> res = res.substring(0, startIndex) + value + res.substring(endIndex + 1);
> }
> {code}
> When neither has a value the placeholder is left as it is, and the request is
> sent anyway. {{hasPath}} is set only when at least one segment resolved, so a
> fully unresolved template falls through to the endpoint uri unchanged, and a
> partly resolved one carries the rest into {{REST_HTTP_URI}}.
> h3. What to do
> # *Fail instead of sending it.* After the resolution loop, when the template
> still holds a {{\{name\}}}, throw and say which parameter it is and where the
> value comes from, e.g. "the path parameter \{sku\} of
> /api/stock/\{sku\}/reserve has no value: set a header sku, or an exchange
> variable, before the call". {{prepareUriTemplate=false}} remains the way to
> send a uri with braces on purpose.
> # *Say it before it runs.* The YAML validator knows the OpenAPI specification
> already - {{OpenApiVerbs}} reads it for CAMEL-24844 phase B - so for a
> {{rest-openapi}} producer it can see that {{reserveStock}} needs a {{sku}}
> path parameter and that no step before the call sets that header, and report
> it at write time.
> h3. How often it happens
> Measured on an overnight local-model benchmark (22 examples, 5 passes, 360
> steps): this single mistake produced *2580* of the runtime errors, the most
> of any cause, and appeared in *13 of 15 runs* across the three HTTP examples.
> The model does set the header in the route; the value does not arrive, and
> nothing in the failure says so. A person reading a 404 has the same problem.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
