diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuild.java b/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuild.java index 0dddac1005f..136b03f779c 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuild.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuild.java @@ -15,7 +15,12 @@ package software.amazon.smithy.build; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.PathMatcher; import java.nio.file.Paths; import java.util.Collections; import java.util.HashSet; @@ -29,6 +34,7 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; +import java.util.logging.Logger; import software.amazon.smithy.build.model.SmithyBuildConfig; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.loader.ModelAssembler; @@ -42,6 +48,9 @@ public final class SmithyBuild { /** The version of Smithy build. */ public static final String VERSION = "1.0"; + private static final Logger LOGGER = Logger.getLogger(SmithyBuild.class.getName()); + private static final PathMatcher VALID_MODELS = FileSystems.getDefault().getPathMatcher("glob:*.{json,jar,smithy}"); + SmithyBuildConfig config; Path outputDirectory; Function<String, Optional<ProjectionTransformer>> transformFactory; @@ -187,8 +196,27 @@ public SmithyBuild config(SmithyBuildConfig config) { // Add a source path using absolute paths to better de-conflict source files. ModelAssembler also // de-conflicts imports with absolute paths, but this ensures the same file doesn't appear twice in // the build plugin output (though it does not use realpath to de-conflict based on symlinks). + // + // Ignores and logs when an unsupported model file is encountered. private void addSource(Path path) { - sources.add(path.toAbsolutePath()); + try { + if (Files.isDirectory(path)) { + // Pre-emptively crawl the given models to filter them up front into a flat, file-only, list. + Files.list(path).filter(Files::isRegularFile).forEach(this::addSourceFile); + } else { + addSourceFile(path); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private void addSourceFile(Path file) { + if (!VALID_MODELS.matches(file.getFileName())) { + LOGGER.warning("Omitting unsupported Smithy model file from model sources: " + file); + } else { + sources.add(file.toAbsolutePath()); + } } /** @@ -380,6 +408,12 @@ public SmithyBuild pluginClassLoader(ClassLoader pluginClassLoader) { * unique across the entire set of files. The sources directories are * essentially flattened into a single directory. * + * <p>Unsupported model files are ignored and not treated as sources. + * This can happen when adding model files from a directory that contains + * a mix of model files and non-model files. Filtering models here prevents + * unsupported files from appearing in places like JAR manifest files where + * they are not allowed. + * * @param pathToSources Path to source directories to mark. * @return Returns the builder. */ diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/plugins/SourcesPlugin.java b/smithy-build/src/main/java/software/amazon/smithy/build/plugins/SourcesPlugin.java index d6a50629aba..603aa4c2f74 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/plugins/SourcesPlugin.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/plugins/SourcesPlugin.java @@ -145,8 +145,16 @@ private static void copyFile(List<String> names, FileManifest manifest, Path tar + ValidationUtils.tickedList(manifest.getFiles())); } - manifest.writeFile(target, contents); - names.add(target.toString()); + String filename = target.toString(); + + // Even though sources are filtered in SmithyBuild, it's theoretically possible that someone could call this + // plugin manually. In that case, refuse to write unsupported files to the manifest. + if (filename.endsWith(".smithy") || filename.endsWith(".json")) { + manifest.writeFile(target, contents); + names.add(target.toString()); + } else { + LOGGER.warning("Omitting unrecognized file from Smithy model manifest: " + filename); + } } private static void projectSources(PluginContext context) { diff --git a/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java b/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java index f140b2d7005..ad941ffeb86 100644 --- a/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java +++ b/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java @@ -48,12 +48,16 @@ import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import software.amazon.smithy.build.model.ProjectionConfig; import software.amazon.smithy.build.model.SmithyBuildConfig; import software.amazon.smithy.build.model.TransformConfig; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.SourceException; import software.amazon.smithy.model.SourceLocation; +import software.amazon.smithy.model.loader.ModelAssembler; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.node.ObjectNode; import software.amazon.smithy.model.shapes.ShapeId; @@ -898,5 +902,51 @@ public void detectsConflictingArtifactNames() throws Exception { assertThat(e.getMessage(), containsString("Multiple plugins use the same artifact name 'foo' in " + "the 'source' projection")); } -} + @ParameterizedTest + @MethodSource("unrecognizedModelPaths") + public void ignoreUnrecognizedModels(List<Path> models) throws URISyntaxException { + ModelAssembler assembler = Model.assembler(); + for (Path path : models) { + assembler.addImport(path); + } + Model model = assembler.assemble().unwrap(); + + SmithyBuild builder = new SmithyBuild().model(model).fileManifestFactory(MockManifest::new); + models.forEach(builder::registerSources); + + // Apply multiple projections to ensure that sources are filtered even in projections. + builder.config(SmithyBuildConfig.builder() + .load(Paths.get(getClass().getResource("apply-multiple-projections.json").toURI())) + .outputDirectory("/foo") + .build()); + + SmithyBuildResult results = builder.build(); + assertTrue(results.getProjectionResult("source").isPresent()); + assertTrue(results.getProjectionResult("a").isPresent()); + + ProjectionResult sourceResult = results.getProjectionResult("source").get(); + MockManifest sourceManifest = (MockManifest) sourceResult.getPluginManifest("sources").get(); + String sourceSourcesManifestText = sourceManifest.getFileString("manifest").get(); + + assertThat(sourceSourcesManifestText, containsString("a.smithy")); + assertThat(sourceSourcesManifestText, not(containsString("foo.md"))); + + ProjectionResult aResult = results.getProjectionResult("source").get(); + MockManifest aManifest = (MockManifest) aResult.getPluginManifest("sources").get(); + String aSourcesManifestText = aManifest.getFileString("manifest").get(); + + assertThat(aSourcesManifestText, not(containsString("foo.md"))); + } + + public static List<Arguments> unrecognizedModelPaths() throws URISyntaxException { + Path rootPath = Paths.get(SmithyBuildTest.class.getResource("plugins/sources-ignores-unrecognized-files") + .toURI()); + return ListUtils.of( + // Test that crawling the directory works. + Arguments.of(ListUtils.of(rootPath)), + // Test that passing explicit files works too. + Arguments.of(ListUtils.of(rootPath.resolve("a.smithy"), rootPath.resolve("foo.md"))) + ); + } +} diff --git a/smithy-build/src/test/java/software/amazon/smithy/build/plugins/SourcesPluginTest.java b/smithy-build/src/test/java/software/amazon/smithy/build/plugins/SourcesPluginTest.java index 74e943c30e9..28eca8dff7d 100644 --- a/smithy-build/src/test/java/software/amazon/smithy/build/plugins/SourcesPluginTest.java +++ b/smithy-build/src/test/java/software/amazon/smithy/build/plugins/SourcesPluginTest.java @@ -208,4 +208,28 @@ public void copiesModelsDefinedInConfigAsSources() throws URISyntaxException { assertThat(manifest.hasFile("a.smithy"), is(true)); assertThat(manifest.hasFile("d.smithy"), is(false)); } + + // When the sources plugin sees a file it does not support, it is ignored. + @Test + public void omitsUnsupportedFilesFromManifest() throws URISyntaxException { + Model model = Model.assembler() + .addImport(getClass().getResource("sources-ignores-unrecognized-files/a.smithy")) + .addImport(getClass().getResource("sources-ignores-unrecognized-files/foo.md")) + .assemble() + .unwrap(); + MockManifest manifest = new MockManifest(); + PluginContext context = PluginContext.builder() + .fileManifest(manifest) + .model(model) + .originalModel(model) + .sources(ListUtils.of(Paths.get(getClass().getResource("sources-ignores-unrecognized-files").toURI()))) + .build(); + new SourcesPlugin().execute(context); + String manifestString = manifest.getFileString("manifest").get(); + // Normalize for Windows. + manifestString = manifestString.replace("\\", "/"); + + assertThat(manifestString, containsString("a.smithy\n")); + assertThat(manifestString, not(containsString("foo.md"))); + } } diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/plugins/sources-ignores-unrecognized-files/a.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/plugins/sources-ignores-unrecognized-files/a.smithy new file mode 100644 index 00000000000..b528584b5d0 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/plugins/sources-ignores-unrecognized-files/a.smithy @@ -0,0 +1,5 @@ +$version: "2.0" + +namespace smithy.example + +string MyString diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/plugins/sources-ignores-unrecognized-files/foo.md b/smithy-build/src/test/resources/software/amazon/smithy/build/plugins/sources-ignores-unrecognized-files/foo.md new file mode 100644 index 00000000000..3b20c7ce0ef --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/plugins/sources-ignores-unrecognized-files/foo.md @@ -0,0 +1,4 @@ +# Ignore me! + +The sources plugin should ignore unsupported files so that they don't show up +in places like JARs. diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java index cc189a5af29..c2ee05ffd99 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java @@ -82,7 +82,7 @@ static boolean load( return loadParsedNode(Node.parse(inputStream, filename), operationConsumer); } } else { - LOGGER.warning(() -> "Ignoring unrecognized file: " + filename); + LOGGER.warning(() -> "Ignoring unrecognized Smithy model file: " + filename); return false; } } catch (IOException e) {