From 5f4d2e810942215a4fb768b4009c766134fe8244 Mon Sep 17 00:00:00 2001 From: Noel Yap Date: Wed, 16 Sep 2020 14:23:48 -0700 Subject: [PATCH 1/4] What: Allow proto definition providers to specify the root of the proto directory hierarchy. Why: To allow greater flexibility in source code organization. --- README.md | 41 ++---- build.gradle | 17 ++- .../io/protop/core/manifest/Manifest.java | 11 +- .../io/protop/core/sync/FileWithRootDir.java | 37 +++++ .../java/io/protop/core/sync/SyncService.java | 94 +++++++++--- .../protop/core/sync/FileWithRootDirTest.java | 44 ++++++ .../io/protop/core/sync/SyncServiceTest.java | 139 ++++++++++++++++++ 7 files changed, 321 insertions(+), 62 deletions(-) create mode 100644 protop-core/src/main/java/io/protop/core/sync/FileWithRootDir.java create mode 100644 protop-core/src/test/java/io/protop/core/sync/FileWithRootDirTest.java create mode 100644 protop-core/src/test/java/io/protop/core/sync/SyncServiceTest.java diff --git a/README.md b/README.md index 1cde0e4..41f818f 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,13 @@ This will generate a manifest file named `protop.json`. } ``` +If the root of the proto files are a subdirectory (eg `src/main/proto`), add the following to `protop.json`: +```json + "root-dir": "src/main/proto" +``` + +This will allow clients not to have to adjust the protoc include path. + ## Publish ("link") locally This will make the project accessible to all other projects that run `sync` with links enabled. ```bash @@ -81,12 +88,12 @@ To unlink all projects system-wide: $ protop links clean ``` -### Publish to a repository* +### Publish to a repository ```bash $ protop publish -r=https://repository.example.com ``` -*There is an implementation of a Nexus plugin for protop required for this to work. More details [here](https://github.com/protop-io/nexus-repository-protop). Coming soon, there will be better documentation on the API of the repository itself. +> There is an implementation of a Nexus plugin for protop required for this to work. More details [here](https://github.com/protop-io/nexus-repository-protop). Coming soon, there will be better documentation on the API of the repository itself. ### Sync local/external dependencies Run the following with `-l` or `--use-links` to include local/linked dependencies, or run without it to only include dependencies from the network. @@ -120,35 +127,7 @@ $ protop cache clean ## Use with Gradle or other build tools ### Use with Gradle -There isn't a custom Gradle plugin for protop (yet). Even so, the implementation is quite simple. Assuming you have an existing `build.gradle` setup for a protobuf project, add the following task to the root project: -```groovy -task protop(type: Exec) { - workingDir "." - commandLine "protop", "sync" -} -``` -This task will simply run `protop sync`. To invoke it upon `gradle build` and ensure that it is run before the protos are generated, alter the `protobuf` block (or add it now): -```groovy -protobuf { - // ... - generateProtoTasks { - // ... - all().each { task -> task.dependsOn protop } - } -} -``` - -Finally, make sure the compiler will find all the synced protos: -```groovy -sourceSets { - main { - // ... - proto { - srcDir ".protop/path" - } - } -} -``` +Use the [Gradle plugin](https://github.com/google/protobuf-gradle-plugin). ### Use with protoc directly With dependencies already synced, you can call `protoc` in a project that looks like the one above: diff --git a/build.gradle b/build.gradle index ce5a938..98ce4cd 100644 --- a/build.gradle +++ b/build.gradle @@ -23,19 +23,24 @@ allprojects { subprojects { dependencies { + annotationProcessor "org.projectlombok:lombok:$lombokVersion" + + compileOnly "org.projectlombok:lombok:$lombokVersion" + + implementation "ch.qos.logback:logback-classic:1.2.3" implementation "com.google.guava:guava:27.0.1-jre" implementation "com.fasterxml.jackson.core:jackson-databind:2.9.9.3" implementation "com.fasterxml.jackson.core:jackson-annotations:2.9.9" - implementation "javax.validation:validation-api:2.0.1.Final" implementation "commons-io:commons-io:2.6" - implementation "ch.qos.logback:logback-classic:1.2.3" implementation "io.reactivex.rxjava2:rxjava:2.2.17" - - compileOnly "org.projectlombok:lombok:$lombokVersion" + implementation "javax.validation:validation-api:2.0.1.Final" - annotationProcessor "org.projectlombok:lombok:$lombokVersion" + testImplementation "org.assertj:assertj-core:3.+" + testImplementation "org.junit.jupiter:junit-jupiter-engine:5.+" + } - testImplementation "junit:junit:4.12" + test { + useJUnitPlatform() } tasks.withType(org.gradle.api.tasks.javadoc.Javadoc).all { enabled = false } diff --git a/protop-core/src/main/java/io/protop/core/manifest/Manifest.java b/protop-core/src/main/java/io/protop/core/manifest/Manifest.java index cd56b3b..e8f7554 100644 --- a/protop-core/src/main/java/io/protop/core/manifest/Manifest.java +++ b/protop-core/src/main/java/io/protop/core/manifest/Manifest.java @@ -23,7 +23,7 @@ @SuperBuilder @JsonInclude(JsonInclude.Include.NON_NULL) @JsonIgnoreProperties(ignoreUnknown = true) -@JsonPropertyOrder({"name", "org", "version", "description", "keywords", "license", "homepage"}) +@JsonPropertyOrder({"name", "org", "version", "description", "root-dir", "keywords", "license", "homepage"}) public class Manifest { private static final Logger logger = Logger.getLogger(Manifest.class); @@ -42,6 +42,9 @@ public class Manifest { // @JsonSerialize(converter = PathListToStringList.class) // private List include; + @JsonProperty("root-dir") + private Path rootDir; + @JsonInclude(JsonInclude.Include.NON_NULL) @JsonProperty("dependencies") private DependencyMap dependencies; @@ -67,6 +70,7 @@ public class Manifest { @JsonProperty("version") @NotNull Version version, @JsonProperty("organization") @NotNull String organization, // @JsonProperty("include") List include, + @JsonProperty("root-dir") @NotNull Path rootDir, @JsonProperty("dependencies") @JsonDeserialize(converter = DependencyMapDeserializer.class) DependencyMap dependencies, @JsonProperty("description") String description, @@ -78,6 +82,7 @@ public class Manifest { this.version = version; this.organization = organization; // this.include = Objects.isNull(include) ? ImmutableList.of() : ImmutableList.copyOf(include); + this.rootDir = rootDir; this.dependencies = dependencies; this.description = description; this.readme = readme; @@ -106,4 +111,8 @@ public static Optional from(Path directory) { throw new RuntimeException(message, e); } } + + public static Optional from(final File directory) { + return from(directory.toPath()); + } } diff --git a/protop-core/src/main/java/io/protop/core/sync/FileWithRootDir.java b/protop-core/src/main/java/io/protop/core/sync/FileWithRootDir.java new file mode 100644 index 0000000..eccfc50 --- /dev/null +++ b/protop-core/src/main/java/io/protop/core/sync/FileWithRootDir.java @@ -0,0 +1,37 @@ +package io.protop.core.sync; + +import lombok.EqualsAndHashCode; +import lombok.Getter; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +@EqualsAndHashCode +@Getter +class FileWithRootDir { + private final Path rootDir; + private final Path target; + + public FileWithRootDir(final Path rootDir, final File file) { + this(rootDir, file.toPath()); + } + + public FileWithRootDir(final Path rootDir, final Path target) { + this.rootDir = rootDir != null ? rootDir : Path.of(""); + this.target = target; + } + + public void createSymbolicLink(final Path protopPathDir, final Path linkWithRootDir) throws IOException { + final Path linkRelative = protopPathDir + .resolve(rootDir) + .relativize(linkWithRootDir); + final Path linkWithoutRootDir = protopPathDir.resolve(linkRelative); + + Files.createDirectories(linkWithoutRootDir.getParent()); + Files.createSymbolicLink( + linkWithoutRootDir, + target); + } +} diff --git a/protop-core/src/main/java/io/protop/core/sync/SyncService.java b/protop-core/src/main/java/io/protop/core/sync/SyncService.java index 62f4b2f..81a2d15 100644 --- a/protop-core/src/main/java/io/protop/core/sync/SyncService.java +++ b/protop-core/src/main/java/io/protop/core/sync/SyncService.java @@ -1,5 +1,6 @@ package io.protop.core.sync; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import io.protop.core.Context; import io.protop.core.auth.AuthService; @@ -7,22 +8,29 @@ import io.protop.core.logs.Logger; import io.protop.core.manifest.Coordinate; import io.protop.core.manifest.DependencyMap; +import io.protop.core.manifest.Manifest; import io.protop.core.manifest.revision.RevisionSource; import io.protop.core.storage.Storage; import io.protop.core.storage.StorageService; import io.protop.core.sync.status.SyncStatus; import io.protop.core.sync.status.Syncing; import io.reactivex.Observable; +import lombok.EqualsAndHashCode; import lombok.Getter; +import lombok.RequiredArgsConstructor; import org.apache.commons.io.FileUtils; import org.apache.commons.io.filefilter.TrueFileFilter; import java.io.File; import java.io.IOException; import java.nio.file.FileAlreadyExistsException; -import java.nio.file.Files; import java.nio.file.Path; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; @@ -97,20 +105,34 @@ private List getProtoFilesInDir(Path dir) { .collect(Collectors.toCollection(ArrayList::new)); } + @VisibleForTesting + @EqualsAndHashCode @Getter - private static class DirectoryWithFiles { - private final Map files = new HashMap<>(); - private final Map subdirectories = new HashMap<>(); + static class DirectoryWithFiles { + private final Map files; + private final Map subdirectories; + + DirectoryWithFiles() { + this(new HashMap<>(), new HashMap<>()); + } + + @VisibleForTesting + DirectoryWithFiles(final Map files, final Map subdirectories) { + this.files = files; + this.subdirectories = subdirectories; + } } private List validProtoFilesOrDirectories(File[] files) { List invalidDirectoryNames = ImmutableList.of("node_modules"); List validFiles = new ArrayList<>(); for (File file : files) { - if (!file.getName().startsWith(".") && !file.isHidden()) { - if (file.isDirectory() && !invalidDirectoryNames.contains(file.getName())) { + final String fileName = file.getName(); + + if (!fileName.startsWith(".") && !file.isHidden()) { + if (file.isDirectory() && !invalidDirectoryNames.contains(fileName)) { validFiles.add(file); - } else if (file.isFile() && file.getName().endsWith(".proto")) { + } else if (file.isFile() && fileName.endsWith(".proto")) { validFiles.add(file); } } @@ -121,16 +143,32 @@ private List validProtoFilesOrDirectories(File[] files) { /** * Filter the children directories and files and add to the given tree. */ - private void filterValidChildren(DirectoryWithFiles directoryWithFiles, File path) throws FileAlreadyExistsException { + private void filterValidChildren( + final DirectoryWithFiles directoryWithFiles, + final File project) throws FileAlreadyExistsException { + final Path rootDir = Manifest + .from(project) + .map(Manifest::getRootDir) + .orElse(Path.of("")); + + filterValidChildren(directoryWithFiles, project, rootDir); + } + + @VisibleForTesting + void filterValidChildren( + final DirectoryWithFiles directoryWithFiles, + final File path, + final Path rootDir) throws FileAlreadyExistsException { if (path.isDirectory()) { - File[] children = Optional.ofNullable(path.listFiles()) + final File[] children = Optional.ofNullable(path.listFiles()) .orElse(new File[]{}); + for (File child : validProtoFilesOrDirectories(children)) { if (child.isDirectory()) { DirectoryWithFiles childDirectory = directoryWithFiles.subdirectories.computeIfAbsent( child.getName(), name -> new DirectoryWithFiles()); - filterValidChildren(childDirectory, child); + filterValidChildren(childDirectory, child, rootDir); // If the child directory ended up with nothing in it, remove it. // This isn't the most efficient thing, but it does save us from having to traverse the // entire tree again in order to remove empty directories. @@ -145,7 +183,7 @@ private void filterValidChildren(DirectoryWithFiles directoryWithFiles, File pat path.getName()); throw new FileAlreadyExistsException(message); } else { - directoryWithFiles.files.put(child.getName(), child); + directoryWithFiles.files.put(child.getName(), new FileWithRootDir(rootDir, child)); } } } @@ -157,25 +195,33 @@ private void filterValidChildren(DirectoryWithFiles directoryWithFiles, File pat /** * Create subdirectories and symlink files in the given parent. */ - private void mergeChildrenToParentDirectory(File parent, DirectoryWithFiles children) throws IOException { - for (Map.Entry fileEntry : children.getFiles().entrySet()) { - Files.createSymbolicLink(parent.toPath().resolve(fileEntry.getKey()), - fileEntry.getValue().toPath()); + private void mergeChildrenToParentDirectory( + final Path protopPathDir, final Path parent, final DirectoryWithFiles children) throws IOException { + for (Map.Entry fileEntry : children.getFiles().entrySet()) { + final Path link = parent.resolve(fileEntry.getKey()); + + fileEntry.getValue().createSymbolicLink(protopPathDir, link); } + for (Map.Entry directoryEntry : children.getSubdirectories().entrySet()) { - Path created = Files.createDirectory(parent.toPath().resolve(directoryEntry.getKey())); - mergeChildrenToParentDirectory(created.toFile(), directoryEntry.getValue()); + final Path subdirectory = parent.resolve(directoryEntry.getKey()); + + mergeChildrenToParentDirectory(protopPathDir, subdirectory, directoryEntry.getValue()); } } - private void mergeDepsToPath(Path depsDir) throws IOException { - DirectoryWithFiles depsTree = new DirectoryWithFiles(); - File[] orgs = Optional.ofNullable(depsDir.toFile().listFiles()) + private void mergeDepsToPath(final Path depsDir) throws IOException { + final DirectoryWithFiles depsTree = new DirectoryWithFiles(); + final File[] orgs = Optional + .ofNullable(depsDir.toFile().listFiles()) .orElse(new File[]{}); + for (File org : orgs) { if (org.isDirectory()) { - File[] projects = Optional.ofNullable(org.listFiles()) + final File[] projects = Optional + .ofNullable(org.listFiles()) .orElse(new File[]{}); + for (File project : projects) { // For every project, traverse its items and add to the dependency tree. if (project.isDirectory()) { @@ -185,8 +231,8 @@ private void mergeDepsToPath(Path depsDir) throws IOException { } } - Path mergedDepsPath = resolveEmptySubDir(Storage.ProjectDirectory.PATH); - mergeChildrenToParentDirectory(mergedDepsPath.toFile(), depsTree); + final Path mergedDepsPath = resolveEmptySubDir(Storage.ProjectDirectory.PATH); + mergeChildrenToParentDirectory(mergedDepsPath, mergedDepsPath, depsTree); } /** diff --git a/protop-core/src/test/java/io/protop/core/sync/FileWithRootDirTest.java b/protop-core/src/test/java/io/protop/core/sync/FileWithRootDirTest.java new file mode 100644 index 0000000..a16652e --- /dev/null +++ b/protop-core/src/test/java/io/protop/core/sync/FileWithRootDirTest.java @@ -0,0 +1,44 @@ +package io.protop.core.sync; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; + +class FileWithRootDirTest { + Path protopPath; + Path protopDeps; + + @BeforeEach + public void setUp(@TempDir final Path workspace) { + protopPath = workspace.resolve(".protop/path"); + protopDeps = workspace.resolve(".protop/deps"); + } + + @Test + @DisplayName("Should create symlink using null rootDir.") + public void shouldCreateSymlinkUsingNullRootDir(@TempDir final Path workspace) throws Exception { + final var fileWithRootDir = new FileWithRootDir(null, protopDeps.resolve("org/name/package/file.proto")); + + fileWithRootDir.createSymbolicLink(protopPath, protopPath.resolve("package/file.proto")); + + assertThat(protopPath.resolve("package/file.proto")) + .isSymbolicLink(); + } + + @Test + @DisplayName("Should create symlink using rootDir.") + public void shouldCreateSymlinkUsingRootDir(@TempDir final Path workspace) throws Exception { + final var rootDir = Path.of("src/main/proto"); + final var fileWithRootDir = new FileWithRootDir(rootDir, protopDeps.resolve("org/name/src/main/proto/package/file.proto")); + + fileWithRootDir.createSymbolicLink(protopPath, protopPath.resolve("src/main/proto/package/file.proto")); + + assertThat(protopPath.resolve("package/file.proto")) + .isSymbolicLink(); + } +} \ No newline at end of file diff --git a/protop-core/src/test/java/io/protop/core/sync/SyncServiceTest.java b/protop-core/src/test/java/io/protop/core/sync/SyncServiceTest.java new file mode 100644 index 0000000..794df67 --- /dev/null +++ b/protop-core/src/test/java/io/protop/core/sync/SyncServiceTest.java @@ -0,0 +1,139 @@ +package io.protop.core.sync; + +import org.assertj.core.api.SoftAssertions; +import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.AbstractMap; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +@ExtendWith(SoftAssertionsExtension.class) +class SyncServiceTest { + Path workspace; + + @BeforeEach + public void setUp(@TempDir Path workspace) { + this.workspace = workspace; + } + + @Test + @DisplayName("Should throw exception for non-directory.") + public void shouldThrowExceptionForNonDirectory() throws Exception { + final var syncService = new SyncService(null, null, null); + + final var file = workspace.resolve("file"); + + Files.createFile(file); + + assertThatThrownBy(() -> syncService.filterValidChildren(null, file.toFile(), null)) + .isInstanceOf(RuntimeException.class); + } + + @Test + @DisplayName("Should handle empty directory.") + public void shouldHandleEmptyDirectory(final SoftAssertions softly) throws Exception { + final var syncService = new SyncService(null, null, null); + + final var directoryWithFiles = new SyncService.DirectoryWithFiles(); + + syncService.filterValidChildren(directoryWithFiles, workspace.toFile(), Path.of("")); + + softly.assertThat(directoryWithFiles.getFiles()) + .isEmpty(); + softly.assertThat(directoryWithFiles.getSubdirectories()) + .isEmpty(); + } + + @Test + @DisplayName("Should handle non-proto file.") + public void shouldHandleNonProtoFile(final SoftAssertions softly) throws Exception { + final var syncService = new SyncService(null, null, null); + + final var file = workspace.resolve("file"); + + Files.createFile(file); + + final var directoryWithFiles = new SyncService.DirectoryWithFiles(); + + syncService.filterValidChildren(directoryWithFiles, workspace.toFile(), Path.of("")); + + softly.assertThat(directoryWithFiles.getFiles()) + .isEmpty(); + softly.assertThat(directoryWithFiles.getSubdirectories()) + .isEmpty(); + } + + @Test + @DisplayName("Should handle proto file.") + public void shouldHandleProtoFile(final SoftAssertions softly) throws Exception { + final var syncService = new SyncService(null, null, null); + + final var file = workspace.resolve("file.proto"); + + Files.createFile(file); + + final var directoryWithFiles = new SyncService.DirectoryWithFiles(); + + syncService.filterValidChildren(directoryWithFiles, workspace.toFile(), Path.of("")); + + // noinspection unchecked + softly.assertThat(directoryWithFiles.getFiles()) + .containsExactly(new AbstractMap.SimpleEntry<>("file.proto", new FileWithRootDir(null, file))); + softly.assertThat(directoryWithFiles.getSubdirectories()) + .isEmpty(); + } + + @Test + @DisplayName("Should throw exception for already defined proto file.") + public void shouldThrowExceptionForAlreadyDefinedProtoFile() throws Exception { + final var syncService = new SyncService(null, null, null); + + final var file = workspace.resolve("file.proto"); + + Files.createFile(file); + + final var directoryWithFiles = new SyncService.DirectoryWithFiles(); + directoryWithFiles.getFiles().put("file.proto", new FileWithRootDir(null, file)); + + assertThatThrownBy(() -> syncService.filterValidChildren(directoryWithFiles, workspace.toFile(), Path.of(""))) + .isInstanceOf(FileAlreadyExistsException.class); + } + + @Test + @DisplayName("Should handle proto files in subdirectories.") + public void shouldHandleProtoFilesInSubdirectories(final SoftAssertions softly) throws Exception { + final var syncService = new SyncService(null, null, null); + + final var directory = workspace.resolve("directory"); + final var file = directory.resolve("file.proto"); + + Files.createDirectories(file.getParent()); + Files.createFile(file); + + final var directoryWithFiles = new SyncService.DirectoryWithFiles(); + + syncService.filterValidChildren(directoryWithFiles, workspace.toFile(), Path.of("")); + + softly.assertThat(directoryWithFiles.getFiles()) + .isEmpty(); + // noinspection unchecked + softly.assertThat(directoryWithFiles.getSubdirectories()) + .containsExactly( + new AbstractMap.SimpleEntry<>( + "directory", + new SyncService.DirectoryWithFiles( + Collections.singletonMap("file.proto", new FileWithRootDir(null, file)), + new HashMap<>()))); + } +} \ No newline at end of file From a90bbfdd9a5adf3bca4a294480ed851e487f37c3 Mon Sep 17 00:00:00 2001 From: Noel Yap Date: Mon, 19 Oct 2020 21:40:26 -0700 Subject: [PATCH 2/4] Test added to ensure symlinking works even when project root directory is specified. --- protop-core/build.gradle | 15 ++++-- .../io/protop/core/sync/FileWithRootDir.java | 5 +- .../java/io/protop/core/sync/SyncService.java | 5 +- .../.protop/deps/org0/project00/protop.json | 8 ++++ .../src/main/proto/package000/file000.proto | 0 .../package000/package0000/file0000.proto | 0 .../src/main/proto/package001/file001.proto | 0 .../src/main/proto/package010/file010.proto | 0 .../src/main/proto/package100/file100.proto | 0 .../protop/core/sync/FileWithRootDirTest.java | 19 ++++++-- .../io/protop/core/sync/SyncServiceTest.java | 46 +++++++++++++++++-- 11 files changed, 82 insertions(+), 16 deletions(-) create mode 100644 protop-core/src/test/data/.protop/deps/org0/project00/protop.json create mode 100644 protop-core/src/test/data/.protop/deps/org0/project00/src/main/proto/package000/file000.proto create mode 100644 protop-core/src/test/data/.protop/deps/org0/project00/src/main/proto/package000/package0000/file0000.proto create mode 100644 protop-core/src/test/data/.protop/deps/org0/project00/src/main/proto/package001/file001.proto create mode 100644 protop-core/src/test/data/.protop/deps/org0/project01/src/main/proto/package010/file010.proto create mode 100644 protop-core/src/test/data/.protop/deps/org1/project10/src/main/proto/package100/file100.proto diff --git a/protop-core/build.gradle b/protop-core/build.gradle index f2bdd96..339274e 100644 --- a/protop-core/build.gradle +++ b/protop-core/build.gradle @@ -1,6 +1,13 @@ dependencies { - implementation "org.apache.httpcomponents:httpclient:4.5.11" - implementation "org.apache.httpcomponents:httpmime:4.5.11" - implementation "org.apache.commons:commons-compress:1.19" - implementation "org.eclipse.jgit:org.eclipse.jgit:5.7.0.202003110725-r" + implementation 'org.apache.commons:commons-compress:1.19' + implementation 'org.apache.httpcomponents:httpclient:4.5.11' + implementation 'org.apache.httpcomponents:httpmime:4.5.11' + implementation 'org.eclipse.jgit:org.eclipse.jgit:5.7.0.202003110725-r' + + testImplementation 'org.mockito:mockito-core:3.3.3' + testImplementation 'org.mockito:mockito-junit-jupiter:3.3.3' +} + +test { + environment 'TEST_DATA_DIR', "${projectDir}/src/test/data" } diff --git a/protop-core/src/main/java/io/protop/core/sync/FileWithRootDir.java b/protop-core/src/main/java/io/protop/core/sync/FileWithRootDir.java index eccfc50..fac0b0b 100644 --- a/protop-core/src/main/java/io/protop/core/sync/FileWithRootDir.java +++ b/protop-core/src/main/java/io/protop/core/sync/FileWithRootDir.java @@ -30,8 +30,7 @@ public void createSymbolicLink(final Path protopPathDir, final Path linkWithRoot final Path linkWithoutRootDir = protopPathDir.resolve(linkRelative); Files.createDirectories(linkWithoutRootDir.getParent()); - Files.createSymbolicLink( - linkWithoutRootDir, - target); + Files.deleteIfExists(linkWithoutRootDir); + Files.createSymbolicLink(linkWithoutRootDir, target); } } diff --git a/protop-core/src/main/java/io/protop/core/sync/SyncService.java b/protop-core/src/main/java/io/protop/core/sync/SyncService.java index 81a2d15..a3670b4 100644 --- a/protop-core/src/main/java/io/protop/core/sync/SyncService.java +++ b/protop-core/src/main/java/io/protop/core/sync/SyncService.java @@ -195,7 +195,7 @@ void filterValidChildren( /** * Create subdirectories and symlink files in the given parent. */ - private void mergeChildrenToParentDirectory( + private static void mergeChildrenToParentDirectory( final Path protopPathDir, final Path parent, final DirectoryWithFiles children) throws IOException { for (Map.Entry fileEntry : children.getFiles().entrySet()) { final Path link = parent.resolve(fileEntry.getKey()); @@ -210,7 +210,8 @@ private void mergeChildrenToParentDirectory( } } - private void mergeDepsToPath(final Path depsDir) throws IOException { + @VisibleForTesting + void mergeDepsToPath(final Path depsDir) throws IOException { final DirectoryWithFiles depsTree = new DirectoryWithFiles(); final File[] orgs = Optional .ofNullable(depsDir.toFile().listFiles()) diff --git a/protop-core/src/test/data/.protop/deps/org0/project00/protop.json b/protop-core/src/test/data/.protop/deps/org0/project00/protop.json new file mode 100644 index 0000000..febf5ec --- /dev/null +++ b/protop-core/src/test/data/.protop/deps/org0/project00/protop.json @@ -0,0 +1,8 @@ +{ + "organization" : "org1", + "name" : "project10", + "version" : "0.1.0", + + "root-dir": "src/main/proto" +} + diff --git a/protop-core/src/test/data/.protop/deps/org0/project00/src/main/proto/package000/file000.proto b/protop-core/src/test/data/.protop/deps/org0/project00/src/main/proto/package000/file000.proto new file mode 100644 index 0000000..e69de29 diff --git a/protop-core/src/test/data/.protop/deps/org0/project00/src/main/proto/package000/package0000/file0000.proto b/protop-core/src/test/data/.protop/deps/org0/project00/src/main/proto/package000/package0000/file0000.proto new file mode 100644 index 0000000..e69de29 diff --git a/protop-core/src/test/data/.protop/deps/org0/project00/src/main/proto/package001/file001.proto b/protop-core/src/test/data/.protop/deps/org0/project00/src/main/proto/package001/file001.proto new file mode 100644 index 0000000..e69de29 diff --git a/protop-core/src/test/data/.protop/deps/org0/project01/src/main/proto/package010/file010.proto b/protop-core/src/test/data/.protop/deps/org0/project01/src/main/proto/package010/file010.proto new file mode 100644 index 0000000..e69de29 diff --git a/protop-core/src/test/data/.protop/deps/org1/project10/src/main/proto/package100/file100.proto b/protop-core/src/test/data/.protop/deps/org1/project10/src/main/proto/package100/file100.proto new file mode 100644 index 0000000..e69de29 diff --git a/protop-core/src/test/java/io/protop/core/sync/FileWithRootDirTest.java b/protop-core/src/test/java/io/protop/core/sync/FileWithRootDirTest.java index a16652e..e956868 100644 --- a/protop-core/src/test/java/io/protop/core/sync/FileWithRootDirTest.java +++ b/protop-core/src/test/java/io/protop/core/sync/FileWithRootDirTest.java @@ -21,7 +21,7 @@ public void setUp(@TempDir final Path workspace) { @Test @DisplayName("Should create symlink using null rootDir.") - public void shouldCreateSymlinkUsingNullRootDir(@TempDir final Path workspace) throws Exception { + public void shouldCreateSymlinkUsingNullRootDir() throws Exception { final var fileWithRootDir = new FileWithRootDir(null, protopDeps.resolve("org/name/package/file.proto")); fileWithRootDir.createSymbolicLink(protopPath, protopPath.resolve("package/file.proto")); @@ -32,7 +32,7 @@ public void shouldCreateSymlinkUsingNullRootDir(@TempDir final Path workspace) t @Test @DisplayName("Should create symlink using rootDir.") - public void shouldCreateSymlinkUsingRootDir(@TempDir final Path workspace) throws Exception { + public void shouldCreateSymlinkUsingRootDir() throws Exception { final var rootDir = Path.of("src/main/proto"); final var fileWithRootDir = new FileWithRootDir(rootDir, protopDeps.resolve("org/name/src/main/proto/package/file.proto")); @@ -41,4 +41,17 @@ public void shouldCreateSymlinkUsingRootDir(@TempDir final Path workspace) throw assertThat(protopPath.resolve("package/file.proto")) .isSymbolicLink(); } -} \ No newline at end of file + + @Test + @DisplayName("Symlink creation should be idempotent.") + public void symlinkCreationShouldBeIdempotent() throws Exception { + final var rootDir = Path.of("src/main/proto"); + final var fileWithRootDir = new FileWithRootDir(rootDir, protopDeps.resolve("org/name/src/main/proto/package/file.proto")); + + fileWithRootDir.createSymbolicLink(protopPath, protopPath.resolve("src/main/proto/package/file.proto")); + fileWithRootDir.createSymbolicLink(protopPath, protopPath.resolve("src/main/proto/package/file.proto")); + + assertThat(protopPath.resolve("package/file.proto")) + .isSymbolicLink(); + } +} diff --git a/protop-core/src/test/java/io/protop/core/sync/SyncServiceTest.java b/protop-core/src/test/java/io/protop/core/sync/SyncServiceTest.java index 794df67..29e68b4 100644 --- a/protop-core/src/test/java/io/protop/core/sync/SyncServiceTest.java +++ b/protop-core/src/test/java/io/protop/core/sync/SyncServiceTest.java @@ -1,5 +1,9 @@ package io.protop.core.sync; +import io.protop.core.Context; +import io.protop.core.manifest.Manifest; +import io.protop.core.storage.StorageService; +import org.apache.commons.io.FileUtils; import org.assertj.core.api.SoftAssertions; import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension; import org.junit.jupiter.api.BeforeEach; @@ -7,19 +11,24 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import java.io.File; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; import java.util.AbstractMap; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.when; -@ExtendWith(SoftAssertionsExtension.class) +@ExtendWith({MockitoExtension.class, SoftAssertionsExtension.class}) class SyncServiceTest { + final File TEST_DATA_DIR = new File(System.getenv("TEST_DATA_DIR")); + Path workspace; @BeforeEach @@ -134,6 +143,35 @@ public void shouldHandleProtoFilesInSubdirectories(final SoftAssertions softly) "directory", new SyncService.DirectoryWithFiles( Collections.singletonMap("file.proto", new FileWithRootDir(null, file)), - new HashMap<>()))); + Collections.emptyMap()))); + } + + @Test + @DisplayName("Should link all proto files.") + public void shouldLinkAllProtoFiles( + final SoftAssertions softly, + @Mock final Context contextMock) throws Exception { + when(contextMock.getProjectLocation()) + .thenReturn(workspace); + + final var syncService = new SyncService(null, new StorageService(), contextMock); + + FileUtils.copyDirectory(TEST_DATA_DIR, workspace.toFile()); + + final var protopDir = workspace.resolve(".protop"); + final var depsDir = protopDir.resolve("deps"); + + syncService.mergeDepsToPath(depsDir); + + final var pathDir = protopDir.resolve("path"); + + softly.assertThat(pathDir.resolve("package000/file000.proto")) + .isSymbolicLink(); + softly.assertThat(pathDir.resolve("package000/package0000/file0000.proto")) + .isSymbolicLink(); + softly.assertThat(pathDir.resolve("package001/file001.proto")) + .isSymbolicLink(); + softly.assertThat(pathDir.resolve("src/main/proto/package100/file100.proto")) + .isSymbolicLink(); } -} \ No newline at end of file +} From 0d472ece3652c38258587fde121d3414a64b2558 Mon Sep 17 00:00:00 2001 From: Noel Yap Date: Tue, 20 Oct 2020 07:20:22 -0700 Subject: [PATCH 3/4] CI enabled. --- .github/workflows/check-pull-request.yml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .github/workflows/check-pull-request.yml diff --git a/.github/workflows/check-pull-request.yml b/.github/workflows/check-pull-request.yml new file mode 100644 index 0000000..dfd8e01 --- /dev/null +++ b/.github/workflows/check-pull-request.yml @@ -0,0 +1,23 @@ +# When a release branch is merged into master, builds the artifacts and creates a release. + +name: Build Release + +on: + pull_request: + branches: + - development + +jobs: + build_artifacts: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + - name: Set up JDK 13 + uses: actions/setup-java@v1 + with: + java-version: 13 + - name: Grant execute permission for gradlew + run: chmod +x gradlew + - name: Build with Gradle + run: ./gradlew check From c2ebc989f58b9432ee2a608543d5ff26de1a7b26 Mon Sep 17 00:00:00 2001 From: Noel Yap Date: Wed, 21 Oct 2020 18:38:46 -0700 Subject: [PATCH 4/4] Build-Jdk setting fixed. --- build.gradle | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index 98ce4cd..360d9c6 100644 --- a/build.gradle +++ b/build.gradle @@ -66,16 +66,16 @@ task distFatJar(type: Jar) { 'Built-By' : System.properties['user.name'], 'Build-Timestamp': new java.text.SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date()), 'Created-By' : "Gradle ${gradle.gradleVersion}", - 'Build-Jdk' : "${System.properties['java.revision']}", + 'Build-Jdk' : "${System.properties['java.version']}", "Main-Class" : "io.protop.cli.ProtopCli" ) } from sourceSets.main.output - + // Do the rest of this task as such because https://stackoverflow.com/questions/999489/invalid-signature-file-when-attempting-to-run-a-jar doFirst { - + from configurations.runtimeClasspath. findAll { it.name.endsWith('jar') }. collect { zipTree(it) }