allthingssecurity commented on code in PR #27078:
URL: https://github.com/apache/camel/pull/27078#discussion_r4132910696
##########
core/camel-core-model/src/main/java/org/apache/camel/model/RouteDefinitionHelper.java:
##########
@@ -157,7 +157,9 @@ public static void forceAssignIds(CamelContext context,
List<RouteDefinition> ro
VerbDefinition verb = findVerbDefinition(context, rest,
route.getInput().getEndpointUri());
if (verb != null) {
String id =
context.resolvePropertyPlaceholders(verb.getId());
- if (verb.hasCustomIdAssigned() &&
ObjectHelper.isNotEmpty(id) && !customIds.contains(id)) {
+ if (verb.hasCustomIdAssigned() &&
ObjectHelper.isNotEmpty(id)) {
Review Comment:
This also makes a verb fail when its id equals a non-rest route's id, e.g.
`.get().id("hello").to("direct:hello")` with
`from("direct:hello").routeId("hello")` and `inlineRoutes(false)`. That started
on the merge base and now fails with `Duplicate id detected: hello`. Intended?
If so, the upgrade-guide bullet should cover it.
_Claude Code on behalf of allthingssecurity_
##########
core/camel-core-model/src/main/java/org/apache/camel/model/rest/RestDefinition.java:
##########
@@ -1198,6 +1198,20 @@ private void addRouteDefinition(
} else {
binding.setEnableNoContentResponse(getEnableNoContentResponse());
}
+ // the body parameter from the type must be added before the
parameters are registered on the binding,
+ // so a required body is enforced
+ if (verb.getType() != null) {
+ String bodyType = parseText(camelContext, verb.getType());
+ ParamDefinition param = findParam(verb,
RestParamType.body.name());
Review Comment:
Looked up by name, so a body param declared as
`name("payload").type(body).required(false)` is not found, and a second
required `body` param is added. With `clientRequestValidation`, an empty body
is now rejected (400). Match on `RestParamType.body` instead?
_Claude Code on behalf of allthingssecurity_
##########
core/camel-core/src/test/java/org/apache/camel/component/rest/RestDslEdgeCasesTest.java:
##########
@@ -128,4 +129,119 @@ public void configure() {
assertThat(out.getMessage().getHeader(Exchange.HTTP_RESPONSE_CODE)).isNull();
assertMockEndpointsSatisfied();
}
+
+ @Test
+ public void testClientResponseValidationWithBindingOff() throws Exception {
+ context.addRoutes(new RouteBuilder() {
+ @Override
+ public void configure() {
+
restConfiguration().host("localhost").clientResponseValidation(true);
+ rest("/h").get()
+
.responseMessage().code(200).header("X-Id").endHeader().endResponseMessage()
+ .to("direct:h");
+
+ from("direct:h").setBody(constant("Hello"));
+ }
+ });
+ context.start();
+
+ // the response is validated also when binding is off
+ Exchange out = template.request("seda:get-h", e -> {
+ });
+
assertThat(out.getMessage().getHeader(Exchange.HTTP_RESPONSE_CODE)).isEqualTo(500);
+ assertThat(out.getMessage().getBody(String.class)).isEqualTo("Some of
the response HTTP headers are missing.");
+ }
+
+ @Test
+ public void testClientResponseValidationHeadersPerResponseCode() throws
Exception {
+ context.addRoutes(new RouteBuilder() {
+ @Override
+ public void configure() {
+
restConfiguration().host("localhost").clientResponseValidation(true);
+ rest("/c").get()
+
.responseMessage().code(200).header("X-Id").endHeader().endResponseMessage()
+ .responseMessage().code(404).message("Not
found").endResponseMessage()
+ .to("direct:c");
+
+ from("direct:c")
+ .choice()
+ .when(header("found"))
+ .setHeader("X-Id",
constant("123")).setBody(constant("Found"))
+ .otherwise()
+ .setHeader(Exchange.HTTP_RESPONSE_CODE,
constant(404)).setBody(constant("Not found"));
+ }
+ });
+ context.start();
+
+ // X-Id is only required on the 200 response
+ Exchange out = template.request("seda:get-c", e -> {
+ });
+ System.out.println(
Review Comment:
Leftover debug print.
_Claude Code on behalf of allthingssecurity_
--
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]
