davsclaus commented on code in PR #27841:
URL: https://github.com/apache/camel/pull/27841#discussion_r4291702413
##########
core/camel-core-catalog/src/main/java/org/apache/camel/catalog/impl/AbstractCamelCatalog.java:
##########
@@ -1185,7 +1185,10 @@ public ConfigurationPropertiesValidationResult
validateConfigurationProperty(Str
}
ConfigurationPropertiesValidationResult result = new
ConfigurationPropertiesValidationResult();
- boolean accept = acceptConfigurationPropertyKey(key);
+ // Load the main model once here to avoid repeated JSON parsing in
acceptConfigurationPropertyKey
+ // and matchMainModelGroup (both called for camel.* keys that are not
component/dataformat/language).
+ MainModel cachedMainModel = key != null && key.startsWith("camel.") ?
mainModel() : null;
Review Comment:
This loads the main model for `camel.component.*`, `camel.dataformat.*` and
`camel.language.*` keys as well (the comment above says it does not), but
`acceptConfigurationPropertyKey` returns true for those before it looks at the
model. Since `mainModel()` parses the JSON on each call, every component line
now parses the main metadata. Skipping those prefixes keeps the single load for
main-group keys:
```suggestion
MainModel cachedMainModel = key != null && key.startsWith("camel.")
&& !key.startsWith("camel.component.")
&& !key.startsWith("camel.dataformat.")
&& !key.startsWith("camel.language.")
? mainModel() : null;
```
##########
dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/ConfigurationValidateToolsTest.java:
##########
@@ -172,14 +172,114 @@ 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();
+ }
+
+ @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();
}
Review Comment:
Dash-insensitive group matching is the main change of this round, but no
test covers it. `camel.errorRegistry.enabled` exists in the main metadata, so
something like:
```suggestion
}
@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();
}
```
--
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]
