This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch feature/CAMEL-24715-table-driven-hints in repository https://gitbox.apache.org/repos/asf/camel.git
commit 0f3ed59153b443d33f411b0dd333fb67b50c2d71 Author: Claus Ibsen <[email protected]> AuthorDate: Wed Sep 16 09:25:38 2026 +0200 CAMEL-24715: camel-yaml-dsl-validator - the hints are rows of five tables (step, expression, list, property, compact notation) with one matcher, not branches in four methods; a drift test cross-checks the hardcoded lists of the camel-jbang checks against the catalog, and drops the two Camel 3 names it found in the bean-reference fallback Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Signed-off-by: Claus Ibsen <[email protected]> --- .../dsl/jbang/core/commands/ai/BeanRefChecks.java | 8 +- .../core/commands/ai/ChecksCatalogDriftTest.java | 123 ++++++ .../camel/dsl/yaml/validator/SchemaHints.java | 412 ++++++++++++++++++++ .../camel/dsl/yaml/validator/YamlValidator.java | 425 +-------------------- .../camel/dsl/yaml/validator/SchemaHintsTest.java | 127 ++++++ 5 files changed, 684 insertions(+), 411 deletions(-) diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/BeanRefChecks.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/BeanRefChecks.java index f67f9a3d6440..2134a18d1e85 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/BeanRefChecks.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/BeanRefChecks.java @@ -76,13 +76,11 @@ final class BeanRefChecks { } /** - * Without a catalog: the options whose bean must implement an interface, and which (a subset of the EIP models). + * Without a catalog: the options whose bean must implement an interface, and which (a subset of what + * {@link #requiredType(CamelCatalog, String)} reads from the EIP models; ChecksCatalogDriftTest keeps it one). */ static final Map<String, String> REQUIRED_TYPES = Map.of( - "aggregationStrategy", "org.apache.camel.AggregationStrategy", - "strategyRef", "org.apache.camel.AggregationStrategy", - "processorRef", "org.apache.camel.Processor", - "processor", "org.apache.camel.Processor"); + "aggregationStrategy", "org.apache.camel.AggregationStrategy"); private static final Map<CamelCatalog, Map<String, String>> REQUIRED_TYPES_BY_CATALOG = java.util.Collections.synchronizedMap(new java.util.WeakHashMap<>()); diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ChecksCatalogDriftTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ChecksCatalogDriftTest.java new file mode 100644 index 000000000000..e5bd473fea8d --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/ChecksCatalogDriftTest.java @@ -0,0 +1,123 @@ +/* + * 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.jbang.core.commands.ai; + +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; + +import org.apache.camel.Exchange; +import org.apache.camel.catalog.CamelCatalog; +import org.apache.camel.catalog.DefaultCamelCatalog; +import org.apache.camel.tooling.model.ComponentModel; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * CAMEL-24715: the write-time checks carry a few lists the catalog has no metadata for. Each states something about a + * component or an EIP that a later release can change; this cross-checks them against the catalog and the API so they + * cannot drift silently. A failure names every entry that drifted. + */ +class ChecksCatalogDriftTest { + + private static CamelCatalog catalog; + + @BeforeAll + static void loadCatalog() { + catalog = new DefaultCamelCatalog(); + } + + /** + * The premise of every entry is that the component does not have that option: the day it does, the hint tells the + * user to remove or rename an option that works. + */ + @Test + void inventedOptionsAreNotComponentOptions() { + List<String> drifted = new ArrayList<>(); + for (String key : new TreeSet<>(EndpointChecks.INVENTED_OPTIONS.keySet())) { + String scheme = key.substring(0, key.indexOf(':')); + String option = key.substring(scheme.length() + 1); + ComponentModel model = catalog.componentModel(scheme); + if (model == null) { + drifted.add(key + ": no component " + scheme + " in the catalog"); + continue; + } + Set<String> options = new TreeSet<>(); + model.getEndpointOptions().forEach(o -> options.add(o.getName())); + model.getComponentOptions().forEach(o -> options.add(o.getName())); + if (options.contains(option)) { + drifted.add(key + ": " + option + " is a real option of " + scheme + " now, drop the entry"); + } + } + assertThat(drifted).as("INVENTED_OPTIONS entries the catalog contradicts").isEmpty(); + } + + /** + * The exchange properties of the timer are the Exchange.TIMER_* constants the consumer sets; a name the catalog + * lists as a header of the component is a header, and the hint would send the user to the wrong place. + */ + @Test + void exchangePropertiesAreTheApiConstantsAndNotHeaders() throws Exception { + Set<String> timerConstants = new TreeSet<>(); + for (Field f : Exchange.class.getFields()) { + if (f.getName().startsWith("TIMER_") && Modifier.isStatic(f.getModifiers()) && f.getType() == String.class) { + timerConstants.add((String) f.get(null)); + } + } + assertThat(timerConstants).isNotEmpty(); + List<String> drifted = new ArrayList<>(); + for (Map.Entry<String, List<String>> entry : HeaderChecks.EXCHANGE_PROPERTIES.entrySet()) { + String scheme = entry.getKey(); + ComponentModel model = catalog.componentModel(scheme); + if (model == null) { + drifted.add(scheme + ": no such component in the catalog"); + continue; + } + Set<String> headers = new TreeSet<>(); + model.getEndpointHeaders().forEach(h -> headers.add(h.getName())); + for (String name : entry.getValue()) { + if ("timer".equals(scheme) && !timerConstants.contains(name)) { + drifted.add(scheme + ": " + name + " is not an Exchange.TIMER_* constant"); + } + if (headers.contains(name)) { + drifted.add(scheme + ": " + name + " is a header in the catalog, not an exchange property"); + } + } + } + assertThat(drifted).as("EXCHANGE_PROPERTIES entries the API or the catalog contradicts").isEmpty(); + } + + /** The static fallback for a missing catalog must say what the catalog-derived map says. */ + @Test + void requiredTypesFallbackMatchesTheCatalog() { + List<String> drifted = new ArrayList<>(); + for (Map.Entry<String, String> entry : BeanRefChecks.REQUIRED_TYPES.entrySet()) { + String fromCatalog = BeanRefChecks.requiredType(catalog, entry.getKey()); + if (!entry.getValue().equals(fromCatalog)) { + drifted.add(entry.getKey() + ": the fallback says " + entry.getValue() + ", the catalog " + + (fromCatalog == null ? "has no such option" : "says " + fromCatalog)); + } + } + assertThat(drifted).as("REQUIRED_TYPES entries the catalog contradicts").isEmpty(); + } +} diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java new file mode 100644 index 000000000000..7e2c096c1cf6 --- /dev/null +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java @@ -0,0 +1,412 @@ +/* + * 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.text.MessageFormat; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.regex.Pattern; + +import com.fasterxml.jackson.databind.JsonNode; +import com.networknt.schema.Error; + +/** + * The hints the validator adds to the schema library's errors so they say what to write: one table per stage of the + * pipeline in {@link YamlValidator}, each row a keyword, a location pattern, a condition and the text. The first row + * that matches an error rewrites it; an error no row matches is kept as is. + */ +final class SchemaHints { + + /** What a row's condition and text see: the error and the pieces every row would otherwise recompute. */ + record Match(Error error, String location, String name, String message, String unknown, YamlValidator validator) { + + static Match of(Error error, YamlValidator validator) { + String location = String.valueOf(error.getInstanceLocation()); + String message = error.getMessage(); + return new Match( + error, location, location.substring(location.lastIndexOf('/') + 1), message, + message != null && "additionalProperties".equals(error.getKeyword()) + ? between(message, "property '", "'") : null, + validator); + } + + /** The key before the last segment: the EIP a when/0 or doCatch/0 item belongs to. */ + String parentName() { + String parent = location.substring(0, location.lastIndexOf('/')); + return parent.substring(parent.lastIndexOf('/') + 1); + } + + boolean nameIsIndex() { + return name.matches("\\d+"); + } + + boolean locationEndsWith(String suffix) { + return location.endsWith(suffix); + } + + String schemaLocation() { + return String.valueOf(error.getSchemaLocation()); + } + } + + /** + * A row of a table. The error's keyword and location select the row, the condition refines it, and the text is + * either appended to the error's message in parentheses, or replaces it. + */ + record Hint(String keyword, Pattern location, Predicate<Match> when, Function<Match, String> text, String outKeyword, + String messageKey, boolean append) { + + boolean matches(Match m) { + return keyword.equals(m.error().getKeyword()) + && (location == null || location.matcher(m.location()).matches()) + && when.test(m); + } + + Error rewrite(Match m) { + String hint = text.apply(m); + // the message is not a MessageFormat pattern (it contains braces), so pass it as the single argument + return Error.builder() + .keyword(outKeyword) + .instanceLocation(m.error().getInstanceLocation()) + .messageKey(messageKey) + .format(new MessageFormat("{0}")) + .arguments(append ? m.message() + " (" + hint + ")" : hint) + .build(); + } + } + + /** A row that keeps the error's message and appends the hint in parentheses. */ + static Hint append(String keyword, String location, Predicate<Match> when, Function<Match, String> text) { + return new Hint(keyword, location == null ? null : Pattern.compile(location), when, text, keyword, keyword, true); + } + + /** A row that replaces the error's message with the hint, reported under the given keyword and message key. */ + static Hint replace( + String keyword, String location, Predicate<Match> when, Function<Match, String> text, String outKeyword, + String messageKey) { + return new Hint( + keyword, location == null ? null : Pattern.compile(location), when, text, outKeyword, messageKey, + false); + } + + /** An additionalProperties row: the unknown property name is what the condition and the text are about. */ + static Hint unknownProperty(String location, Predicate<Match> when, Function<Match, String> text) { + return append("additionalProperties", location, m -> m.unknown() != null && when.test(m), text); + } + + private static final Predicate<Match> ANY = m -> true; + + /** + * Applies a table to the errors: the first matching row rewrites each error. Two rewrites that say the same at the + * same location (the branches of an anyOf, once the hint no longer names the branch) are reported once. + */ + static List<Error> apply(List<Hint> table, List<Error> errors, YamlValidator validator) { + List<Error> answer = new ArrayList<>(errors.size()); + Set<String> seen = new LinkedHashSet<>(); + for (Error error : errors) { + Error hinted = apply(table, error, validator); + if (hinted == error || seen.add(hinted.getInstanceLocation() + " " + hinted.getMessage())) { + answer.add(hinted); + } + } + return answer; + } + + static Error apply(List<Hint> table, Error error, YamlValidator validator) { + if (error.getMessage() == null) { + return error; + } + Match m = Match.of(error, validator); + for (Hint row : table) { + if (row.matches(m)) { + return row.rewrite(m); + } + } + return error; + } + + // ------------------------------------------------------------------------------------------------------------- + // step hints + // ------------------------------------------------------------------------------------------------------------- + + /** + * "must have at most 1 properties" at a step: a step holds one EIP, and the second key is either an option that + * belongs under the EIP (indented one level more) or another step (its own - item). + */ + static final List<Hint> STEP = List.of( + // - beans:\n myBean: ... : the second key was meant to be inside the first; it is not indented enough + append("maxProperties", "/\\d+", ANY, + m -> "a top-level entry is one key: - route:, - beans:, - rest:...; the lines that belong to it" + + " must be indented under it, a second key at the same level as the entry is read as a" + + " separate property"), + append("maxProperties", ".*/steps/\\d+", ANY, + m -> "a step is one EIP: an option of that EIP is indented under its key, and the next EIP is its" + + " own - item")); + + // ------------------------------------------------------------------------------------------------------------- + // expression hints + // ------------------------------------------------------------------------------------------------------------- + + private static final String EXPRESSION_SUB_ELEMENT = "ExpressionSubElementDefinition"; + + /** + * "string found, object expected" where the schema wants an expression: says to write it as a language map, + * constant for a fixed value and simple for a dynamic one. + */ + static final List<Hint> EXPRESSION = List.of( + replace("type", null, + m -> { + JsonNode instance = m.error().getInstanceNode(); + return instance != null && instance.isValueNode() + && (m.schemaLocation().contains(EXPRESSION_SUB_ELEMENT) + || String.valueOf(m.error().getEvaluationPath()).contains(EXPRESSION_SUB_ELEMENT)); + }, + m -> { + String value = m.error().getInstanceNode().asText(); + return String.format( + "a plain value (%s) found, an expression expected: write %s: {constant: {expression: \"%s\"}} for a fixed value, or %s: {simple: {expression: \"...\"}} for a dynamic one", + value, m.name(), value, m.name()); + }, + "type", "expression")); + + // ------------------------------------------------------------------------------------------------------------- + // list hints + // ------------------------------------------------------------------------------------------------------------- + + /** + * "object found, array expected" says what the schema wants, not how to write it: a list, each item starting with + * "- ". At the root of the file it also names the entries (route, from, beans, rest, onException). + */ + static final List<Hint> LIST = List.of( + // script: {language: groovy, text: ...}: the language is the key of the expression, not a property + append("type", ".*/language", m -> m.message().contains("object expected"), + m -> "an expression is written with the language as the key and its expression: property, e.g." + + " groovy: {expression: \"...\"}, simple: {expression: \"...\"}, constant: {expression: \"...\"};" + + " the language: form is language: {language: groovy, expression: \"...\"}"), + // message: {simple: "..."}: a string property that is already an expression, or a plain option + append("type", null, m -> m.message().contains("object found, string expected"), + m -> m.name() + " is a plain string" + + (m.locationEndsWith("/log/message") + ? " that is already a simple expression: write message: \"... ${body} ...\"" + : ": write " + m.name() + ": \"...\", not a language map")), + // - onException: [ ... ]: the entry is a map; several handlers are several - onException: items + append("type", + "/\\d+/(onException|onCompletion|intercept|interceptFrom|interceptSendToEndpoint|errorHandler|route|rest|restConfiguration)", + m -> m.message().contains("array found, object expected"), + m -> m.name() + " is a map, not a list: - " + m.name() + ": followed by its properties indented" + + (m.name().equals("onException") + ? " (exception: [java.lang.Exception], handled: {constant: {expression: \"true\"}}," + + " steps: [...])" + : "") + + "; several of them are several - " + m.name() + ": items"), + // otherwise: [- log: ...]: the block is a map whose steps: holds the list + append("type", ".*/(otherwise|doTry|doFinally|doCatch/\\d+)", m -> m.message().contains("object expected"), + m -> { + String eip = m.nameIsIndex() ? "doCatch" : m.name(); + return eip + " holds its EIPs under steps: " + eip + ": {steps: [- log: \"...\"]}" + + (eip.equals("doCatch") ? ", each - doCatch: item with exception: and steps:" : ""); + }), + append("type", "/?", m -> m.message().contains("array expected"), + m -> "a Camel YAML file is a list of entries, each starting with \"- \": - route:, - from:, - beans:," + + " - rest:, - onException:"), + append("type", ".*/beans", m -> m.message().contains("array expected"), + m -> "beans is a list: - name: myBean followed by type: \"#class:com.example.MyBean\" (indented" + + " under the -)"), + append("type", ".*/(steps|when|get|post|exception|doCatch)", m -> m.message().contains("array expected"), + m -> m.name() + " is a list: each item starts with \"- \""), + append("type", null, m -> m.message().contains("array expected"), + m -> "write it as a list: each item starts with \"- \"")); + + // ------------------------------------------------------------------------------------------------------------- + // property hints + // ------------------------------------------------------------------------------------------------------------- + + /** The options of the log component: written on the log EIP, they are reported as such. */ + private static final Set<String> LOG_COMPONENT_OPTIONS = Set.of( + "showAll", "showBody", "showBodyType", "showHeaders", "showExchangePattern", "showProperties", + "showAllProperties", "showVariables", "showExchangeId", "showException", "showCaughtException", + "showStackTrace", "showStreams", "showFiles", "showFuture", "showRouteId", "showRouteGroup", "multiline", + "maxChars", "skipBodyLineSeparator", "groupSize", "groupInterval", "groupDelay", "groupActiveOnly", + "level", "plain", "sourceLocationLoggerName", "style"); + + private static final Pattern CLASS_NAME = Pattern.compile("([a-z][\\w]*\\.)+[A-Z]\\w*"); + + /** + * Adds a hint to "property 'x' is not defined in the schema": the closest property name of that node (did you mean + * 'logName'?), or, when the property is a top-level entry such as onException written inside a route, where it goes + * instead. + */ + static final List<Hint> PROPERTY = List.of( + append("required", ".*/route/from", m -> m.message().contains("required property 'steps' not found"), + m -> "steps: is a property of from:, next to uri:; a steps: written at the route level must be" + + " indented under from:"), + // - id: myBean / class: ... : the bean properties are name and type + unknownProperty("/\\d+/beans/\\d+", m -> Set.of("id", "ref", "class").contains(m.unknown()), + m -> "a bean is - name: myBean followed by type: \"#class:com.example.MyBean\" (name instead of " + + m.unknown() + (m.unknown().equals("class") ? ", type instead of class" : "") + ")"), + // - myBean: {type: ...} instead of - name: myBean / type: ... + unknownProperty("/\\d+/beans/\\d+", ANY, + m -> "a bean item is written as - name: " + m.unknown() + + " followed by type: \"#class:com.example.MyBean\" (the name is a property, not the key)"), + unknownProperty(null, + m -> m.validator().topLevelEntries().contains(m.unknown()) + && m.location().chars().filter(c -> c == '/').count() >= 2, + m -> "'" + m.unknown() + "' is a top-level entry: write it as a list item at the same level as the" + + " route, not inside it"), + // onException: {java.lang.Exception: ...}: the class is a list item under exception: + unknownProperty(".*/(onException|doCatch/\\d+)", m -> CLASS_NAME.matcher(m.unknown()).matches(), + m -> "the exception class is a list item under exception: (" + + (m.locationEndsWith("/onException") ? "onException" : "doCatch") + ": {exception: [" + + m.unknown() + "], steps: [...]})"), + unknownProperty(".*/circuitBreaker", m -> m.unknown().equals("name"), + m -> "the circuit breaker's name is its id: circuitBreaker: {id: myBreaker, ...}"), + // circuitBreaker: {failureThreshold: 5}: the thresholds and timeouts are resilience4j configuration + unknownProperty(".*/circuitBreaker", m -> { + Set<String> resilience = m.validator().resilienceProperties(); + String lower = m.unknown().toLowerCase(Locale.ROOT); + return !resilience.isEmpty() + && (resilience.contains(m.unknown()) || YamlValidator.closest(m.unknown(), resilience) != null + || lower.contains("threshold") || lower.contains("timeout")); + }, m -> { + Set<String> resilience = m.validator().resilienceProperties(); + String best = resilience.contains(m.unknown()) ? m.unknown() : YamlValidator.closest(m.unknown(), resilience); + return "the thresholds, timeouts and the like are written under resilience4jConfiguration:" + + " (circuitBreaker: {resilience4jConfiguration: {" + (best != null ? best : "failureRateThreshold") + + ": ...}, steps: [...], onFallback: {steps: [...]}})"; + }), + unknownProperty(".*/log", m -> m.unknown().equals("level") || m.unknown().equals("logLevel"), + m -> "did you mean 'loggingLevel'?"), + // log: {message: ..., showHeaders: true}: those are options of the log component endpoint + unknownProperty(".*/log", m -> LOG_COMPONENT_OPTIONS.contains(m.unknown()), + m -> "'" + m.unknown() + "' is an option of the log component, not of the log EIP: write a to: step" + + " with uri: \"log:com.example?" + m.unknown() + "=...\" (the log EIP has message," + + " loggingLevel, logName, marker)"), + // - route: {from: {uri: ...}, steps: [...]}: steps belongs under from: + unknownProperty(".*/route", m -> m.unknown().equals("steps"), + m -> "steps: goes under from:, indented at the same level as uri: (route: {from: {uri: ...," + + " steps: [...]}})"), + // otherwise: {log: ...} or when: [- simple: ..., log: ...]: the EIPs go under steps: + unknownProperty(".*/(otherwise|when/\\d+|doTry|doCatch/\\d+|doFinally|split|filter|loop|aggregate" + + "|circuitBreaker|onFallback|multicast|pipeline|saga|resequence|throttle|delay" + + "|onException|onCompletion|intercept|interceptFrom|interceptSendToEndpoint|route|from)", + m -> m.validator().stepNames().contains(m.unknown()), + m -> { + String eip = m.nameIsIndex() ? m.parentName() : m.name(); + return "'" + m.unknown() + "' is a step: the steps of " + eip + " go under steps: (" + eip + + ": {steps: [- " + m.unknown() + ": ...]})"; + }), + // setBody: {script: ...}: script is an EIP; the language is the key of an expression + unknownProperty(null, + m -> m.unknown().equals("script") && !m.locationEndsWith("/steps") + && (YamlValidator.EXPRESSION_REQUIRED.contains(m.name()) || m.locationEndsWith("/expression")), + m -> "script is an EIP step, not a language: write the language as the key of the expression" + + " (expression: {groovy: {expression: \"...\"}}, expression: {simple: {expression: \"...\"}})," + + " or run a script as its own step with - script: {expression: {groovy: {expression: \"...\"}}}"), + // setBody: {bean: myBean} : the bean language is method: + unknownProperty(null, m -> m.unknown().equals("bean") && !m.locationEndsWith("/steps"), + m -> "the bean language is written as method: (expression: {method: {ref: myBean, method:" + + " process}}), or call the bean as a step with - bean: {ref: myBean, method: process}"), + // setHeader: {CamelNumberA: {simple: ...}} : the name is a property, not the key + unknownProperty(".*/(setHeader|setProperty|setVariable|removeHeader|removeProperty|removeVariable)", + m -> YamlValidator.closest(m.unknown(), m.validator().knownProperties(m.schemaLocation())) == null, + m -> "the name is a property: " + m.name() + ": {name: " + m.unknown() + + (m.name().startsWith("set") ? ", expression: {simple: {expression: \"...\"}}}" : "}") + + " (" + m.unknown() + " is not the key)"), + unknownProperty(".*/bean", m -> Set.of("parameters", "args", "arguments").contains(m.unknown()), + m -> "arguments are written in the method call: bean: {ref: myBean, method: \"process(${body}," + + " 'x')\"}"), + unknownProperty(null, + m -> YamlValidator.closest(m.unknown(), m.validator().knownProperties(m.schemaLocation())) != null, + m -> "did you mean '" + + YamlValidator.closest(m.unknown(), m.validator().knownProperties(m.schemaLocation())) + "'?")); + + // ------------------------------------------------------------------------------------------------------------- + // compact notation hints (canonical schema only) + // ------------------------------------------------------------------------------------------------------------- + + /** The property a step written as a string sets: the argument of the definition's String constructor. */ + private static final Map<String, String> STRING_STEP_PROPERTY = Map.ofEntries( + Map.entry("bean", "ref"), Map.entry("convertBodyTo", "type"), Map.entry("log", "message"), + Map.entry("poll", "uri"), Map.entry("removeHeader", "name"), Map.entry("removeHeaders", "pattern"), + Map.entry("removeProperties", "pattern"), Map.entry("removeProperty", "name"), + Map.entry("removeVariable", "name"), Map.entry("rollback", "message"), + Map.entry("setExchangePattern", "pattern"), Map.entry("to", "uri"), Map.entry("toD", "uri")); + + private static final String NORMALIZE_HINT = "; camel validate normalize rewrites a file in the canonical format"; + + private static final String COMPACT_NOTATION = "compactNotation"; + + /** + * The canonical schema rejects the compact notation as a schema error that says nothing about it: "property + * 'simple' is not defined" for a language key directly on the EIP, "string found, object expected" for a step or a + * language written as a string. Each is replaced with a message that names the notation, the canonical form of that + * line, and the normalize command. + */ + static final List<Hint> COMPACT = List.of( + // setBody: {simple: ...} or when: [- simple: ...]: the language key sits on the EIP, not under expression: + replace("additionalProperties", null, + m -> m.unknown() != null && m.validator().languageKeys().contains(m.unknown()), + m -> { + String form = m.validator().languageForm(m.unknown()); + if (m.nameIsIndex()) { + return "a " + m.parentName() + " item with " + m.unknown() + ": ... is the deprecated compact" + + " notation: an expression is written under expression: (- expression: {" + + m.unknown() + ": {" + form + "}})" + NORMALIZE_HINT; + } + return m.name() + ": {" + m.unknown() + ": ...} is the deprecated compact notation: an" + + " expression is written under expression: (" + m.name() + ": {expression: {" + + m.unknown() + ": {" + form + "}}})" + NORMALIZE_HINT; + }, + COMPACT_NOTATION, COMPACT_NOTATION), + // simple: "..." : the language is a map with its expression + replace("type", null, + m -> m.message().contains("string found, object expected") + && m.validator().languageKeys().contains(m.name()), + m -> m.name() + ": \"...\" is the deprecated compact notation: write " + m.name() + ": {" + + m.validator().languageForm(m.name()) + "}" + NORMALIZE_HINT, + COMPACT_NOTATION, COMPACT_NOTATION), + // log: "..." : the step is a map with its properties + replace("type", null, + m -> m.message().contains("string found, object expected") + && (m.validator().stepNames().contains(m.name()) + || m.validator().topLevelEntries().contains(m.name())), + m -> { + String property = STRING_STEP_PROPERTY.get(m.name()); + return m.name() + ": \"...\" is the deprecated compact notation: write " + m.name() + + (property != null ? ": {" + property + ": \"...\"}" : " as a map with its properties") + + NORMALIZE_HINT; + }, + COMPACT_NOTATION, COMPACT_NOTATION)); + + static String between(String text, String start, String end) { + int i = text.indexOf(start); + if (i < 0) { + return null; + } + int j = text.indexOf(end, i + start.length()); + return j < 0 ? null : text.substring(i + start.length(), j); + } + + private SchemaHints() { + } +} diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java index 366604c23b5a..c55e05948a58 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java @@ -268,12 +268,12 @@ public class YamlValidator { var errors = filterOneOfNoise(new ArrayList<>(schema.validate(target))); errors.removeIf(YamlValidator::isRuntimeAcceptedScalar); if (canonical) { - errors = withCompactNotationHints(errors); + errors = SchemaHints.apply(SchemaHints.COMPACT, errors, this); } - errors = withExpressionHints(errors); - errors = withPropertyHints(errors); - errors = withListHints(errors); - errors = withStepHints(errors); + errors = SchemaHints.apply(SchemaHints.EXPRESSION, errors, this); + errors = SchemaHints.apply(SchemaHints.PROPERTY, errors, this); + errors = SchemaHints.apply(SchemaHints.LIST, errors, this); + errors = SchemaHints.apply(SchemaHints.STEP, errors, this); // CAMEL-24707: the schema requires the expression, so a node without one fails its oneOf with "0 are valid" // plus one "required property <language> not found" per language; replace that with one line that says // what to write, at the node's own location @@ -306,82 +306,8 @@ public class YamlValidator { return errors; } - /** The property a step written as a string sets: the argument of the definition's String constructor. */ - private static final Map<String, String> STRING_STEP_PROPERTY = Map.ofEntries( - Map.entry("bean", "ref"), Map.entry("convertBodyTo", "type"), Map.entry("log", "message"), - Map.entry("poll", "uri"), Map.entry("removeHeader", "name"), Map.entry("removeHeaders", "pattern"), - Map.entry("removeProperties", "pattern"), Map.entry("removeProperty", "name"), - Map.entry("removeVariable", "name"), Map.entry("rollback", "message"), - Map.entry("setExchangePattern", "pattern"), Map.entry("to", "uri"), Map.entry("toD", "uri")); - - private static final String NORMALIZE_HINT = "; camel validate normalize rewrites a file in the canonical format"; - - /** - * The canonical schema rejects the compact notation as a schema error that says nothing about it: "property - * 'simple' is not defined" for a language key directly on the EIP, "string found, object expected" for a step or a - * language written as a string. Each is replaced with a message that names the notation, the canonical form of that - * line, and the normalize command. - */ - List<Error> withCompactNotationHints(List<Error> errors) { - List<Error> answer = new ArrayList<>(errors.size()); - for (Error error : errors) { - String hint = compactNotationHint(error); - if (hint == null) { - answer.add(error); - continue; - } - answer.add(Error.builder() - .keyword("compactNotation") - .instanceLocation(error.getInstanceLocation()) - .messageKey("compactNotation") - .format(new MessageFormat("{0}")) - .arguments(hint + NORMALIZE_HINT) - .build()); - } - return answer; - } - - private String compactNotationHint(Error error) { - String message = error.getMessage(); - if (message == null) { - return null; - } - String location = String.valueOf(error.getInstanceLocation()); - String name = location.substring(location.lastIndexOf('/') + 1); - if ("additionalProperties".equals(error.getKeyword())) { - // setBody: {simple: ...} or when: [- simple: ...]: the language key sits on the EIP, not under expression: - String unknown = between(message, "property '", "'"); - if (unknown == null || !languageKeys.contains(unknown)) { - return null; - } - if (name.matches("\\d+")) { - String parent = location.substring(0, location.lastIndexOf('/')); - name = parent.substring(parent.lastIndexOf('/') + 1); - return "a " + name + " item with " + unknown + ": ... is the deprecated compact notation: an expression" - + " is written under expression: (- expression: {" + unknown + ": {" + languageForm(unknown) - + "}})"; - } - return name + ": {" + unknown + ": ...} is the deprecated compact notation: an expression is written under" - + " expression: (" + name + ": {expression: {" + unknown + ": {" + languageForm(unknown) + "}}})"; - } - if ("type".equals(error.getKeyword()) && message.contains("string found, object expected")) { - if (languageKeys.contains(name)) { - // simple: "..." : the language is a map with its expression - return name + ": \"...\" is the deprecated compact notation: write " + name + ": {" + languageForm(name) - + "}"; - } - if (stepNames.contains(name) || topLevelEntries.contains(name)) { - // log: "..." : the step is a map with its properties - String property = STRING_STEP_PROPERTY.get(name); - return name + ": \"...\" is the deprecated compact notation: write " + name - + (property != null ? ": {" + property + ": \"...\"}" : " as a map with its properties"); - } - } - return null; - } - /** The canonical body of a language: its expression property, or token for tokenize, as key: "...". */ - private String languageForm(String language) { + String languageForm(String language) { JsonNode ref = model.at("/items/definitions/org.apache.camel.model.language.ExpressionDefinition/properties/" + language + "/$ref"); JsonNode properties = ref.isTextual() ? model.at(ref.asText().substring(1) + "/properties") : null; @@ -390,56 +316,12 @@ public class YamlValidator { return property + ": \"...\""; } - /** - * "must have at most 1 properties" at a step: a step holds one EIP, and the second key is either an option that - * belongs under the EIP (indented one level more) or another step (its own - item). - */ - static List<Error> withStepHints(List<Error> errors) { - List<Error> answer = new ArrayList<>(errors.size()); - for (Error error : errors) { - String location = String.valueOf(error.getInstanceLocation()); - if ("maxProperties".equals(error.getKeyword()) && location.matches("/\\d+")) { - // - beans:\n myBean: ... : the second key was meant to be inside the first; it is not indented enough - answer.add(Error.builder() - .keyword("maxProperties") - .instanceLocation(error.getInstanceLocation()) - .messageKey("maxProperties") - .format(new MessageFormat("{0}")) - .arguments(error.getMessage() + " (a top-level entry is one key: - route:, - beans:, - rest:...;" - + " the lines that belong to it must be indented under it, a second key at the same" - + " level as the entry is read as a separate property)") - .build()); - continue; - } - if ("maxProperties".equals(error.getKeyword()) && location.matches(".*/steps/\\d+")) { - answer.add(Error.builder() - .keyword("maxProperties") - .instanceLocation(error.getInstanceLocation()) - .messageKey("maxProperties") - .format(new MessageFormat("{0}")) - .arguments(error.getMessage() + " (a step is one EIP: an option of that EIP is indented under" - + " its key, and the next EIP is its own - item)") - .build()); - } else { - answer.add(error); - } - } - return answer; - } - /** * The EIPs whose expression the runtime needs: the schema leaves it optional for every expression node, and a split * written with only delimiter: "," (or a filter, setBody, when... with only options) fails when the route is * created with "Unsupported definition: null". */ - private static final Set<String> LOG_COMPONENT_OPTIONS = Set.of( - "showAll", "showBody", "showBodyType", "showHeaders", "showExchangePattern", "showProperties", - "showAllProperties", "showVariables", "showExchangeId", "showException", "showCaughtException", - "showStackTrace", "showStreams", "showFiles", "showFuture", "showRouteId", "showRouteGroup", "multiline", - "maxChars", "skipBodyLineSeparator", "groupSize", "groupInterval", "groupDelay", "groupActiveOnly", - "level", "plain", "sourceLocationLoggerName", "style"); - - private static final Set<String> EXPRESSION_REQUIRED = Set.of( + static final Set<String> EXPRESSION_REQUIRED = Set.of( "split", "filter", "when", "setBody", "setHeader", "setProperty", "setVariable", "transform", "loop", "delay", "recipientList", "routingSlip", "dynamicRouter", "validate", "script", "throttle", "resequence", "idempotentConsumer"); @@ -843,290 +725,30 @@ public class YamlValidator { return (instance.isNumber() || instance.isBoolean()) && isExpectedType(error, "string"); } - /** - * Replaces the schema's "boolean found, object expected" for a plain value at an option that takes an expression - * (such as {@code handled: true} on onException, or {@code completionSizeExpression: 10} on aggregate) with a - * message that shows the expression form, and drops the duplicates the schema composition produces for it. - */ - static List<Error> withExpressionHints(List<Error> errors) { - List<Error> answer = new ArrayList<>(errors.size()); - Set<String> seen = new LinkedHashSet<>(); - for (Error error : errors) { - Error hinted = withExpressionHint(error); - if (hinted == error || seen.add(hinted.getInstanceLocation() + " " + hinted.getMessage())) { - answer.add(hinted); - } - } - return answer; - } - - static Error withExpressionHint(Error error) { - if (!"type".equals(error.getKeyword())) { - return error; - } - JsonNode instance = error.getInstanceNode(); - if (instance == null || !instance.isValueNode()) { - return error; - } - String schemaLocation = String.valueOf(error.getSchemaLocation()); - String evaluationPath = String.valueOf(error.getEvaluationPath()); - if (!schemaLocation.contains(EXPRESSION_SUB_ELEMENT) && !evaluationPath.contains(EXPRESSION_SUB_ELEMENT)) { - return error; - } - String location = String.valueOf(error.getInstanceLocation()); - String name = location.substring(location.lastIndexOf('/') + 1); - String value = instance.asText(); - String message = String.format( - "a plain value (%s) found, an expression expected: write %s: {constant: {expression: \"%s\"}} for a fixed value, or %s: {simple: {expression: \"...\"}} for a dynamic one", - value, name, value, name); - // the message is not a MessageFormat pattern (it contains braces), so pass it as the single argument - return Error.builder() - .keyword("type") - .instanceLocation(error.getInstanceLocation()) - .messageKey("expression") - .format(new MessageFormat("{0}")) - .arguments(message) - .build(); - } - - private static final String EXPRESSION_SUB_ELEMENT = "ExpressionSubElementDefinition"; - private JsonNode model; private Set<String> topLevelEntries = Set.of(); private Set<String> languageKeys = Set.of(); private Set<String> stepNames = Set.of(); private Set<String> resilienceProperties = Set.of(); - /** - * "object found, array expected" says what the schema wants, not how to write it: a list, each item starting with - * "- ". At the root of the file it also names the entries (route, from, beans, rest, onException). - */ - static List<Error> withListHints(List<Error> errors) { - List<Error> answer = new ArrayList<>(errors.size()); - for (Error error : errors) { - answer.add(withListHint(error)); - } - return answer; + /** The keys of the file's entries (route, from, beans, rest, onException...), from the schema. */ + Set<String> topLevelEntries() { + return topLevelEntries; } - static Error withListHint(Error error) { - if (!"type".equals(error.getKeyword()) || error.getMessage() == null) { - return error; - } - String location = String.valueOf(error.getInstanceLocation()); - if (location.endsWith("/language") && error.getMessage().contains("object expected")) { - // script: {language: groovy, text: ...}: the language is the key of the expression, not a property - return Error.builder() - .keyword("type") - .instanceLocation(error.getInstanceLocation()) - .messageKey("type") - .format(new MessageFormat("{0}")) - .arguments(error.getMessage() + " (an expression is written with the language as the key and its" - + " expression: property, e.g. groovy: {expression: \"...\"}, simple: {expression: \"...\"}," - + " constant: {expression: \"...\"}; the language: form is" - + " language: {language: groovy, expression: \"...\"})") - .build(); - } - if (error.getMessage().contains("object found, string expected")) { - // message: {simple: "..."}: a string property that is already an expression, or a plain option - String prop = location.substring(location.lastIndexOf('/') + 1); - return Error.builder() - .keyword("type") - .instanceLocation(error.getInstanceLocation()) - .messageKey("type") - .format(new MessageFormat("{0}")) - .arguments(error.getMessage() + " (" + prop + " is a plain string" - + (location.endsWith("/log/message") - ? " that is already a simple expression: write message: \"... ${body} ...\"" - : ": write " + prop + ": \"...\", not a language map") - + ")") - .build(); - } - if (location.matches( - "/\\d+/(onException|onCompletion|intercept|interceptFrom|interceptSendToEndpoint|errorHandler|route|rest|restConfiguration)") - && error.getMessage().contains("array found, object expected")) { - // - onException: [ ... ]: the entry is a map; several handlers are several - onException: items - String entry = location.substring(location.lastIndexOf('/') + 1); - return Error.builder() - .keyword("type") - .instanceLocation(error.getInstanceLocation()) - .messageKey("type") - .format(new MessageFormat("{0}")) - .arguments(error.getMessage() + " (" + entry + " is a map, not a list: - " + entry + ": followed by its" - + " properties indented" + (entry.equals("onException") - ? " (exception: [java.lang.Exception], handled: {constant: {expression: \"true\"}}," - + " steps: [...])" - : "") - + "; several of them are several - " + entry + ": items)") - .build(); - } - if (location.matches(".*/(otherwise|doTry|doFinally|doCatch/\\d+)") && error.getMessage().contains("object expected")) { - // otherwise: [- log: ...]: the block is a map whose steps: holds the list - String eip = location.substring(location.lastIndexOf('/') + 1); - if (eip.matches("\\d+")) { - eip = "doCatch"; - } - return Error.builder() - .keyword("type") - .instanceLocation(error.getInstanceLocation()) - .messageKey("type") - .format(new MessageFormat("{0}")) - .arguments(error.getMessage() + " (" + eip + " holds its EIPs under steps: " + eip - + ": {steps: [- log: \"...\"]}" + (eip.equals("doCatch") - ? ", each - doCatch: item with" - + " exception: and steps:" - : "") - + ")") - .build(); - } - if (!error.getMessage().contains("array expected")) { - return error; - } - String name = location.substring(location.lastIndexOf('/') + 1); - String hint; - if (location.isEmpty() || location.equals("/")) { - hint = "a Camel YAML file is a list of entries, each starting with \"- \": - route:, - from:, - beans:, - rest:," - + " - onException:"; - } else if (name.equals("beans")) { - hint = "beans is a list: - name: myBean followed by type: \"#class:com.example.MyBean\" (indented under the -)"; - } else if (name.equals("steps") || name.equals("when") || name.equals("get") || name.equals("post") - || name.equals("exception") || name.equals("doCatch")) { - hint = name + " is a list: each item starts with \"- \""; - } else { - hint = "write it as a list: each item starts with \"- \""; - } - return Error.builder() - .keyword("type") - .instanceLocation(error.getInstanceLocation()) - .messageKey("type") - .format(new MessageFormat("{0}")) - .arguments(error.getMessage() + " (" + hint + ")") - .build(); + /** The language keys of an expression (simple, constant, groovy...), from the schema. */ + Set<String> languageKeys() { + return languageKeys; } - /** - * Adds a hint to "property 'x' is not defined in the schema": the closest property name of that node (did you mean - * 'logName'?), or, when the property is a top-level entry such as onException written inside a route, where it goes - * instead. - */ - List<Error> withPropertyHints(List<Error> errors) { - List<Error> answer = new ArrayList<>(errors.size()); - for (Error error : errors) { - answer.add(withPropertyHint(error)); - } - return answer; + /** The names of the EIP steps, from the schema. */ + Set<String> stepNames() { + return stepNames; } - Error withPropertyHint(Error error) { - if ("required".equals(error.getKeyword()) && error.getMessage() != null - && error.getMessage().contains("required property 'steps' not found") - && String.valueOf(error.getInstanceLocation()).matches(".*/route/from")) { - return Error.builder() - .keyword("required") - .instanceLocation(error.getInstanceLocation()) - .messageKey("required") - .format(new MessageFormat("{0}")) - .arguments(error.getMessage() + " (steps: is a property of from:, next to uri:; a steps: written at" - + " the route level must be indented under from:)") - .build(); - } - if (!"additionalProperties".equals(error.getKeyword()) || error.getMessage() == null) { - return error; - } - String message = error.getMessage(); - String unknown = between(message, "property '", "'"); - if (unknown == null) { - return error; - } - String location = String.valueOf(error.getInstanceLocation()); - String hint = null; - if (location.matches("/\\d+/beans/\\d+") - && (unknown.equals("id") || unknown.equals("ref") || unknown.equals("class"))) { - // - id: myBean / class: ... : the bean properties are name and type - hint = "a bean is - name: myBean followed by type: \"#class:com.example.MyBean\" (name instead of " + unknown - + (unknown.equals("class") ? ", type instead of class" : "") + ")"; - } else if (location.matches("/\\d+/beans/\\d+")) { - // - myBean: {type: ...} instead of - name: myBean / type: ... - hint = "a bean item is written as - name: " + unknown + " followed by type: \"#class:com.example.MyBean\" " - + "(the name is a property, not the key)"; - } else if (topLevelEntries.contains(unknown) && location.chars().filter(c -> c == '/').count() >= 2) { - hint = "'" + unknown + "' is a top-level entry: write it as a list item at the same level as the route, " - + "not inside it"; - } else if (location.matches(".*/(onException|doCatch/\\d+)") && unknown.matches("([a-z][\\w]*\\.)+[A-Z]\\w*")) { - // onException: {java.lang.Exception: ...}: the class is a list item under exception: - String eip = location.endsWith("/onException") ? "onException" : "doCatch"; - hint = "the exception class is a list item under exception: (" + eip + ": {exception: [" + unknown - + "], steps: [...]})"; - } else if (location.endsWith("/circuitBreaker") && unknown.equals("name")) { - hint = "the circuit breaker's name is its id: circuitBreaker: {id: myBreaker, ...}"; - } else if (location.endsWith("/circuitBreaker") && !resilienceProperties.isEmpty() - && (resilienceProperties.contains(unknown) || closest(unknown, resilienceProperties) != null - || unknown.toLowerCase(Locale.ROOT).contains("threshold") - || unknown.toLowerCase(Locale.ROOT).contains("timeout"))) { - // circuitBreaker: {failureThreshold: 5}: the thresholds and timeouts are resilience4j configuration - String best = resilienceProperties.contains(unknown) ? unknown : closest(unknown, resilienceProperties); - hint = "the thresholds, timeouts and the like are written under resilience4jConfiguration: (circuitBreaker:" - + " {resilience4jConfiguration: {" + (best != null ? best : "failureRateThreshold") + ": ...}, steps:" - + " [...], onFallback: {steps: [...]}})"; - } else if (location.endsWith("/log") && (unknown.equals("level") || unknown.equals("logLevel"))) { - hint = "did you mean 'loggingLevel'?"; - } else if (location.endsWith("/log") && LOG_COMPONENT_OPTIONS.contains(unknown)) { - // log: {message: ..., showHeaders: true}: those are options of the log component endpoint - hint = "'" + unknown + "' is an option of the log component, not of the log EIP: write a to: step with" - + " uri: \"log:com.example?" + unknown + "=...\" (the log EIP has message, loggingLevel, logName," - + " marker)"; - } else if (unknown.equals("steps") && location.matches(".*/route")) { - // - route: {from: {uri: ...}, steps: [...]}: steps belongs under from: - hint = "steps: goes under from:, indented at the same level as uri: (route: {from: {uri: ..., steps: [...]}})"; - } else if (stepNames.contains(unknown) && !location.matches(".*/steps/\\d+") - && location.matches(".*/(otherwise|when/\\d+|doTry|doCatch/\\d+|doFinally|split|filter|loop|aggregate" - + "|circuitBreaker|onFallback|multicast|pipeline|saga|resequence|throttle|delay" - + "|onException|onCompletion|intercept|interceptFrom|interceptSendToEndpoint|route|from)")) { - // otherwise: {log: ...} or when: [- simple: ..., log: ...]: the EIPs go under steps: - String eip = location.substring(location.lastIndexOf('/') + 1); - if (eip.matches("\\d+")) { - String parent = location.substring(0, location.lastIndexOf('/')); - eip = parent.substring(parent.lastIndexOf('/') + 1); - } - hint = "'" + unknown + "' is a step: the steps of " + eip + " go under steps: (" + eip - + ": {steps: [- " + unknown + ": ...]})"; - } else if (unknown.equals("script") && !location.endsWith("/steps") - && (EXPRESSION_REQUIRED.contains(location.substring(location.lastIndexOf('/') + 1)) - || location.endsWith("/expression"))) { - // setBody: {script: ...}: script is an EIP; the language is the key of an expression - hint = "script is an EIP step, not a language: write the language as the key of the expression (expression:" - + " {groovy: {expression: \"...\"}}, expression: {simple: {expression: \"...\"}}), or run a script as" - + " its own step with - script: {expression: {groovy: {expression: \"...\"}}}"; - } else if (unknown.equals("bean") && !location.endsWith("/steps")) { - // setBody: {bean: myBean} : the bean language is method: - hint = "the bean language is written as method: (expression: {method: {ref: myBean, method: process}}), or" - + " call the bean as a step with - bean: {ref: myBean, method: process}"; - } else if (location.matches(".*/(setHeader|setProperty|setVariable|removeHeader|removeProperty|removeVariable)") - && closest(unknown, knownProperties(String.valueOf(error.getSchemaLocation()))) == null) { - // setHeader: {CamelNumberA: {simple: ...}} : the name is a property, not the key - String eip = location.substring(location.lastIndexOf('/') + 1); - hint = "the name is a property: " + eip + ": {name: " + unknown - + (eip.startsWith("set") ? ", expression: {simple: {expression: \"...\"}}}" : "}") - + " (" + unknown + " is not the key)"; - } else if (location.endsWith("/bean") - && (unknown.equals("parameters") || unknown.equals("args") || unknown.equals("arguments"))) { - hint = "arguments are written in the method call: bean: {ref: myBean, method: \"process(${body}, 'x')\"}"; - } else { - String best = closest(unknown, knownProperties(String.valueOf(error.getSchemaLocation()))); - if (best != null) { - hint = "did you mean '" + best + "'?"; - } - } - if (hint == null) { - return error; - } - return Error.builder() - .keyword("additionalProperties") - .instanceLocation(error.getInstanceLocation()) - .messageKey("additionalProperties") - .format(new MessageFormat("{0}")) - .arguments(message + " (" + hint + ")") - .build(); + /** The properties of resilience4jConfiguration, from the schema. */ + Set<String> resilienceProperties() { + return resilienceProperties; } /** The property names the schema allows at the definition an additionalProperties error points to. */ @@ -1212,15 +834,6 @@ public class YamlValidator { return prev[b.length()]; } - private static String between(String text, String start, String end) { - int i = text.indexOf(start); - if (i < 0) { - return null; - } - int j = text.indexOf(end, i + start.length()); - return j < 0 ? null : text.substring(i + start.length(), j); - } - private static boolean isBooleanText(String text) { String s = text.trim(); return "true".equalsIgnoreCase(s) || "false".equalsIgnoreCase(s); diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/SchemaHintsTest.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/SchemaHintsTest.java new file mode 100644 index 000000000000..3dfc0d49d529 --- /dev/null +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/SchemaHintsTest.java @@ -0,0 +1,127 @@ +/* + * 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.text.MessageFormat; +import java.util.List; + +import com.networknt.schema.Error; +import com.networknt.schema.path.NodePath; +import com.networknt.schema.path.PathType; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * CAMEL-24715: the matcher behind the hint tables. The texts of the rows are pinned by the YamlValidator*Test classes + * through the validator; this pins how a row is selected and how the error is rewritten. + */ +public class SchemaHintsTest { + + private static final YamlValidator VALIDATOR = new YamlValidator(); + + private static Error error(String keyword, String location, String message) { + NodePath path = new NodePath(PathType.JSON_POINTER); + for (String segment : location.split("/")) { + if (!segment.isEmpty()) { + path = segment.matches("\\d+") ? path.append(Integer.parseInt(segment)) : path.append(segment); + } + } + return Error.builder().keyword(keyword).instanceLocation(path).messageKey(keyword) + .format(new MessageFormat("{0}")).arguments(message).build(); + } + + @Test + public void testFirstMatchingRowWins() { + List<SchemaHints.Hint> table = List.of( + SchemaHints.append("type", ".*/beans", m -> true, m -> "first"), + SchemaHints.append("type", ".*/beans", m -> true, m -> "second"), + SchemaHints.append("type", null, m -> true, m -> "any")); + Error beans = SchemaHints.apply(table, error("type", "/0/beans", "object found, array expected"), VALIDATOR); + assertThat(beans.getMessage()).isEqualTo("object found, array expected (first)"); + Error other = SchemaHints.apply(table, error("type", "/0/route/from", "object found, array expected"), VALIDATOR); + assertThat(other.getMessage()).isEqualTo("object found, array expected (any)"); + } + + @Test + public void testKeywordLocationAndConditionSelectTheRow() { + List<SchemaHints.Hint> table = List.of( + SchemaHints.append("type", "/\\d+/steps/\\d+", m -> m.message().contains("array expected"), m -> "hint")); + Error unrelatedKeyword = error("required", "/0/steps/1", "array expected"); + Error unrelatedLocation = error("type", "/0/steps", "array expected"); + Error unrelatedMessage = error("type", "/0/steps/1", "object expected"); + Error match = error("type", "/0/steps/1", "array expected"); + assertThat(SchemaHints.apply(table, unrelatedKeyword, VALIDATOR)).isSameAs(unrelatedKeyword); + assertThat(SchemaHints.apply(table, unrelatedLocation, VALIDATOR)).isSameAs(unrelatedLocation); + assertThat(SchemaHints.apply(table, unrelatedMessage, VALIDATOR)).isSameAs(unrelatedMessage); + assertThat(SchemaHints.apply(table, match, VALIDATOR).getMessage()).isEqualTo("array expected (hint)"); + } + + @Test + public void testReplaceRewritesKeywordAndMessage() { + List<SchemaHints.Hint> table = List.of( + SchemaHints.replace("additionalProperties", null, m -> "simple".equals(m.unknown()), + m -> m.name() + ": {" + m.unknown() + ": ...} is the compact notation", "compactNotation", + "compactNotation")); + Error hinted = SchemaHints.apply(table, + error("additionalProperties", "/0/route/from/steps/0/setBody", "property 'simple' is not defined"), + VALIDATOR); + assertThat(hinted.getKeyword()).isEqualTo("compactNotation"); + assertThat(hinted.getMessageKey()).isEqualTo("compactNotation"); + assertThat(hinted.getMessage()).isEqualTo("setBody: {simple: ...} is the compact notation"); + assertThat(String.valueOf(hinted.getInstanceLocation())).isEqualTo("/0/route/from/steps/0/setBody"); + } + + @Test + public void testUnknownPropertyRowNeedsTheName() { + List<SchemaHints.Hint> table = List.of( + SchemaHints.unknownProperty(null, m -> true, m -> "did you mean '" + m.unknown() + "'?")); + Error noName = error("additionalProperties", "/0/log", "not defined in the schema"); + assertThat(SchemaHints.apply(table, noName, VALIDATOR)).isSameAs(noName); + Error named = error("additionalProperties", "/0/log", "property 'lvl' is not defined in the schema"); + assertThat(SchemaHints.apply(table, named, VALIDATOR).getMessage()) + .isEqualTo("property 'lvl' is not defined in the schema (did you mean 'lvl'?)"); + } + + @Test + public void testSameHintAtSameLocationReportedOnce() { + List<SchemaHints.Hint> table = List.of( + SchemaHints.replace("type", null, m -> true, m -> "an expression expected", "type", "expression")); + List<Error> errors = List.of( + error("type", "/0/onException/handled", "boolean found, object expected"), + error("type", "/0/onException/handled", "boolean found, string expected"), + error("type", "/0/onException/continued", "boolean found, object expected"), + error("required", "/0/onException", "required property 'steps' not found")); + List<Error> hinted = SchemaHints.apply(table, errors, VALIDATOR); + assertThat(hinted).hasSize(3); + assertThat(hinted.get(0).getMessage()).isEqualTo("an expression expected"); + assertThat(String.valueOf(hinted.get(1).getInstanceLocation())).isEqualTo("/0/onException/continued"); + assertThat(hinted.get(2)).isSameAs(errors.get(3)); + } + + @Test + public void testMatchNamesTheItemAndItsParent() { + SchemaHints.Match m = SchemaHints.Match.of( + error("additionalProperties", "/0/route/from/steps/2/choice/when/0", "property 'log' is not defined"), + VALIDATOR); + assertThat(m.name()).isEqualTo("0"); + assertThat(m.nameIsIndex()).isTrue(); + assertThat(m.parentName()).isEqualTo("when"); + assertThat(m.unknown()).isEqualTo("log"); + assertThat(m.locationEndsWith("/when/0")).isTrue(); + } +}
