Skip to content

Commit 93b9bae

Browse files
mtdowlingmtdowling
committed
Make CLI output improvements
This commit closes #797 The CLI will now log validation events as they occur rather than waiting until all events are encountered before writing them. This makes the CLI a bit more responsive when validating hundreds of thousands of shapes. Other improvements were added to the CLI output as well, including the --severity parameter to set the minimum severity to display in output. This can significantly cut down on noise when model files emit things < DANGER.
1 parent 3712c98 commit 93b9bae

File tree

10 files changed

+237
-24
lines changed

10 files changed

+237
-24
lines changed

‎smithy-cli/src/main/java/software/amazon/smithy/cli/SmithyCli.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ public final class SmithyCli {
2929
public static final String DISCOVER = "--discover";
3030
public static final String DISCOVER_CLASSPATH = "--discover-classpath";
3131
public static final String ALLOW_UNKNOWN_TRAITS = "--allow-unknown-traits";
32+
public static final String SEVERITY = "--severity";
3233

3334
private ClassLoader classLoader = getClass().getClassLoader();
3435

‎smithy-cli/src/main/java/software/amazon/smithy/cli/commands/BuildCommand.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ public Parser getParser() {
6868
.option(SmithyCli.DISCOVER, "-d", "Enables model discovery, merging in models found inside of jars")
6969
.parameter(SmithyCli.DISCOVER_CLASSPATH, "Enables model discovery using a custom classpath for models")
7070
.option(SmithyCli.ALLOW_UNKNOWN_TRAITS, "Ignores unknown traits when building models")
71+
.parameter(SmithyCli.SEVERITY, "Sets a minimum validation event severity to display. "
72+
+ "Defaults to NOTE. Can be set to SUPPRESSED, NOTE, WARNING, "
73+
+ "DANGER, ERROR.")
7174
.positional("<MODELS>", "Path to Smithy models or directories")
7275
.build();
7376
}

‎smithy-cli/src/main/java/software/amazon/smithy/cli/commands/CommandUtils.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,20 @@
2121
import java.nio.file.Paths;
2222
import java.security.AccessController;
2323
import java.security.PrivilegedAction;
24+
import java.util.Arrays;
2425
import java.util.List;
2526
import java.util.Set;
27+
import java.util.function.Consumer;
2628
import java.util.logging.Logger;
2729
import software.amazon.smithy.cli.Arguments;
30+
import software.amazon.smithy.cli.Cli;
2831
import software.amazon.smithy.cli.CliError;
32+
import software.amazon.smithy.cli.Colors;
2933
import software.amazon.smithy.cli.SmithyCli;
3034
import software.amazon.smithy.model.Model;
3135
import software.amazon.smithy.model.loader.ModelAssembler;
36+
import software.amazon.smithy.model.validation.ContextualValidationEventFormatter;
37+
import software.amazon.smithy.model.validation.Severity;
3238
import software.amazon.smithy.model.validation.ValidatedResult;
3339

3440
final class CommandUtils {
@@ -40,6 +46,32 @@ private CommandUtils() {}
4046
static Model buildModel(Arguments arguments, ClassLoader classLoader, Set<Validator.Feature> features) {
4147
List<String> models = arguments.positionalArguments();
4248
ModelAssembler assembler = CommandUtils.createModelAssembler(classLoader);
49+
50+
ContextualValidationEventFormatter formatter = new ContextualValidationEventFormatter();
51+
boolean stdout = features.contains(Validator.Feature.STDOUT);
52+
boolean quiet = features.contains(Validator.Feature.QUIET);
53+
Consumer<String> writer = stdout ? Cli.getStdout() : Cli.getStderr();
54+
55+
// --severity defaults to NOTE.
56+
Severity minSeverity = arguments.has(SmithyCli.SEVERITY)
57+
? parseSeverity(arguments.parameter(SmithyCli.SEVERITY))
58+
: Severity.NOTE;
59+
60+
assembler.validationEventListener(event -> {
61+
// Only log events that are >= --severity.
62+
if (event.getSeverity().ordinal() >= minSeverity.ordinal()) {
63+
if (event.getSeverity() == Severity.WARNING && !quiet) {
64+
// Only log warnings when not quiet
65+
Colors.YELLOW.write(writer, formatter.format(event) + System.lineSeparator());
66+
} else if (event.getSeverity() == Severity.DANGER || event.getSeverity() == Severity.ERROR) {
67+
// Always output error and danger events, even when quiet.
68+
Colors.RED.write(writer, formatter.format(event) + System.lineSeparator());
69+
} else if (!quiet) {
70+
writer.accept(formatter.format(event) + System.lineSeparator());
71+
}
72+
}
73+
});
74+
4375
CommandUtils.handleModelDiscovery(arguments, assembler, classLoader);
4476
CommandUtils.handleUnknownTraitsOption(arguments, assembler);
4577
models.forEach(assembler::addImport);
@@ -48,6 +80,11 @@ static Model buildModel(Arguments arguments, ClassLoader classLoader, Set<Valida
4880
return result.getResult().orElseThrow(() -> new RuntimeException("Expected Validator to throw"));
4981
}
5082

83+
static Severity parseSeverity(String str) {
84+
return Severity.fromString(str).orElseThrow(() -> new IllegalArgumentException(
85+
"Invalid severity: " + str + ". Expected one of: " + Arrays.toString(Severity.values())));
86+
}
87+
5188
static ModelAssembler createModelAssembler(ClassLoader classLoader) {
5289
return Model.assembler(classLoader).putProperty(ModelAssembler.DISABLE_JAR_CACHE, true);
5390
}

‎smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ValidateCommand.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ public Parser getParser() {
4242
.option(SmithyCli.ALLOW_UNKNOWN_TRAITS, "Ignores unknown traits when validating models")
4343
.option(SmithyCli.DISCOVER, "-d", "Enables model discovery, merging in models found inside of jars")
4444
.parameter(SmithyCli.DISCOVER_CLASSPATH, "Enables model discovery using a custom classpath for models")
45+
.parameter(SmithyCli.SEVERITY, "Sets a minimum validation event severity to display. "
46+
+ "Defaults to NOTE. Can be set to SUPPRESSED, NOTE, WARNING, "
47+
+ "DANGER, ERROR.")
4548
.positional("<MODELS>", "Path to Smithy models or directories")
4649
.build();
4750
}

‎smithy-cli/src/main/java/software/amazon/smithy/cli/commands/Validator.java‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121
import java.util.function.Consumer;
2222
import software.amazon.smithy.cli.Cli;
2323
import software.amazon.smithy.cli.CliError;
24-
import software.amazon.smithy.cli.Colors;
2524
import software.amazon.smithy.model.Model;
26-
import software.amazon.smithy.model.validation.ContextualValidationEventFormatter;
2725
import software.amazon.smithy.model.validation.Severity;
2826
import software.amazon.smithy.model.validation.ValidatedResult;
2927

@@ -46,26 +44,10 @@ enum Feature {
4644
}
4745

4846
static void validate(ValidatedResult<Model> result, Set<Feature> features) {
49-
ContextualValidationEventFormatter formatter = new ContextualValidationEventFormatter();
50-
51-
boolean stdout = features.contains(Feature.STDOUT);
5247
boolean quiet = features.contains(Feature.QUIET);
48+
boolean stdout = features.contains(Feature.STDOUT);
5349
Consumer<String> writer = stdout ? Cli.getStdout() : Cli.getStderr();
5450

55-
result.getValidationEvents().stream()
56-
.filter(event -> event.getSeverity() != Severity.SUPPRESSED)
57-
.sorted()
58-
.forEach(event -> {
59-
if (event.getSeverity() == Severity.WARNING) {
60-
Colors.YELLOW.write(writer, formatter.format(event));
61-
} else if (event.getSeverity() == Severity.DANGER || event.getSeverity() == Severity.ERROR) {
62-
Colors.RED.write(writer, formatter.format(event));
63-
} else {
64-
writer.accept(event.toString());
65-
}
66-
writer.accept("");
67-
});
68-
6951
long errors = result.getValidationEvents(Severity.ERROR).size();
7052
long dangers = result.getValidationEvents(Severity.DANGER).size();
7153

‎smithy-cli/src/test/java/software/amazon/smithy/cli/commands/ValidateCommandTest.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,18 @@
1717

1818
import static org.hamcrest.MatcherAssert.assertThat;
1919
import static org.hamcrest.Matchers.containsString;
20+
import static org.hamcrest.Matchers.not;
2021

2122
import java.io.ByteArrayOutputStream;
2223
import java.io.PrintStream;
2324
import java.net.URISyntaxException;
25+
import java.nio.file.Path;
2426
import java.nio.file.Paths;
2527
import org.junit.jupiter.api.Assertions;
2628
import org.junit.jupiter.api.Test;
2729
import software.amazon.smithy.cli.CliError;
2830
import software.amazon.smithy.cli.SmithyCli;
31+
import software.amazon.smithy.model.validation.Severity;
2932

3033
public class ValidateCommandTest {
3134
@Test
@@ -72,4 +75,83 @@ public void allowsUnknownTrait() throws URISyntaxException {
7275
String model = Paths.get(getClass().getResource("unknown-trait.smithy").toURI()).toString();
7376
SmithyCli.create().run("validate", "--allow-unknown-traits", model);
7477
}
78+
79+
@Test
80+
public void canSetSeverityToSuppressed() throws Exception {
81+
String result = runValidationEventsTest(Severity.SUPPRESSED);
82+
83+
assertThat(result, containsString("EmitSuppressed"));
84+
assertThat(result, containsString("EmitNotes"));
85+
assertThat(result, containsString("EmitWarnings"));
86+
assertThat(result, containsString("EmitDangers"));
87+
assertThat(result, containsString("TraitTarget"));
88+
}
89+
90+
@Test
91+
public void canSetSeverityToNote() throws Exception {
92+
String result = runValidationEventsTest(Severity.NOTE);
93+
94+
assertThat(result, not(containsString("EmitSuppressed")));
95+
assertThat(result, containsString("EmitNotes"));
96+
assertThat(result, containsString("EmitWarnings"));
97+
assertThat(result, containsString("EmitDangers"));
98+
assertThat(result, containsString("TraitTarget"));
99+
}
100+
101+
@Test
102+
public void canSetSeverityToWarning() throws Exception {
103+
String result = runValidationEventsTest(Severity.WARNING);
104+
105+
assertThat(result, not(containsString("EmitSuppressed")));
106+
assertThat(result, not(containsString("EmitNotes")));
107+
assertThat(result, containsString("EmitWarnings"));
108+
assertThat(result, containsString("EmitDangers"));
109+
assertThat(result, containsString("TraitTarget"));
110+
}
111+
112+
@Test
113+
public void canSetSeverityToDanger() throws Exception {
114+
String result = runValidationEventsTest(Severity.DANGER);
115+
116+
assertThat(result, not(containsString("EmitSuppressed")));
117+
assertThat(result, not(containsString("EmitNotes")));
118+
assertThat(result, not(containsString("EmitWarnings")));
119+
assertThat(result, containsString("EmitDangers"));
120+
assertThat(result, containsString("TraitTarget"));
121+
}
122+
123+
@Test
124+
public void canSetSeverityToError() throws Exception {
125+
String result = runValidationEventsTest(Severity.ERROR);
126+
127+
assertThat(result, not(containsString("EmitSuppressed")));
128+
assertThat(result, not(containsString("EmitNotes")));
129+
assertThat(result, not(containsString("EmitWarnings")));
130+
assertThat(result, not(containsString("EmitDangers")));
131+
assertThat(result, containsString("TraitTarget"));
132+
}
133+
134+
private String runValidationEventsTest(Severity severity) throws Exception {
135+
PrintStream err = System.err;
136+
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
137+
PrintStream printStream = new PrintStream(outputStream);
138+
System.setErr(printStream);
139+
140+
Path validationEventsModel = Paths.get(getClass().getResource("validation-events.smithy").toURI());
141+
try {
142+
SmithyCli.create().run("validate", "--severity", severity.toString(), validationEventsModel.toString());
143+
} catch (RuntimeException e) {
144+
// ignore the error since everything we need was captured via stderr.
145+
}
146+
147+
System.setErr(err);
148+
return outputStream.toString("UTF-8");
149+
}
150+
151+
@Test
152+
public void validatesSeverity() {
153+
Assertions.assertThrows(
154+
IllegalArgumentException.class,
155+
() -> SmithyCli.create().run("validate", "--severity", "FOO"));
156+
}
75157
}

‎smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/validation-events.smithy‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
$version: "1.0"
2+
3+
metadata validators = [
4+
{
5+
name: "EmitEachSelector",
6+
id: "EmitSuppressed",
7+
severity: "NOTE",
8+
configuration: {
9+
selector: "[id = smithy.example#Suppressed]"
10+
}
11+
},
12+
{
13+
name: "EmitEachSelector",
14+
id: "EmitNotes",
15+
severity: "NOTE",
16+
configuration: {
17+
selector: "[id = smithy.example#Note]"
18+
}
19+
},
20+
{
21+
name: "EmitEachSelector",
22+
id: "EmitWarnings",
23+
severity: "WARNING",
24+
configuration: {
25+
selector: "[id = smithy.example#Warning]"
26+
}
27+
},
28+
{
29+
name: "EmitEachSelector",
30+
id: "EmitDangers",
31+
severity: "DANGER",
32+
configuration: {
33+
selector: "[id = smithy.example#Danger]"
34+
}
35+
}
36+
]
37+
38+
namespace smithy.example
39+
40+
@suppress(["EmitSuppressed"])
41+
string Suppressed
42+
43+
string Note
44+
45+
string Warning
46+
47+
string Danger
48+
49+
@required // this trait is invalid and causes an error.
50+
string Error

‎smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.Map;
3434
import java.util.Objects;
3535
import java.util.Set;
36+
import java.util.function.Consumer;
3637
import java.util.function.Supplier;
3738
import java.util.logging.Logger;
3839
import java.util.stream.Collectors;
@@ -97,6 +98,7 @@ public final class ModelAssembler {
9798
private final Map<String, Node> metadata = new HashMap<>();
9899
private final Map<String, Object> properties = new HashMap<>();
99100
private boolean disablePrelude;
101+
private Consumer<ValidationEvent> validationEventListener = event -> {};
100102

101103
// Lazy initialization holder class idiom to hold a default validator factory.
102104
private static final class LazyValidatorFactoryHolder {
@@ -128,6 +130,7 @@ public ModelAssembler copy() {
128130
assembler.disablePrelude = disablePrelude;
129131
assembler.properties.putAll(properties);
130132
assembler.disableValidation = disableValidation;
133+
assembler.validationEventListener = validationEventListener;
131134
return assembler;
132135
}
133136

@@ -146,6 +149,7 @@ public ModelAssembler copy() {
146149
* <li>Shape registered via {@link #addModel}</li>
147150
* <li>Metadata registered via {@link #putMetadata}</li>
148151
* <li>Validation is re-enabled if it was disabled.</li>
152+
* <li>Validation event listener via {@link #validationEventListener(Consumer)}</li>
149153
* </ul>
150154
*
151155
* <p>The state of {@link #disablePrelude} is reset such that the prelude
@@ -163,6 +167,7 @@ public ModelAssembler reset() {
163167
documentNodes.clear();
164168
disablePrelude = false;
165169
disableValidation = false;
170+
validationEventListener = null;
166171
return this;
167172
}
168173

@@ -475,6 +480,24 @@ public ModelAssembler disableValidation() {
475480
return this;
476481
}
477482

483+
/**
484+
* Sets a listener that is invoked each time a ValidationEvent is encountered
485+
* while loading and validating the model.
486+
*
487+
* <p>The consumer could be invoked simultaneously by multiple threads. It's
488+
* up to the consumer to perform any necessary synchronization. Providing
489+
* an event listener is useful for things like CLIs so that events can
490+
* be streamed to stdout as soon as they are encountered, rather than
491+
* waiting until the entire model is parsed and validated.
492+
*
493+
* @param eventListener Listener invoked for each ValidationEvent.
494+
* @return Returns the assembler.
495+
*/
496+
public ModelAssembler validationEventListener(Consumer<ValidationEvent> eventListener) {
497+
validationEventListener = eventListener == null ? event -> {} : eventListener;
498+
return this;
499+
}
500+
478501
/**
479502
* Assembles the model and returns the validated result.
480503
*
@@ -599,6 +622,7 @@ private List<ModelFile> createModelFiles() {
599622

600623
private ValidatedResult<Model> validate(Model model, TraitContainer traits, List<ValidationEvent> events) {
601624
validateTraits(model.getShapeIds(), traits, events);
625+
events.forEach(validationEventListener);
602626

603627
// If ERROR validation events occur while loading, then performing more
604628
// granular semantic validation will only obscure the root cause of errors.
@@ -611,7 +635,10 @@ private ValidatedResult<Model> validate(Model model, TraitContainer traits, List
611635
}
612636

613637
// Validate the model based on the explicit validators and model metadata.
614-
List<ValidationEvent> mergedEvents = ModelValidator.validate(model, validatorFactory, assembleValidators());
638+
// Note the ModelValidator handles emitting events to the validationEventListener.
639+
List<ValidationEvent> mergedEvents = ModelValidator
640+
.validate(model, validatorFactory, assembleValidators(), validationEventListener);
641+
615642
mergedEvents.addAll(events);
616643
return new ValidatedResult<>(model, mergedEvents);
617644
}

0 commit comments

Comments
 (0)