davsclaus commented on code in PR #27231:
URL: https://github.com/apache/camel/pull/27231#discussion_r4159552732
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java:
##########
@@ -1128,19 +1235,81 @@ private Map<String, OneOfGroup> loadOneOfGroups() {
* {@code BeanConstructorDefinition.index}, which is a map key and is not
reachable from the schema - see
* CAMEL-24696 before exposing it.
*/
+ /**
Review Comment:
Fixed in 8036ea5a19a7: the new methods now sit above the original Javadoc,
which is back directly on `isRuntimeAcceptedScalar`.
_Claude Code on behalf of davsclaus_
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java:
##########
@@ -1128,19 +1235,81 @@ private Map<String, OneOfGroup> loadOneOfGroups() {
* {@code BeanConstructorDefinition.index}, which is a map key and is not
reachable from the schema - see
* CAMEL-24696 before exposing it.
*/
+ /**
+ * Drops "required property X not found" where one of the alternatives is
in fact there.
+ * <p/>
+ * The branches of a {@code oneOf} each require their own key, and when
all of them are reported the file is told it
+ * needs a key it does not: {@code unmarshal: {fhirXml: ...}} in one when
branch of a choice was told "required
+ * property 'fhirJson' not found", because the winner of the other when
branch was chosen for it as well. Collected
+ * per construct and per place, the required names of the branches are the
alternatives; if the object at that place
+ * has any of them, one branch is satisfied and the rest are noise. When
none of them is there, the file really does
+ * have to pick one and the errors stay (CAMEL-25238).
+ */
+ private static void removeSatisfiedAlternatives(List<Error> errors) {
+ Map<String, List<Error>> groups = new LinkedHashMap<>();
+ for (Error e : errors) {
+ if (!"required".equals(e.getKeyword())) {
+ continue;
+ }
+ String construct =
constructOf(String.valueOf(e.getEvaluationPath()));
+ if (construct != null) {
+ groups.computeIfAbsent(construct + "@" +
e.getInstanceLocation(), k -> new ArrayList<>()).add(e);
+ }
+ }
+ for (List<Error> group : groups.values()) {
+ JsonNode instance = group.get(0).getInstanceNode();
+ if (instance == null || !instance.isObject()) {
+ continue;
+ }
+ // the branches of a pick-one construct each require their own
key, so one error per key the file did not
+ // write. The key it did write is the one missing from them: that
branch got past its required check and
+ // failed, if at all, further in. So a non-empty object means a
branch was chosen and the rest are noise.
+ // An object with nothing in it, or with a key no branch knows,
still gets the construct's own error and
+ // the additionalProperties error, which is what actually tells
the author what to do.
+ if (!instance.isEmpty() && group.size() > 1) {
+ errors.removeAll(group);
+ }
+ }
+ }
+
+ private static final Pattern REQUIRED_PROPERTY = Pattern.compile("required
property '([^']+)' not found");
+
+ /** The property name a "required property 'X' not found" error names, or
null. */
+ private static String requiredPropertyOf(Error error) {
+ Matcher m =
REQUIRED_PROPERTY.matcher(String.valueOf(error.getMessage()));
+ return m.find() ? m.group(1) : null;
+ }
Review Comment:
Removed in 8036ea5a19a7. They were left over from an approach that compared
the required key names; the final check only looks at whether the object is
non-empty. The Javadoc of `removeSatisfiedAlternatives` described the old
approach too, so it now says what the code does.
_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]
