gnodet-bot commented on code in PR #27792: URL: https://github.com/apache/camel/pull/27792#discussion_r4280459789
########## components/camel-test/camel-test-citrus/src/main/java/org/apache/camel/component/citrus/CitrusEndpoint.java: ########## @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.component.citrus; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.camel.Category; +import org.apache.camel.Exchange; +import org.apache.camel.component.mock.MockEndpoint; +import org.apache.camel.spi.Metadata; +import org.apache.camel.spi.UriEndpoint; +import org.apache.camel.spi.UriParam; +import org.apache.camel.spi.UriPath; +import org.citrusframework.CitrusContext; +import org.citrusframework.message.MessagePayloadBuilder; +import org.citrusframework.validation.context.ValidationContext; + +@UriEndpoint(firstVersion = "4.24.0", scheme = "citrus", title = "Citrus", syntax = "citrus:operation/name", + producerOnly = true, remote = false, category = { Category.TESTING }) +public class CitrusEndpoint extends MockEndpoint { + + @UriPath(description = "The operation to perform. 'validate' validates exchanges without modifying them." + + " 'receive' validates exchanges and returns a simulated response.", + enums = "validate,receive") + @Metadata(required = true) + private String operation; + + @UriParam(description = "The message type for validation. Determines which Citrus validation module is used." + + " Supported values: json, xml, yaml, plaintext.") + private String messageType; + + private volatile CitrusContext citrusContext; + private volatile AtomicInteger nextSpecIndex = new AtomicInteger(0); Review Comment: ⚠️ **Misleading `volatile` on `AtomicInteger` field** `AtomicInteger` is already thread-safe for value operations, but the `volatile` here is needed because `reset()` replaces the *reference* (assigns a brand-new `AtomicInteger(0)`). The simpler and clearer design is to make the field `final` and call `set(0)` in `reset()` — no reference replacement, no `volatile` needed: ```suggestion private final AtomicInteger nextSpecIndex = new AtomicInteger(0); ``` ########## components/camel-test/camel-test-citrus/src/main/java/org/apache/camel/component/citrus/CitrusEndpoint.java: ########## @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.component.citrus; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.camel.Category; +import org.apache.camel.Exchange; +import org.apache.camel.component.mock.MockEndpoint; +import org.apache.camel.spi.Metadata; +import org.apache.camel.spi.UriEndpoint; +import org.apache.camel.spi.UriParam; +import org.apache.camel.spi.UriPath; +import org.citrusframework.CitrusContext; +import org.citrusframework.message.MessagePayloadBuilder; +import org.citrusframework.validation.context.ValidationContext; + +@UriEndpoint(firstVersion = "4.24.0", scheme = "citrus", title = "Citrus", syntax = "citrus:operation/name", + producerOnly = true, remote = false, category = { Category.TESTING }) +public class CitrusEndpoint extends MockEndpoint { + + @UriPath(description = "The operation to perform. 'validate' validates exchanges without modifying them." + + " 'receive' validates exchanges and returns a simulated response.", + enums = "validate,receive") + @Metadata(required = true) + private String operation; + + @UriParam(description = "The message type for validation. Determines which Citrus validation module is used." + + " Supported values: json, xml, yaml, plaintext.") + private String messageType; + + private volatile CitrusContext citrusContext; + private volatile AtomicInteger nextSpecIndex = new AtomicInteger(0); + + public CitrusEndpoint(String endpointUri, CitrusComponent component) { + super(endpointUri, component); + } + + // --- Lifecycle --- + + @Override + protected void doStart() throws Exception { + super.doStart(); + citrusContext = CitrusContext.create(); + } + + @Override + protected void doStop() throws Exception { + if (citrusContext != null) { + citrusContext.close(); + citrusContext = null; + } + super.doStop(); + } + + // --- Fluent validation specification API --- + + public MessageValidationSpec validateAll() { + MessageValidationSpec messageValidationSpec = new MessageValidationSpec(); + whenAnyExchangeReceived(createProcessor(messageValidationSpec)); + return messageValidationSpec; + } + + public MessageValidationSpec validate() { + return validate(nextSpecIndex.incrementAndGet()); + } + + public MessageValidationSpec validate(int index) { + MessageValidationSpec messageValidationSpec = new MessageValidationSpec(); + whenExchangeReceived(index, createProcessor(messageValidationSpec)); + return messageValidationSpec; + } + + private CitrusValidationProcessor createProcessor(MessageValidationSpec validationSpec) { + return new CitrusValidationProcessor(citrusContext, messageType, validationSpec); + } + + private CitrusValidationProcessor createProcessor( + MessageValidationSpec validationSpec, + MessageResponseSpec responseSpec) { + return new CitrusValidationProcessor(citrusContext, messageType, validationSpec, responseSpec); + } + + // --- Gherkin-style API (for 'receive' operation) --- + + public WhenSpec when() { + return new WhenSpec(); + } + + @Override + public void reset() { + super.reset(); + nextSpecIndex = new AtomicInteger(0); + } + + @Override + protected void performAssertions(Exchange exchange, Exchange copy) throws Exception { + super.performAssertions(exchange, copy); + + // tryProcessing() in the superclass swallows processor exceptions and sets them on the + // exchange instead of adding them to the failures list. Capture them here so + // assertIsSatisfied() can report Citrus validation errors properly. + Exception e = exchange.getException(); + if (e != null && !getFailures().contains(e)) { + getFailures().add(e); + if ("validate".equals(operation)) { + // For validate-only mode, clear the exception so it does not propagate + // through the route — validation errors are collected, not thrown. + exchange.setException(null); + } + } + } + + // --- Getters and setters --- + + public String getOperation() { + return operation; + } + + public void setOperation(String operation) { + this.operation = operation; + } + + public String getMessageType() { + return messageType; + } + + public void setMessageType(String messageType) { + this.messageType = messageType; + } + + // --- Gherkin-style inner classes --- + + public class WhenSpec { + + public WhenMessageValidationSpec receive() { + return receive(nextSpecIndex.incrementAndGet()); + } + + public WhenMessageValidationSpec receive(int index) { + WhenMessageValidationSpec messageValidationSpec = new WhenMessageValidationSpec(index); + whenExchangeReceived(index, createProcessor(messageValidationSpec)); Review Comment: ⚠️ **Double registration at the same index — first processor is silently overwritten** `receive(int index)` registers a validation-only processor via `whenExchangeReceived(index, ...)`. When the caller then invokes `ThenSpec.respond()`, it registers a *second* processor at the same index via `whenExchangeReceived(index, ...)`, overwriting the first. The first call is therefore always wasted. Consider skipping the initial registration in `receive(int)` and only registering in `respond()`, or redesigning to compose into the existing processor rather than replacing it. ########## components/camel-test/camel-test-citrus/src/main/java/org/apache/camel/component/citrus/CitrusValidationProcessor.java: ########## @@ -0,0 +1,223 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.component.citrus; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +import org.apache.camel.Exchange; +import org.apache.camel.Processor; +import org.apache.camel.StreamCache; +import org.citrusframework.CitrusContext; +import org.citrusframework.context.TestContext; +import org.citrusframework.message.DefaultMessage; +import org.citrusframework.message.MessagePayloadBuilder; +import org.citrusframework.message.MessagePayloadUtils; +import org.citrusframework.message.MessageType; +import org.citrusframework.validation.MessageValidator; +import org.citrusframework.validation.MessageValidatorRegistry; +import org.citrusframework.validation.context.DefaultMessageValidationContext; +import org.citrusframework.validation.context.HeaderValidationContext; +import org.citrusframework.validation.context.ValidationContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class CitrusValidationProcessor implements Processor { + + private static final Logger LOG = LoggerFactory.getLogger(CitrusValidationProcessor.class); + + private final CitrusContext citrusContext; + private final String messageType; + private final MessageValidationSpec spec; + private final MessageResponseSpec responseSpec; + + CitrusValidationProcessor(CitrusContext citrusContext, String messageType, + MessageValidationSpec validationSpec) { + this(citrusContext, messageType, validationSpec, null); + } + + CitrusValidationProcessor(CitrusContext citrusContext, String messageType, + MessageValidationSpec validationSpec, + MessageResponseSpec responseSpec) { + this.citrusContext = citrusContext; + this.messageType = messageType; + this.spec = validationSpec; + this.responseSpec = responseSpec; + } + + @Override + public void process(Exchange exchange) throws Exception { + TestContext testContext = citrusContext.createTestContext(); + propagateExchangeVariables(exchange, testContext); + + try { + if (spec != null) { + DefaultMessage receivedMessage = buildReceivedMessage(exchange); + DefaultMessage controlMessage = buildControlMessage(spec, testContext); + + String resolvedType = resolveMessageType(spec); + controlMessage.setType(resolvedType); + + List<ValidationContext> validationContexts = buildValidationContexts(spec); + performCitrusValidation(receivedMessage, controlMessage, testContext, validationContexts, resolvedType); + } + + if (responseSpec != null) { + applyResponse(exchange, responseSpec, testContext); + } + } finally { + writeBackExtractedVariables(exchange, testContext); + } + } + + // --- Citrus validation helpers --- + + private DefaultMessage buildReceivedMessage(Exchange exchange) { + Object body = exchange.getIn().getBody(); + if (body instanceof StreamCache sc) { + body = exchange.getIn().getBody(String.class); + sc.reset(); + } + Map<String, Object> headers = new LinkedHashMap<>(exchange.getIn().getHeaders()); + return new DefaultMessage(body, headers); + } + + private DefaultMessage buildControlMessage(MessageValidationSpec spec, TestContext testContext) { + Object resolvedBody = resolveBody(spec.getBody(), spec.getBodyBuilder(), testContext); + DefaultMessage controlMessage = resolvedBody != null + ? new DefaultMessage(resolvedBody) + : new DefaultMessage(); + + for (Map.Entry<String, Object> entry : spec.getHeaders().entrySet()) { + Object value = entry.getValue(); + if (value instanceof String s) { + value = testContext.replaceDynamicContentInString(s); + } + controlMessage.setHeader(entry.getKey(), value); + } + return controlMessage; + } + + private String resolveMessageType(MessageValidationSpec spec) { + if (messageType != null) { + if (MessageType.knows(messageType)) { + return MessageType.valueOf(messageType.toUpperCase(Locale.ROOT)).name(); + } else { + return messageType.toUpperCase(Locale.ROOT); + } + } + + String expectedBody = spec.getBody(); + if (expectedBody != null) { + if (MessagePayloadUtils.isJson(expectedBody)) { + return MessageType.JSON.name(); + } else if (MessagePayloadUtils.isXml(expectedBody)) { + return MessageType.XML.name(); + } else if (MessagePayloadUtils.isYaml(expectedBody)) { + return MessageType.YAML.name(); + } + } + return MessageType.PLAINTEXT.name(); + } + + private List<ValidationContext> buildValidationContexts(MessageValidationSpec spec) { + List<ValidationContext> contexts = new ArrayList<>(); + contexts.add(new DefaultMessageValidationContext()); + if (!spec.getHeaders().isEmpty()) { + contexts.add(new HeaderValidationContext.Builder().ignoreCase(true).build()); + } + for (ValidationContext.Builder<?, ?> builder : spec.getValidationContextBuilders()) { + contexts.add(builder.build()); + } + return contexts; + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private void performCitrusValidation( + DefaultMessage receivedMessage, DefaultMessage controlMessage, + TestContext testContext, List<ValidationContext> validationContexts, + String resolvedType) { + + MessageValidatorRegistry registry = testContext.getMessageValidatorRegistry(); + List<MessageValidator<? extends ValidationContext>> validators + = registry.findMessageValidators(resolvedType, receivedMessage); + + if (validators.isEmpty()) { + throw new IllegalStateException( + "No Citrus message validator found for message type '" + resolvedType + + "'. Make sure to add the required Citrus validation module " + + "to the test classpath (e.g. citrus-validation-json, " + + "citrus-validation-xml, citrus-validation-text)."); + } + + LOG.debug("Validating exchange with {} Citrus validator(s) for message type '{}'", + validators.size(), resolvedType); + + for (MessageValidator validator : validators) { + validator.validateMessage(receivedMessage, controlMessage, testContext, validationContexts); + } + } + + private void applyResponse(Exchange exchange, MessageResponseSpec responseSpec, TestContext testContext) { + Object resolvedBody = resolveBody(responseSpec.getBody(), responseSpec.getBodyBuilder(), testContext); + if (resolvedBody != null) { + exchange.getIn().setBody(resolvedBody); + } + Map<String, Object> resolvedHeaders = testContext.resolveDynamicValuesInMap(responseSpec.getHeaders()); + resolvedHeaders.forEach((key, value) -> exchange.getIn().setHeader(key, value)); + } + + private Object resolveBody(String body, MessagePayloadBuilder bodyBuilder, TestContext testContext) { + if (bodyBuilder != null) { + return bodyBuilder.buildPayload(testContext); + } + if (body != null) { + return testContext.replaceDynamicContentInString(body); + } + return null; + } + + // --- Variable propagation --- + + private void propagateExchangeVariables(Exchange exchange, TestContext testContext) { + for (Map.Entry<String, Object> entry : exchange.getProperties().entrySet()) { + if (entry.getValue() != null) { + testContext.setVariable(entry.getKey(), entry.getValue()); + } + } + for (Map.Entry<String, Object> entry : exchange.getVariables().entrySet()) { + if (entry.getValue() != null) { + testContext.setVariable(entry.getKey(), entry.getValue()); + } + } + } + + private void writeBackExtractedVariables(Exchange exchange, TestContext testContext) { + for (Map.Entry<String, Object> entry : testContext.getVariables().entrySet()) { + String key = entry.getKey(); + if (key.startsWith("citrus.") || key.startsWith("CITRUS_")) { + continue; + } + if (!exchange.getProperties().containsKey(key) && !exchange.getVariables().containsKey(key)) { + exchange.setVariable(key, entry.getValue()); Review Comment: ⚠️ **`writeBackExtractedVariables` silently drops values when a key already exists** The guard `if (!exchange.getProperties().containsKey(key) && !exchange.getVariables().containsKey(key))` skips writing back any extracted variable whose key already exists in the exchange — even if the value was updated by the Citrus validation step. This will silently break test scenarios that expect an extracted variable to be overwritten. If the intent is to avoid polluting user-set exchange properties, the check should be documented; if it's unintentional, remove the guard. ########## components/camel-test/camel-test-citrus/src/main/java/org/apache/camel/component/citrus/CitrusEndpoint.java: ########## @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.component.citrus; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.camel.Category; +import org.apache.camel.Exchange; +import org.apache.camel.component.mock.MockEndpoint; +import org.apache.camel.spi.Metadata; +import org.apache.camel.spi.UriEndpoint; +import org.apache.camel.spi.UriParam; +import org.apache.camel.spi.UriPath; +import org.citrusframework.CitrusContext; +import org.citrusframework.message.MessagePayloadBuilder; +import org.citrusframework.validation.context.ValidationContext; + +@UriEndpoint(firstVersion = "4.24.0", scheme = "citrus", title = "Citrus", syntax = "citrus:operation/name", + producerOnly = true, remote = false, category = { Category.TESTING }) +public class CitrusEndpoint extends MockEndpoint { + + @UriPath(description = "The operation to perform. 'validate' validates exchanges without modifying them." + + " 'receive' validates exchanges and returns a simulated response.", + enums = "validate,receive") + @Metadata(required = true) + private String operation; + + @UriParam(description = "The message type for validation. Determines which Citrus validation module is used." + + " Supported values: json, xml, yaml, plaintext.") + private String messageType; + + private volatile CitrusContext citrusContext; + private volatile AtomicInteger nextSpecIndex = new AtomicInteger(0); + + public CitrusEndpoint(String endpointUri, CitrusComponent component) { + super(endpointUri, component); + } + + // --- Lifecycle --- + + @Override + protected void doStart() throws Exception { + super.doStart(); + citrusContext = CitrusContext.create(); + } + + @Override + protected void doStop() throws Exception { + if (citrusContext != null) { + citrusContext.close(); + citrusContext = null; + } + super.doStop(); + } + + // --- Fluent validation specification API --- + + public MessageValidationSpec validateAll() { + MessageValidationSpec messageValidationSpec = new MessageValidationSpec(); + whenAnyExchangeReceived(createProcessor(messageValidationSpec)); + return messageValidationSpec; + } + + public MessageValidationSpec validate() { + return validate(nextSpecIndex.incrementAndGet()); + } + + public MessageValidationSpec validate(int index) { + MessageValidationSpec messageValidationSpec = new MessageValidationSpec(); + whenExchangeReceived(index, createProcessor(messageValidationSpec)); + return messageValidationSpec; + } + + private CitrusValidationProcessor createProcessor(MessageValidationSpec validationSpec) { + return new CitrusValidationProcessor(citrusContext, messageType, validationSpec); + } + + private CitrusValidationProcessor createProcessor( + MessageValidationSpec validationSpec, + MessageResponseSpec responseSpec) { + return new CitrusValidationProcessor(citrusContext, messageType, validationSpec, responseSpec); + } + + // --- Gherkin-style API (for 'receive' operation) --- + + public WhenSpec when() { + return new WhenSpec(); + } + + @Override + public void reset() { + super.reset(); + nextSpecIndex = new AtomicInteger(0); Review Comment: 💡 **If `nextSpecIndex` is made `final` (see above), update `reset()` to call `set(0)` instead of allocating a new instance:** ```suggestion nextSpecIndex.set(0); ``` -- 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]
