davsclaus commented on code in PR #26619: URL: https://github.com/apache/camel/pull/26619#discussion_r4056478970
########## dsl/camel-yaml-dsl/camel-yaml-dsl-common/src/main/java/org/apache/camel/dsl/yaml/common/DataFormatKeyHints.java: ########## @@ -0,0 +1,145 @@ +/* + * 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.dsl.yaml.common; + +import java.util.Collection; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +/** + * The hint for a {@code marshal}/{@code unmarshal} key that names a data format the way its artifact or the catalog + * does (jackson, json-jackson, jackson-xml, snake-yaml) instead of by its YAML key (json with library Jackson, + * jacksonXml, yaml). + * <p> + * The YAML key is the data format's model name. Ten data formats in the catalog have a name of their own that differs + * from it, because one model serves several libraries or types: {@link #ALIASES} lists them with the option that + * selects the library or type. The table mirrors the catalog ({@code name} and {@code modelName} of each data format, + * and the enum of the {@code library} or {@code type} option of the model); the validator's tests check the two agree. + * The deserializer cannot read the catalog at runtime, which is why the table is here rather than derived. + */ +public final class DataFormatKeyHints { + + /** The YAML key of an aliased data format and the option that selects it: json with library Jackson. */ + public record Alias(String key, String option, String value) { + + /** The key with the option, as it is written in YAML: json: {library: Jackson}, or yaml: {...}. */ + public String form() { + return option != null ? key + ": {" + option + ": " + value + "}" : key + ": {...}"; + } + } + + /** + * The catalog data formats whose name is not their YAML key, by name in lower case with no separators, so that + * jackson, Jackson, snake-yaml and snakeYaml all match. + */ + public static final Map<String, Alias> ALIASES = Map.ofEntries( + Map.entry("jackson", new Alias("json", "library", "Jackson")), + Map.entry("gson", new Alias("json", "library", "Gson")), + Map.entry("jsonb", new Alias("json", "library", "Jsonb")), + Map.entry("fastjson", new Alias("json", "library", "Fastjson")), + Map.entry("avrojackson", new Alias("avro", "library", "Jackson")), + Map.entry("protobufjackson", new Alias("protobuf", "library", "Jackson")), + Map.entry("snakeyaml", new Alias("yaml", null, null)), + Map.entry("bindycsv", new Alias("bindy", "type", "Csv")), + Map.entry("bindyfixed", new Alias("bindy", "type", "Fixed")), + Map.entry("bindykvp", new Alias("bindy", "type", "KeyValue"))); + + /** The models an alias belongs to, which people prefix or suffix the library with: json-jackson, jackson-json. */ + private static final List<String> MODELS = List.of("json", "avro", "protobuf", "yaml", "bindy"); + + private DataFormatKeyHints() { + } + + /** + * The hint for a key that is not a data format key, or null when the key is neither a spelling of one of the known + * keys nor an alias. + * + * @param key the key as written: jackson, json-jackson, jackson-xml, JSON + * @param knownKeys the data format keys of marshal/unmarshal: json, jacksonXml, yaml... + * @return the hint: did you mean 'jacksonXml'? for a spelling of a key, the data format is json, Jackson + * is its library: write json: {library: Jackson} for an alias, or null + */ + public static String hint(String key, Collection<String> knownKeys) { + String normalized = normalize(key); + // jackson-xml, JSON, base-64: the key itself, spelled differently + for (String known : knownKeys) { + if (!known.equals(key) && normalized.equals(normalize(known))) { + return "did you mean '" + known + "'?"; + } + } + Alias alias = alias(normalized); Review Comment: Applied in e01357868ca7: `hint()` now passes the raw `key` and `alias()` is the only place that normalizes. _Claude Code on behalf of davsclaus_ ########## dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/DataFormatKeyHintsCatalogTest.java: ########## @@ -0,0 +1,85 @@ +/* + * 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.dsl.yaml.validator; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.camel.catalog.CamelCatalog; +import org.apache.camel.catalog.DefaultCamelCatalog; +import org.apache.camel.dsl.yaml.common.DataFormatKeyHints; +import org.apache.camel.tooling.model.DataFormatModel; +import org.apache.camel.tooling.model.EipModel; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * CAMEL-24847: the alias table the runtime deserializer uses without a catalog must agree with the catalog: every data + * format whose name differs from its model name is in it, with the model name as the key and a value of the option that + * selects the library or type. + */ +public class DataFormatKeyHintsCatalogTest { + + private final CamelCatalog catalog = new DefaultCamelCatalog(); + + @Test + public void testEveryAliasedDataFormatOfTheCatalogIsInTheTable() { + List<String> aliased = new ArrayList<>(); + for (String name : catalog.findDataFormatNames()) { + DataFormatModel df = catalog.dataFormatModel(name); + if (df.getModelName().equals(name)) { + assertThat(DataFormatKeyHints.alias(name)).as(name + " is its own key").isNull(); + continue; + } + aliased.add(name); + DataFormatKeyHints.Alias alias = DataFormatKeyHints.alias(name); + assertThat(alias).as(name + " maps to " + df.getModelName()).isNotNull(); + assertThat(alias.key()).as(name).isEqualTo(df.getModelName()); + EipModel model = catalog.eipModel(df.getModelName()); + EipModel.EipOptionModel selector = model.getOptions().stream() + .filter(o -> o.getEnums() != null && o.getEnums().size() > 1 + && (o.getName().equals("library") || o.getName().equals("type"))) + .findFirst().orElse(null); + if (selector == null) { + assertThat(alias.option()).as(name + " has no library or type to select").isNull(); + } else { + assertThat(alias.option()).as(name).isEqualTo(selector.getName()); + assertThat(selector.getEnums()).as(name + " " + selector.getName()).contains(alias.value()); + } + } + assertThat(DataFormatKeyHints.ALIASES).as("aliases without a catalog data format: " + aliased) + .hasSize(aliased.size()); Review Comment: Applied in e01357868ca7: the test now also asserts `ALIASES.keySet()` is a subset of the normalized catalog names (`normalize` made public for that), so a misspelled key fails the test rather than keeping the sizes equal. _Claude Code on behalf of davsclaus_ ########## dsl/camel-yaml-dsl/camel-yaml-dsl/src/test/groovy/org/apache/camel/dsl/yaml/UnmarshalTest.groovy: ########## @@ -128,4 +129,31 @@ class UnmarshalTest extends YamlTestSupport { 'true', 'false', null ] } + + // CAMEL-24847: a data format named as its artifact or catalog entry says which key and option to write + def "unmarshal with #key fails with a message naming the data format key"(String key, String hint) { + when: + loadRoutes([ResourceHelper.fromString("route-1.yaml", """ + - from: + uri: timer:tick + steps: + - unmarshal: + ${key}: {} + """.stripIndent())], false) + then: + def e = thrown(Exception) + def messages = [] + for (Throwable t = e; t != null; t = t.cause) { + messages << t.message + } + messages.any { it != null && it.contains("Error constructing YAML node id: unmarshal: unsupported field: ${key}") && it.contains(hint) } + where: + key | hint + 'jackson' | 'the data format is json, Jackson is its library: write json: {library: Jackson}' + 'json-jackson' | 'write json: {library: Jackson}' + 'gson' | 'write json: {library: Gson}' + 'bindy-csv' | 'the data format is bindy, Csv is its type: write bindy: {type: Csv}' + 'snake-yaml' | 'the data format is yaml: write yaml: {...}' + 'JSON' | "did you mean 'json'?" Review Comment: Applied in e01357868ca7: the `where:` table is parametrized on the EIP and has two `marshal` rows (`jackson` and `JSON`). I did not use `jackson-xml` for `marshal` as suggested-adjacent cases might imply: the runtime already accepts the dash form via `dashToCamelCase` in the generated `setProperty`, so it never reaches the unsupported-field path (only the validator's schema rejects it, which `YamlValidatorPropertyHintTest` covers). _Claude Code on behalf of davsclaus_ -- 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]
