Skip to content

Commit 0cf711f

Browse files
authored
Enhance error message for multiple key detection (#2828)
* refactor(validation): enhance error message for multiple key detection - Updated `ConfigurationParsingException` message to provide clearer guidance on separating APIs and configuration using `---`. * refactor(parser): improve code formatting, readability, and exception messages - Adjusted spacing and formatting for better consistency across methods. - Enhanced clarity of exception messages in `ConfigurationParsingException` and improved handling of multi-document YAML scenarios. - Streamlined error logs for better debugging during configuration parsing. * fix(parser): improve exception handling in `GenericYamlParser` - Added additional context to `ConfigurationParsingException` to enhance debugging (`$ref` key and its context).
1 parent 934c76d commit 0cf711f

3 files changed

Lines changed: 36 additions & 32 deletions

File tree

annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanRegistryImplementation.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,16 @@
1717
import com.predic8.membrane.annot.yaml.*;
1818
import org.jetbrains.annotations.*;
1919
import org.slf4j.*;
20+
import org.springframework.beans.factory.xml.*;
2021

2122
import javax.annotation.concurrent.*;
22-
import java.io.IOException;
23-
import java.lang.reflect.Method;
23+
import java.io.*;
24+
import java.lang.reflect.*;
2425
import java.util.*;
2526
import java.util.concurrent.*;
2627
import java.util.function.*;
2728

28-
import static com.predic8.membrane.annot.yaml.WatchAction.ADDED;
29+
import static com.predic8.membrane.annot.yaml.WatchAction.*;
2930

3031
/**
3132
* TODO:

annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,9 @@ public class GenericYamlParser {
6464
* turned into a {@link BeanDefinition}. Validation errors are mapped back to line/column
6565
* numbers using {@link JsonLocationMap} to produce helpful error messages.
6666
* </p>
67+
*
6768
* @param grammar provides schema location and Java type resolution
68-
* @param yaml the raw YAML content (may contain multi-document stream)
69+
* @param yaml the raw YAML content (may contain multi-document stream)
6970
* @throws IOException if schema loading or validation fails
7071
*/
7172
public GenericYamlParser(Grammar grammar, String yaml) throws IOException {
@@ -81,10 +82,10 @@ public GenericYamlParser(Grammar grammar, String yaml) throws IOException {
8182
// Deactivated temporarily to get better error messages
8283
//validateAgainstSchema(grammar, jsonNode, jsonLocationMap);
8384

84-
var pc = new ParsingContext<>("",null,grammar, jsonNode, "$",null);
85+
var pc = new ParsingContext<>("", null, grammar, jsonNode, "$", null);
8586

8687
if ("components".equals(getBeanType(pc, jsonNode))) {
87-
beanDefs.addAll(extractComponentBeanDefinitions(pc.addPath(".components"),jsonNode.get("components")));
88+
beanDefs.addAll(extractComponentBeanDefinitions(pc.addPath(".components"), jsonNode.get("components")));
8889
}
8990

9091
beanDefs.add(new BeanDefinition(
@@ -118,8 +119,9 @@ private static void validateAgainstSchema(Grammar grammar, JsonNode jsonNode, Js
118119
* <li>Validates each document against the JSON Schema provided by {@code grammar}.</li>
119120
* <li>Emits helpful line/column locations for malformed multi-document input.</li>
120121
* </ul>
122+
*
121123
* @param resource the input stream to parse. The method takes care of closing the stream.
122-
* @param grammar the grammar to use for type resolution and schema location
124+
* @param grammar the grammar to use for type resolution and schema location
123125
* @return list of parsed bean definitions
124126
*/
125127
public static List<BeanDefinition> parseMembraneResources(@NotNull InputStream resource, Grammar grammar) throws IOException {
@@ -150,18 +152,18 @@ private static String getBeanType(ParsingContext<?> ctx, JsonNode jsonNode) {
150152
* grammar and delegates to {@link #createAndPopulateNode(ParsingContext, Class, JsonNode)}.</p>
151153
*/
152154
public static <R extends BeanRegistry & BeanLifecycleManager> Object readMembraneObject(String kind, Grammar grammar, JsonNode node, R registry) throws ConfigurationParsingException {
153-
return createAndPopulateNode(new ParsingContext<>(kind, registry, grammar,node, "$." + kind,null), decideClazz(kind, grammar, node), node.get(kind));
155+
return createAndPopulateNode(new ParsingContext<>(kind, registry, grammar, node, "$." + kind, null), decideClazz(kind, grammar, node), node.get(kind));
154156
}
155157

156158
/**
157159
* Detects the class that will be selected to represent the node in Java.
158160
*/
159161
public static Class<?> decideClazz(String kind, Grammar grammar, JsonNode node) {
160-
ensureSingleKey(new ParsingContext("",null, grammar,node,"$",null),node);
162+
ensureSingleKey(new ParsingContext("", null, grammar, node, "$", null), node);
161163
Class<?> clazz = grammar.getElement(kind);
162164
if (clazz == null) {
163-
var pc = new ParsingContext("", null,grammar,node,"$",null).key(kind);
164-
throw new ConfigurationParsingException("Did not find java class for kind '%s'.".formatted(kind),null,pc);
165+
var pc = new ParsingContext("", null, grammar, node, "$", null).key(kind);
166+
throw new ConfigurationParsingException("Did not find java class for kind '%s'.".formatted(kind), null, pc);
165167
}
166168
return clazz;
167169
}
@@ -170,7 +172,7 @@ public static Class<?> decideClazz(String kind, Grammar grammar, JsonNode node)
170172
* Creates and populates an instance of {@code clazz} from the given YAML/JSON node.
171173
* - Arrays: only valid for {@code @MCElement(noEnvelope=true)}; items are parsed and passed to the single {@code @MCChildElement} list setter.
172174
* - Objects: each field is mapped to a setter resolved by {@link MethodSetter#getMethodSetter(ParsingContext, Class, String)};
173-
* values are produced by {@link MethodSetter#getMethodSetter(ParsingContext, Class, String)}. A top-level {@code "$ref"} injects a previously defined bean.
175+
* values are produced by {@link MethodSetter#getMethodSetter(ParsingContext, Class, String)}. A top-level {@code "$ref"} injects a previously defined bean.
174176
* All failures are wrapped in a {@link ConfigurationParsingException} with location information.
175177
*/
176178
public static <T> T createAndPopulateNode(ParsingContext<?> pc, Class<T> clazz, JsonNode node) throws ConfigurationParsingException {
@@ -189,7 +191,7 @@ public static <T> T createAndPopulateNode(ParsingContext<?> pc, Class<T> clazz,
189191
ensureMappingStart(node);
190192
if (isNoEnvelope(clazz)) {
191193
log.error("Class {} is annotated with @MCElement(noEnvelope=true), but the YAML/JSON structure does not contain a list.", clazz.getName());
192-
throw new ConfigurationParsingException("Class %s is annotated with @MCElement(noEnvelope=true), but the YAML/JSON structure does not contain a list.".formatted(clazz.getName()),null,pc);
194+
throw new ConfigurationParsingException("Class %s is annotated with @MCElement(noEnvelope=true), but the YAML/JSON structure does not contain a list.".formatted(clazz.getName()), null, pc);
193195
}
194196

195197
JsonNode refNode = node.get("$ref");
@@ -215,9 +217,8 @@ public static <T> T createAndPopulateNode(ParsingContext<?> pc, Class<T> clazz,
215217
if (e.getParsingContext() == null)
216218
e.setParsingContext(pc);
217219
throw e;
218-
}
219-
catch (Throwable cause) {
220-
log.debug("",cause);
220+
} catch (Throwable cause) {
221+
log.debug("", cause);
221222
throw new ConfigurationParsingException(cause);
222223
}
223224
}
@@ -234,9 +235,8 @@ private static <T> void populateObjectFields(ParsingContext<?> ctx, Class<T> cla
234235
setter.setSetter(configObj, ctx, node, key);
235236
} catch (ConfigurationParsingException e) {
236237
throw e;
237-
}
238-
catch (Throwable cause) {
239-
log.debug("",cause);
238+
} catch (Throwable cause) {
239+
log.debug("", cause);
240240
var e = new ConfigurationParsingException(cause.getMessage());
241241
e.setParsingContext(ctx.key(key));
242242
throw e;
@@ -246,7 +246,8 @@ private static <T> void populateObjectFields(ParsingContext<?> ctx, Class<T> cla
246246

247247
private static <T> @NotNull T handleCollapsed(ParsingContext<?> ctx, Class<T> clazz, JsonNode node, T configObj) {
248248
if (node.isNull()) throw new ConfigurationParsingException("Collapsed element must not be null.");
249-
if (node.isArray() || node.isObject()) throw new ConfigurationParsingException("Element is collapsed; expected an inline scalar value, not an %s.".formatted((node.isArray() ? "array" : "object")));
249+
if (node.isArray() || node.isObject())
250+
throw new ConfigurationParsingException("Element is collapsed; expected an inline scalar value, not an %s.".formatted((node.isArray() ? "array" : "object")));
250251
applyCollapsedScalar(clazz, node, configObj);
251252
return handlePostConstructAndPreDestroy(ctx, configObj);
252253
}
@@ -272,7 +273,7 @@ private static List<BeanDefinition> extractComponentBeanDefinitions(ParsingConte
272273
JsonNode def = componentsNode.get(id);
273274

274275
// Each component definition must have exactly one key (the component type)
275-
ensureSingleKey(pc.addPath("."+id),def);
276+
ensureSingleKey(pc.addPath("." + id), def);
276277
String componentKind = def.fieldNames().next();
277278

278279
// Wrap it into a normal top-level node: { <kind>: <body> }
@@ -311,7 +312,7 @@ private static <T> void applyObjectLevelRef(ParsingContext<?> ctx, Class<T> pare
311312
} catch (RuntimeException e) {
312313
throw new ConfigurationParsingException(
313314
"Referenced component '%s' (type '%s') is not allowed in '%s'."
314-
.formatted(refNode.asText(), refKey, ctx.getContext()));
315+
.formatted(refNode.asText(), refKey, ctx.getContext()),e,ctx.key("$ref"));
315316
} catch (Throwable t) {
316317
throw new ConfigurationParsingException(t);
317318
}
@@ -321,7 +322,7 @@ private static Object getReferenced(ParsingContext<?> ctx, JsonNode refNode) {
321322
try {
322323
return ctx.getRegistry().resolve(refNode.asText());
323324
} catch (RuntimeException e) {
324-
throw new ConfigurationParsingException(e);
325+
throw new ConfigurationParsingException("Cannot resolve reference: " + refNode.asText(),e,ctx.key("$ref"));
325326
}
326327
}
327328

@@ -330,7 +331,7 @@ public static List<Object> parseListIncludingStartEvent(ParsingContext<?> contex
330331
}
331332

332333
public static List<Object> parseListIncludingStartEvent(ParsingContext<?> pc, JsonNode node, Class<?> elemType) throws ConfigurationParsingException {
333-
ensureArray(pc,node);
334+
ensureArray(pc, node);
334335
return parseListExcludingStartEvent(pc, node, elemType);
335336
}
336337

@@ -347,7 +348,7 @@ public static List<Object> parseListIncludingStartEvent(ParsingContext<?> pc, Js
347348
* delegating to {@link #parseMapToObj(ParsingContext, JsonNode, String)}.
348349
*/
349350
private static Object parseMapToObj(ParsingContext<?> pc, JsonNode node) throws ConfigurationParsingException {
350-
ensureSingleKey(pc,node);
351+
ensureSingleKey(pc, node);
351352
String key = node.fieldNames().next();
352353
return parseMapToObj(pc, node.get(key), key);
353354
}
@@ -420,7 +421,8 @@ private static Object parseListItem(ParsingContext<?> ctx, JsonNode item, Class<
420421
}
421422

422423
private static Object parseInlineListItem(ParsingContext<?> ctx, JsonNode node, Class<?> elemType) {
423-
if (elemType == null) throw new ConfigurationParsingException("Inline list item form requires a typed list element.");
424+
if (elemType == null)
425+
throw new ConfigurationParsingException("Inline list item form requires a typed list element.");
424426
if (isScalarElementType(elemType)) {
425427
if (node.isObject() || node.isArray()) {
426428
throw new ConfigurationParsingException(
@@ -436,10 +438,10 @@ private static Object parseInlineListItem(ParsingContext<?> ctx, JsonNode node,
436438

437439
private static boolean isScalarElementType(Class<?> t) {
438440
return t == String.class
439-
|| t == Boolean.class
440-
|| t == Character.class
441-
|| Number.class.isAssignableFrom(t)
442-
|| t.isEnum();
441+
|| t == Boolean.class
442+
|| t == Character.class
443+
|| Number.class.isAssignableFrom(t)
444+
|| t.isEnum();
443445
}
444446

445447
@SuppressWarnings({"unchecked", "rawtypes"})
@@ -450,7 +452,8 @@ private static Object coerceScalarListItem(JsonNode node, Class<?> elemType) {
450452
String raw = node.asText();
451453
try {
452454
return Enum.valueOf((Class<? extends Enum>) elemType, raw);
453-
} catch (IllegalArgumentException ignored) {}
455+
} catch (IllegalArgumentException ignored) {
456+
}
454457
try {
455458
return Enum.valueOf((Class<? extends Enum>) elemType, raw.toUpperCase(ROOT));
456459
} catch (IllegalArgumentException e) {

annot/src/main/java/com/predic8/membrane/annot/yaml/NodeValidationUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public static void ensureMappingStart(JsonNode node) throws ConfigurationParsing
2727
public static void ensureSingleKey(ParsingContext<?> ctx, JsonNode node) {
2828
ensureMappingStart(node);
2929
if (node.size() != 1) {
30-
var e = new ConfigurationParsingException("Expected exactly one key but there are %d.".formatted(node.size()));
30+
var e = new ConfigurationParsingException("Expected exactly one key but there are %d. Separate APIs and configuration by ---".formatted(node.size()));
3131
e.setParsingContext(ctx);
3232
throw e;
3333
}

0 commit comments

Comments
 (0)