davsclaus commented on code in PR #27841:
URL: https://github.com/apache/camel/pull/27841#discussion_r4295602446
##########
dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/ConfigurationValidateToolsTest.java:
##########
@@ -172,14 +172,157 @@ void multipleLinesAggregated() {
}
@Test
- void nonCamelKeyMarkedNotAccepted() {
+ void nonCamelKeyMarkedNotChecked() {
+ // Non-Camel keys (e.g. server.port) are not checked by this catalog:
checked=false,
+ // valid=false, and counted in summary.unchecked (not in
summary.errors or summary.valid).
ConfigurationValidateTools tools = createTools();
ConfigurationValidateTools.ConfigurationValidateResult result
= tools.camel_configuration_validate("server.port=8080", null,
null, null);
ConfigurationValidateTools.PropertyValidationInfo info =
result.properties().get(0);
- assertThat(info.accepted()).isFalse();
+ assertThat(info.checked()).isFalse();
+ assertThat(info.valid()).isFalse();
+ assertThat(result.summary().unchecked()).isEqualTo(1);
+ assertThat(result.summary().errors()).isZero();
+ assertThat(result.summary().valid()).isZero();
+ }
+
+ // --- CAMEL-25538 regression tests ---
+
+ @Test
+ void camelServerPortIsAcceptedAndValid() {
+ // camel.server.* was previously not in the hardcoded prefix list — it
must now be
+ // accepted and validated against the main-model metadata.
+ ConfigurationValidateTools tools = createTools();
+
+ ConfigurationValidateTools.ConfigurationValidateResult result
+ = tools.camel_configuration_validate("camel.server.port=9999",
null, null, null);
+
+ assertThat(result.summary().total()).isEqualTo(1);
+ ConfigurationValidateTools.PropertyValidationInfo info =
result.properties().get(0);
+ assertThat(info.checked()).isTrue();
+ assertThat(info.valid()).isTrue();
+ assertThat(info.errors()).isZero();
+ assertThat(info.issues()).isEmpty();
+ }
+
+ @Test
+ void camelServerInventedKeyIsCheckedButNotValid() {
+ // An invented key under an accepted camel.server.* group must be
reported as unknown,
+ // not silently passed as valid.
+ ConfigurationValidateTools tools = createTools();
+
+ ConfigurationValidateTools.ConfigurationValidateResult result
+ =
tools.camel_configuration_validate("camel.server.bindPort=9999", null, null,
null);
+
+ ConfigurationValidateTools.PropertyValidationInfo info =
result.properties().get(0);
+ assertThat(info.checked()).isTrue();
+ assertThat(info.valid()).isFalse();
+ assertThat(info.issues()).anyMatch(i -> "unknown".equals(i.kind()));
+ }
+
+ @Test
+ void uncheckedKeyImpliesValidFalseAndNotCountedInErrors() {
+ // Invented key outside any known camel.* group: checked=false AND
valid=false.
+ // Before the fix, valid was true despite accepted=false (CAMEL-25538
bug #2).
+ // The key must not appear in summary.errors — it is unchecked, not
rejected.
+ ConfigurationValidateTools tools = createTools();
+
+ ConfigurationValidateTools.ConfigurationValidateResult result
+ =
tools.camel_configuration_validate("camel.invented.completely.unknown=foo",
null, null, null);
+
+ ConfigurationValidateTools.PropertyValidationInfo info =
result.properties().get(0);
+ assertThat(info.checked()).isFalse();
+ assertThat(info.valid()).isFalse();
+ // The summary must not count unchecked keys as valid or as errors
+ assertThat(result.summary().valid()).isZero();
+ assertThat(result.summary().errors()).isZero();
+ assertThat(result.summary().unchecked()).isEqualTo(1);
+ }
+
+ @Test
+ void camelManagementKeyIsCheckedAndValid() {
+ // camel.management.* was also missing from the old hardcoded list.
+ ConfigurationValidateTools tools = createTools();
+
+ ConfigurationValidateTools.ConfigurationValidateResult result
+ =
tools.camel_configuration_validate("camel.management.enabled=true", null, null,
null);
+
+ ConfigurationValidateTools.PropertyValidationInfo info =
result.properties().get(0);
+ assertThat(info.checked()).isTrue();
+ assertThat(info.valid()).isTrue();
+ }
+
+ @Test
+ void camelVaultAwsKeyIsCheckedAndValid() {
+ // camel.vault.aws.* is a nested group (3-segment name) — must also be
accepted.
+ // The dot-offset logic that skips the group prefix is exercised here.
+ ConfigurationValidateTools tools = createTools();
+
+ ConfigurationValidateTools.ConfigurationValidateResult result
+ =
tools.camel_configuration_validate("camel.vault.aws.accessKey=AKIA1234", null,
null, null);
+
+ ConfigurationValidateTools.PropertyValidationInfo info =
result.properties().get(0);
+ assertThat(info.checked()).isTrue();
+ assertThat(info.valid()).isTrue();
+ assertThat(info.issues()).isEmpty();
+ }
+
+ @Test
+ void camelVaultAwsInventedKeyIsCheckedButNotValid() {
+ // An invented option under a nested group (camel.vault.aws.*)
exercises the dot-offset
+ // logic: the option name starts after the 3rd dot and must be checked
against the model.
+ ConfigurationValidateTools tools = createTools();
+
+ ConfigurationValidateTools.ConfigurationValidateResult result
+ =
tools.camel_configuration_validate("camel.vault.aws.inventedOption=foo", null,
null, null);
+
+ ConfigurationValidateTools.PropertyValidationInfo info =
result.properties().get(0);
+ assertThat(info.checked()).isTrue();
+ assertThat(info.valid()).isFalse();
+ }
+
+ @Test
+ void dashedGroupNameIsCheckedAndValid() {
+ // camel-main binds camel.error-registry.* to the camel.errorRegistry
group
+ ConfigurationValidateTools tools = createTools();
+
+ ConfigurationValidateTools.ConfigurationValidateResult result
+ =
tools.camel_configuration_validate("camel.error-registry.enabled=true", null,
null, null);
+
+ ConfigurationValidateTools.PropertyValidationInfo info =
result.properties().get(0);
+ assertThat(info.checked()).isTrue();
+ assertThat(info.valid()).isTrue();
+ }
+
+ @Test
+ void springBootStarterOnlyOptionIsUnchecked() {
+ // camel.metrics.uriTagEnabled exists in camel-micrometer-starter but
not in the
+ // camel-main metadata the catalog uses — it must not be reported as
an error for
+ // runtime=spring-boot.
+ ConfigurationValidateTools tools = createTools();
+
+ ConfigurationValidateTools.ConfigurationValidateResult result
+ = tools.camel_configuration_validate(
+ "camel.metrics.uriTagEnabled=true", "spring-boot",
null, null);
+
+ ConfigurationValidateTools.PropertyValidationInfo info =
result.properties().get(0);
+ assertThat(info.checked()).isFalse();
+ assertThat(info.valid()).isFalse();
+ assertThat(result.summary().unchecked()).isEqualTo(1);
+ assertThat(result.summary().errors()).isZero();
Review Comment:
The narrowing in 006252680b / be91daafd2 has no test, so a later change
could bring back the broad downgrade without any test failing. It would help to
assert here that the downgraded line keeps its issues:
```suggestion
assertThat(result.summary().errors()).isZero();
assertThat(info.issues()).anyMatch(i -> "unknown".equals(i.kind()));
```
and to add a test where an original group stays fully checked under Spring
Boot, including the dashed form:
```java
@Test
void springBootKeepsValidationForOriginalGroups() {
ConfigurationValidateTools tools = createTools();
ConfigurationValidateTools.ConfigurationValidateResult result =
tools.camel_configuration_validate(
"camel.component.kafka.brokerz=x\ncamel.fault-tolerance.timeoutEnabledz=true",
"spring-boot", null, null);
assertThat(result.properties()).allSatisfy(info -> {
assertThat(info.checked()).isTrue();
assertThat(info.valid()).isFalse();
assertThat(info.issues()).anyMatch(i -> "unknown".equals(i.kind()));
});
assertThat(result.summary().unchecked()).isZero();
}
```
##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/ConfigurationValidateTools.java:
##########
@@ -105,16 +130,66 @@ public ConfigurationValidateResult
camel_configuration_validate(
}
private PropertyValidationInfo toPropertyValidationInfo(
- int lineNumber, String text,
ConfigurationPropertiesValidationResult result) {
- boolean valid = result.isSuccess();
- boolean accepted = result.isAccepted();
- int errors = result.getNumberOfErrors();
- int warnings = result.getNumberOfWarnings();
+ int lineNumber, String text,
ConfigurationPropertiesValidationResult result, boolean isSpringBoot) {
+ boolean checked = result.isAccepted();
+ // A checked line is valid when the catalog found no errors; an
unchecked line
+ // (not recognised as a Camel property) is neither valid nor invalid —
it is simply
+ // not inspected by this catalog.
+ boolean valid = checked && result.isSuccess();
+
+ // For Spring Boot, the catalog uses camel-main metadata which does
not include
+ // starter-specific options (e.g. camel.metrics.uriTagEnabled from
camel-micrometer-starter).
+ // When the only issues are "unknown option" errors AND the key
belongs to a group that is
+ // newly accepted by this fix (not one of the original 10 hardcoded
prefixes), downgrade to
+ // unchecked to avoid false positives — the starter may legitimately
bind options outside the
+ // main schema. Keys under the original 10 groups (component,
dataformat, language, main,
+ // resilience4j, faulttolerance, threadpool, health, lra, rest) retain
full validation since
+ // the catalog has always covered them.
+ boolean springBootDowngraded = false;
+ if (isSpringBoot && checked && !valid) {
+ String propertyKey = text != null && text.contains("=") ?
text.substring(0, text.indexOf('=')).trim() : text;
+ // Normalize to dash-free lowercase so that Spring Boot
relaxed-binding forms
+ // (e.g. camel.fault-tolerance.*, camel.thread-pool.*) match the
original group
+ // prefixes the same way the catalog normalises them internally.
+ String normKey = propertyKey != null ? propertyKey.replace("-",
"").toLowerCase(java.util.Locale.ENGLISH) : null;
Review Comment:
Nit: please import `java.util.Locale` instead of writing it fully qualified
(CLAUDE.md: no FQCNs outside generated code):
```suggestion
String normKey = propertyKey != null ? propertyKey.replace("-",
"").toLowerCase(Locale.ENGLISH) : null;
```
(plus `import java.util.Locale;`)
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc:
##########
@@ -13,6 +13,37 @@ See the xref:camel-upgrade-recipes-tool.adoc[documentation]
page for details.
== Upgrading Camel 4.23 to 4.24
+=== camel-report-maven-plugin:validate / configuration-validate
+
+The catalog now accepts all `camel.*` groups defined in
`camel-main-configuration-metadata.json`
+(previously only 10 groups out of 36 were recognized). As a result,
+`camel-report-maven-plugin:validate` and the camel-jbang MCP
`camel_configuration_validate` tool
+now validate approximately 29 additional `camel.*` groups (including
`camel.server.*`,
+`camel.management.*`, `camel.vault.*`, `camel.ssl.*`, and others).
+
+If your project uses Spring Boot and has options such as
`camel.metrics.uriTagEnabled` (from a
+Camel starter) that are not in the camel-main schema,
`camel-report-maven-plugin:validate` may
+report them as "Unknown option". See below for details.
Review Comment:
Nothing below follows up on "See below for details": the next paragraphs are
about dash matching, `failOnError` and the MCP response format. Maybe end with
the consequence instead: "...may report them as "Unknown option", which fails
the build when `failOnError=true`. The `camel_configuration_validate` MCP tool
with `runtime=spring-boot` reports such keys as `checked=false` instead."
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc:
##########
@@ -13,6 +13,37 @@ See the xref:camel-upgrade-recipes-tool.adoc[documentation]
page for details.
== Upgrading Camel 4.23 to 4.24
+=== camel-report-maven-plugin:validate / configuration-validate
+
+The catalog now accepts all `camel.*` groups defined in
`camel-main-configuration-metadata.json`
+(previously only 10 groups out of 36 were recognized). As a result,
Review Comment:
Small inaccuracy: before, 7 of the 36 groups were recognised. The other 3
old prefixes (`camel.component.`, `camel.dataformat.`, `camel.language.`) are
not main groups. Maybe: "(previously only 7 of the 36 groups were recognized,
besides `camel.component.*`, `camel.dataformat.*` and `camel.language.*`)".
Line 21 could then say "29 additional" without "approximately".
--
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]
