luigidemasi commented on PR #27373: URL: https://github.com/apache/camel/pull/27373#issuecomment-5996426967
The shared validator change is now in [CAMEL-25347](https://issues.apache.org/jira/browse/CAMEL-25347), #27383, and removed from this PR's net diff. The companion PR explicitly documents replacing the CAMEL-25238 workaround; its five original regression tests remain unchanged and pass. It corrects `ParamDefinition.type` metadata and regenerates the published schemas instead of altering a schema inside the validator. For the other review points: - **TypeSafe validation:** retained the adapter-level check because callers can invoke the SPI's `validate` or `evaluate` directly, without `SemanticLanguage`. The existing direct-call tests verify capability limits are rejected before transport initialization. The repeated language/adapter check is deliberate. - **Bean rebinding:** documented the compiled-instance lifetime and added a regression for both `expert` and `default-expert`. Rebinding leaves an existing expression on its original bean; a new expression or replacement question declaration resolves the replacement. - **XML ordering:** extended `SemanticXmlAutoDiscoveryTest.mainLoadsOrdinaryXmlWithoutLoaderRegistration` to cover a consuming resource listed before a separate `.semantic.xml` file. Both ordinary XML and `.semantic.xml` declarations are covered. Resource loading finishes before route configuration, so declarations are available when expressions initialize. - **Lock ordering:** I found no direct reverse acquisition in the framework paths inspected. Declaration reads use the volatile immutable snapshot without taking the `SemanticQuestions` monitor; expression compilation releases its monitor before `setValidator`; reload callbacks prepare candidates without taking the expression monitor. Provider callbacks still run under framework locks, so this is not a blanket guarantee about locks introduced by custom provider constructors or lifecycle callbacks. - **Diagnostics:** `SemanticLanguage.expert()` already wraps capability/validation failures with the question name and resolved expert name, and runtime input errors also identify the expert. Those paths are covered by `SemanticExpertTest`. I have not added a `toString()` just for logging. The enum fix is covered in the inline reply. Local validation passed: 198 semantic tests, 187 TypeSafe AI tests, and the full 698-module root build with tests skipped. The companion PR reports its YAML/validator regression coverage separately. _Generated by Codex on behalf of @luigidemasi via /oss-address-review._ -- 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]
