Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omit unrecognized models from sources #1851

Merged
merged 1 commit into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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());
}
}

/**
Expand Down Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")))
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$version: "2.0"

namespace smithy.example

string MyString
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Ignore me!

The sources plugin should ignore unsupported files so that they don't show up
in places like JARs.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down