Skip to content

Commit

Permalink
Merge pull request square#620 from square/jwilson.0728.validate_profile
Browse files Browse the repository at this point in the history 
Validate that profiles are well-formed.
  • Loading branch information
@swankjesse
swankjesse authored Jul 28, 2016
2 parents 1b3c1db + cd2c168 commit b2ec55c
Show file tree
Hide file tree
Showing 3 changed files with 203 additions and 86 deletions.
84 changes: 68 additions & 16 deletions wire-java-generator/src/main/java/com/squareup/wire/java/ProfileLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,51 +21,69 @@
import com.google.common.io.Closer;
import com.squareup.wire.java.internal.ProfileFileElement;
import com.squareup.wire.java.internal.ProfileParser;
import com.squareup.wire.java.internal.TypeConfigElement;
import com.squareup.wire.schema.Location;
import com.squareup.wire.schema.ProtoFile;
import com.squareup.wire.schema.ProtoType;
import com.squareup.wire.schema.Schema;
import com.squareup.wire.schema.SchemaException;
import com.squareup.wire.schema.Type;
import java.io.IOException;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import okio.Okio;
import okio.Source;

import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.Iterables.getOnlyElement;

/**
* Load files with the {@code .wire} suffix for a schema. The file name is the profile name; for
* example the {@code android} profile loads {@code android.wire} files.
*/
public final class ProfileLoader {
private final FileSystem fileSystem;
private final String name;
private final Set<Location> protoLocations = new LinkedHashSet<>();
private Schema schema;

public ProfileLoader(String name) {
public ProfileLoader(FileSystem fileSystem, String name) {
this.fileSystem = fileSystem;
this.name = name;
}

public ProfileLoader addSchema(Schema schema) {
for (ProtoFile protoFile : schema.protoFiles()) {
protoLocations.add(protoFile.location());
}
return this;
public ProfileLoader(String name) {
this(FileSystems.getDefault(), name);
}

public ProfileLoader addProtoLocation(Location location) {
protoLocations.add(location);
public ProfileLoader schema(Schema schema) {
checkState(this.schema == null);
this.schema = schema;
return this;
}

public Profile load() throws IOException {
Multimap<Path, String> pathsToAttempt = pathsToAttempt();
Set<Location> protoLocations = new LinkedHashSet<>();
for (ProtoFile protoFile : schema.protoFiles()) {
protoLocations.add(protoFile.location());
}
Multimap<Path, String> pathsToAttempt = pathsToAttempt(protoLocations);

ImmutableList<ProfileFileElement> profileFiles = loadProfileFiles(pathsToAttempt);
Profile profile = new Profile(profileFiles);
validate(schema, profileFiles);
return profile;
}

private ImmutableList<ProfileFileElement> loadProfileFiles(Multimap<Path, String> pathsToAttempt)
throws IOException {
ImmutableList.Builder<ProfileFileElement> result = ImmutableList.builder();
try (Closer closer = Closer.create()) {
for (Map.Entry<Path, Collection<String>> entry : pathsToAttempt.asMap().entrySet()) {
Expand All @@ -81,16 +99,14 @@ public Profile load() throws IOException {
}
}
}

// TODO(jwilson): validate type references and imports.
return new Profile(result.build());
return result.build();
}

/**
* Returns a multimap whose keys are base directories and whose values are potential locations of
* wire profile files.
*/
Multimap<Path, String> pathsToAttempt() {
Multimap<Path, String> pathsToAttempt(Set<Location> protoLocations) {
Multimap<Path, String> result = MultimapBuilder.linkedHashKeys().linkedHashSetValues().build();
for (Location location : protoLocations) {
pathsToAttempt(result, location);
Expand All @@ -103,7 +119,7 @@ Multimap<Path, String> pathsToAttempt() {
* and adds them to {@code result}.
*/
void pathsToAttempt(Multimap<Path, String> sink, Location location) {
Path base = Paths.get(location.base());
Path base = fileSystem.getPath(location.base());

String path = location.path();
while (!path.isEmpty()) {
Expand Down Expand Up @@ -132,6 +148,42 @@ private ProfileFileElement loadProfileFile(Path base, String path) throws IOExce
}
}

/** Confirms that {@code protoFiles} link correctly against {@code schema}. */
void validate(Schema schema, ImmutableList<ProfileFileElement> profileFiles) {
List<String> errors = new ArrayList<>();

for (ProfileFileElement profileFile : profileFiles) {
for (TypeConfigElement typeConfig : profileFile.typeConfigs()) {
ProtoType type = importedType(ProtoType.get(typeConfig.type()));
if (type == null) continue;

Type resolvedType = schema.getType(type);
if (resolvedType == null) {
errors.add(String.format("unable to resolve %s (%s)",
type, typeConfig.location()));
continue;
}

String requiredImport = resolvedType.location().path();
if (!profileFile.imports().contains(requiredImport)) {
errors.add(String.format("%s needs to import %s (%s)",
typeConfig.location().path(), requiredImport, typeConfig.location()));
}
}
}

if (!errors.isEmpty()) {
throw new SchemaException(errors);
}
}

/** Returns the type to import for {@code type}. */
private ProtoType importedType(ProtoType type) {
// Map key type is always scalar.
if (type.isMap()) type = type.valueType();
return type.isScalar() ? null : type;
}

private static Source source(Path base, String path) throws IOException {
Path resolvedPath = base.resolve(path);
if (Files.exists(resolvedPath)) {
Expand Down
127 changes: 93 additions & 34 deletions wire-java-generator/src/test/java/com/squareup/wire/java/ProfileLoaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,56 +16,59 @@
package com.squareup.wire.java;

import com.google.common.collect.ImmutableSet;
import com.google.common.jimfs.Configuration;
import com.google.common.jimfs.Jimfs;
import com.squareup.javapoet.ClassName;
import com.squareup.wire.schema.Location;
import com.squareup.wire.schema.ProtoType;
import com.squareup.wire.schema.Schema;
import com.squareup.wire.schema.SchemaException;
import com.squareup.wire.schema.SchemaLoader;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.FileSystem;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.nio.file.Path;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;
import org.assertj.core.data.MapEntry;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.fail;

public final class ProfileLoaderTest {
@Rule public final TemporaryFolder temp = new TemporaryFolder();
FileSystem fileSystem = Jimfs.newFileSystem(Configuration.unix());

@Test public void test() throws IOException {
temp.newFolder("a", "b", "c");
writeFile(temp.newFile("a/b/message1.proto"), ""
Files.createDirectories(fileSystem.getPath("/source/a/b/c"));
writeFile("/source/a/b/message1.proto", ""
+ "package a.b;\n"
+ "message Message1 {\n"
+ "}\n");
writeFile(temp.newFile("a/b/c/message2.proto"), ""
writeFile("/source/a/b/c/message2.proto", ""
+ "package a.b.c;\n"
+ "message Message1 {\n"
+ "message Message2 {\n"
+ "}\n");
writeFile(temp.newFile("android.wire"), ""
writeFile("/source/android.wire", ""
+ "syntax = \"wire2\";\n"
+ "import \"a/b/message1.proto\";\n"
+ "type a.b.Message1 {\n"
+ " target java.lang.Object using com.example.Message1#OBJECT_ADAPTER;\n"
+ "}\n");
writeFile(temp.newFile("a/b/c/android.wire"), ""
writeFile("/source/a/b/c/android.wire", ""
+ "syntax = \"wire2\";\n"
+ "import \"a/b/c/message2.proto\";\n"
+ "package a.b.c;\n"
+ "type a.b.c.Message2 {\n"
+ " target java.lang.String using com.example.Message2#STRING_ADAPTER;\n"
+ "}\n");

Schema schema = new SchemaLoader()
.addSource(temp.getRoot())
.addSource(fileSystem.getPath("/source"))
.load();
Profile profile = new ProfileLoader("android")
.addSchema(schema)
Profile profile = new ProfileLoader(fileSystem, "android")
.schema(schema)
.load();

ProtoType message1 = ProtoType.get("a.b.Message1");
Expand All @@ -78,25 +81,27 @@ public final class ProfileLoaderTest {
}

@Test public void profileInZip() throws IOException {
File file = temp.newFile();
ZipOutputStream zipOutputStream = new ZipOutputStream(new FileOutputStream(file));
Files.createDirectories(fileSystem.getPath("/source"));
Path zip = fileSystem.getPath("/source/protos.zip");
ZipOutputStream zipOutputStream = new ZipOutputStream(Files.newOutputStream(zip));
writeFile(zipOutputStream, "a/b/message.proto", ""
+ "package a.b;\n"
+ "message Message {"
+ "}");
writeFile(zipOutputStream, "a/b/android.wire", ""
+ "syntax = \"wire2\";\n"
+ "package a.b;\n"
+ "import \"a/b/message.proto\";\n"
+ "type a.b.Message {\n"
+ " target java.lang.Object using com.example.Message#ADAPTER;\n"
+ "}");
zipOutputStream.close();

Schema schema = new SchemaLoader()
.addSource(file)
.addSource(zip)
.load();
Profile profile = new ProfileLoader("android")
.addSchema(schema)
Profile profile = new ProfileLoader(fileSystem, "android")
.schema(schema)
.load();

ProtoType message = ProtoType.get("a.b.Message");
Expand All @@ -105,37 +110,91 @@ public final class ProfileLoaderTest {
}

@Test public void pathsToAttempt() throws Exception {
ProfileLoader loader = new ProfileLoader("android")
.addProtoLocation(Location.get("/a/b", "c/d/e.proto"));
assertThat(loader.pathsToAttempt().asMap()).containsExactly(
MapEntry.entry(Paths.get("/a/b"), ImmutableSet.of(
ImmutableSet<Location> locations = ImmutableSet.of(
Location.get("/a/b", "c/d/e.proto"));
ProfileLoader loader = new ProfileLoader(fileSystem, "android");
assertThat(loader.pathsToAttempt(locations).asMap()).containsExactly(
MapEntry.entry(fileSystem.getPath("/a/b"), ImmutableSet.of(
"c/d/android.wire",
"c/android.wire",
"android.wire")));
}

@Test public void pathsToAttemptMultipleRoots() throws Exception {
ProfileLoader loader = new ProfileLoader("android")
.addProtoLocation(Location.get("/a/b", "c/d/e.proto"))
.addProtoLocation(Location.get("/a/b", "c/f/g/h.proto"))
.addProtoLocation(Location.get("/i/j.zip", "k/l/m.proto"))
.addProtoLocation(Location.get("/i/j.zip", "k/l/m/n.proto"));
assertThat(loader.pathsToAttempt().asMap()).containsExactly(
MapEntry.entry(Paths.get("/a/b"), ImmutableSet.of(
ImmutableSet<Location> locations = ImmutableSet.of(
Location.get("/a/b", "c/d/e.proto"),
Location.get("/a/b", "c/f/g/h.proto"),
Location.get("/i/j.zip", "k/l/m.proto"),
Location.get("/i/j.zip", "k/l/m/n.proto"));
ProfileLoader loader = new ProfileLoader(fileSystem, "android");
assertThat(loader.pathsToAttempt(locations).asMap()).containsExactly(
MapEntry.entry(fileSystem.getPath("/a/b"), ImmutableSet.of(
"c/d/android.wire",
"c/android.wire",
"android.wire",
"c/f/g/android.wire",
"c/f/android.wire")),
MapEntry.entry(Paths.get("/i/j.zip"), ImmutableSet.of(
MapEntry.entry(fileSystem.getPath("/i/j.zip"), ImmutableSet.of(
"k/l/android.wire",
"k/android.wire",
"android.wire",
"k/l/m/android.wire")));
}

private void writeFile(File file, String content) throws IOException {
Files.write(file.toPath(), content.getBytes(UTF_8));
@Test public void unknownType() throws Exception {
Files.createDirectories(fileSystem.getPath("/source/a/b"));
writeFile("/source/a/b/message.proto", ""
+ "package a.b;\n"
+ "message Message {\n"
+ "}\n");
writeFile("/source/a/b/android.wire", ""
+ "syntax = \"wire2\";\n"
+ "type a.b.Message2 {\n"
+ " target java.lang.Object using com.example.Message#OBJECT_ADAPTER;\n"
+ "}\n");

Schema schema = new SchemaLoader()
.addSource(fileSystem.getPath("/source"))
.load();
try {
new ProfileLoader(fileSystem, "android")
.schema(schema)
.load();
fail();
} catch (SchemaException expected) {
assertThat(expected).hasMessage(
"unable to resolve a.b.Message2 (/source/a/b/android.wire at 2:1)");
}
}

@Test public void missingImport() throws Exception {
Files.createDirectories(fileSystem.getPath("/source/a/b"));
writeFile("/source/a/b/message.proto", ""
+ "package a.b;\n"
+ "message Message {\n"
+ "}\n");
writeFile("/source/a/b/android.wire", ""
+ "syntax = \"wire2\";\n"
+ "type a.b.Message {\n"
+ " target java.lang.Object using com.example.Message#OBJECT_ADAPTER;\n"
+ "}\n");

Schema schema = new SchemaLoader()
.addSource(fileSystem.getPath("/source"))
.load();
try {
new ProfileLoader(fileSystem, "android")
.schema(schema)
.load();
fail();
} catch (SchemaException expected) {
assertThat(expected).hasMessage(
"a/b/android.wire needs to import a/b/message.proto (/source/a/b/android.wire at 2:1)");
}
}

private void writeFile(String path, String content) throws IOException {
Files.write(fileSystem.getPath(path), content.getBytes(UTF_8));
}

private void writeFile(ZipOutputStream out, String file, String content) throws IOException {
Expand Down
Loading

0 comments on commit b2ec55c

Please sign in to comment.