Repository: smithy-lang/smithy
Pull request #1885 (2023-07-27 17:06:40)

Check for existing directory when creating template with init
*Description of changes:*
Updates the smithy init command to fail fast if the directory it is attempting to use as an output directory already exists.

Previously the command did not fail until after cloning the repo to a temp directory

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c283151d5..1981809c1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,9 +1,9 @@
 # Smithy Changelog
 
-## 1.35.0 (2023-07-24)
+## 1.35.0 (2023-07-27)
 
 ### Features
-* Enabled support for SNAPSHOT dependencies ([#1853](https://github.com/smithy-lang/smithy/pull/1853), [#1857](https://github.com/smithy-lang/smithy/pull/1857))
+* Enabled support for SNAPSHOT dependencies ([#1853](https://github.com/smithy-lang/smithy/pull/1853), [#1857](https://github.com/smithy-lang/smithy/pull/1857), [#1884](https://github.com/smithy-lang/smithy/pull/1884))
 * Enabled default mode for `smithy diff` rather than failing when not set ([#1856](https://github.com/smithy-lang/smithy/pull/1856))
 * Added warning to mis-cased standard HTTP verbs ([#1862](https://github.com/smithy-lang/smithy/pull/1862))
 * Relaxed type constraints for `pageSize` property of the `@paginated` trait ([#1866](https://github.com/smithy-lang/smithy/pull/1866))
diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/model/MavenRepository.java b/smithy-build/src/main/java/software/amazon/smithy/build/model/MavenRepository.java
index 0144db2dc..e34263cdb 100644
--- a/smithy-build/src/main/java/software/amazon/smithy/build/model/MavenRepository.java
+++ b/smithy-build/src/main/java/software/amazon/smithy/build/model/MavenRepository.java
@@ -26,10 +26,12 @@ public final class MavenRepository implements ToSmithyBuilder<MavenRepository> {
 
     private final String url;
     private final String httpCredentials;
+    private final String id;
 
     public MavenRepository(Builder builder) {
         this.url = SmithyBuilder.requiredState("url", builder.url);
         this.httpCredentials = builder.httpCredentials;
+        this.id = builder.id;
     }
 
     public static Builder builder() {
@@ -41,7 +43,8 @@ public final class MavenRepository implements ToSmithyBuilder<MavenRepository> {
         node.expectObjectNode()
                 .warnIfAdditionalProperties(Arrays.asList("url", "httpCredentials"))
                 .expectStringMember("url", builder::url)
-                .getStringMember("httpCredentials", builder::httpCredentials);
+                .getStringMember("httpCredentials", builder::httpCredentials)
+                .getStringMember("id", builder::id);
         return builder.build();
     }
 
@@ -49,18 +52,22 @@ public final class MavenRepository implements ToSmithyBuilder<MavenRepository> {
         return url;
     }
 
+    public Optional<String> getId() {
+        return Optional.ofNullable(id);
+    }
+
     public Optional<String> getHttpCredentials() {
         return Optional.ofNullable(httpCredentials);
     }
 
     @Override
     public Builder toBuilder() {
-        return builder().url(url).httpCredentials(httpCredentials);
+        return builder().id(id).url(url).httpCredentials(httpCredentials);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(url, httpCredentials);
+        return Objects.hash(id, url, httpCredentials);
     }
 
     @Override
@@ -71,12 +78,16 @@ public final class MavenRepository implements ToSmithyBuilder<MavenRepository> {
             return false;
         }
         MavenRepository mavenRepo = (MavenRepository) o;
-        return Objects.equals(url, mavenRepo.url) && Objects.equals(httpCredentials, mavenRepo.httpCredentials);
+        return
+            Objects.equals(id, mavenRepo.id)
+                && Objects.equals(url, mavenRepo.url)
+                && Objects.equals(httpCredentials, mavenRepo.httpCredentials);
     }
 
     public static final class Builder implements SmithyBuilder<MavenRepository> {
         private String url;
         private String httpCredentials;
+        private String id;
 
         private Builder() {}
 
@@ -90,6 +101,11 @@ public final class MavenRepository implements ToSmithyBuilder<MavenRepository> {
             return this;
         }
 
+        public Builder id(String id) {
+            this.id = id;
+            return this;
+        }
+
         public Builder httpCredentials(String httpCredentials) {
             this.httpCredentials = httpCredentials;
             if (httpCredentials != null) {
diff --git a/smithy-cli/src/it/resources/software/amazon/smithy/cli/projects/maven-multiple-repos/smithy-build.json b/smithy-cli/src/it/resources/software/amazon/smithy/cli/projects/maven-multiple-repos/smithy-build.json
new file mode 100644
index 000000000..3af06f626
--- /dev/null
+++ b/smithy-cli/src/it/resources/software/amazon/smithy/cli/projects/maven-multiple-repos/smithy-build.json
@@ -0,0 +1,20 @@
+{
+    "version": "1.0",
+    "maven": {
+        "repositories": [
+            {
+                // Use HTTP instead of HTTPS because we're running a mock server during tests
+                "id": "Artifactory 1",
+                "url": "http://localhost:1234/artifactory-1"
+            },
+            {
+                // Use HTTP instead of HTTPS because we're running a mock server during tests
+                "id": "Artifactory 2",
+                "url": "http://localhost:1234/artifactory-2"
+            }
+        ],
+        "dependencies": [
+            "com.example:artifact:1.0.0-SNAPSHOT"
+        ]
+    }
+}
diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/InitCommand.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/InitCommand.java
index 2a78b21bb..5adac8b1b 100644
--- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/InitCommand.java
+++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/InitCommand.java
@@ -160,6 +160,15 @@ final class InitCommand implements Command {
             throw new IllegalArgumentException("Please specify a template name using `--template` or `-t`");
         }
 
+        // Use templateName if directory is not specified
+        if (directory == null) {
+            directory = template;
+        }
+        final Path dest = Paths.get(directory);
+        if (Files.exists(dest)) {
+            throw new CliError("Output directory `" + directory + "` already exists.");
+        }
+
         ObjectNode templatesNode = getTemplatesNode(smithyTemplatesNode);
         if (!templatesNode.containsMember(template)) {
             throw new IllegalArgumentException(String.format(
@@ -180,12 +189,8 @@ final class InitCommand implements Command {
         }
         exec(ListUtils.of("git", "checkout"), temp);
 
-        // Use templateName if directory is not specified
-        if (directory == null) {
-            directory = template;
-        }
 
-        final Path dest = Paths.get(directory);
+
         IoUtils.copyDir(Paths.get(temp.toString(), templatePath), dest);
         copyIncludedFiles(temp.toString(), dest.toString(), includedFiles, template, env);
 
diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolver.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolver.java
index b10d0da50..86c75ad07 100644
--- a/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolver.java
+++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolver.java
@@ -96,11 +96,12 @@ public final class MavenDependencyResolver implements DependencyResolver {
     public void addRepository(MavenRepository repository) {
         try {
             URI uri = new URI(repository.getUrl());
-            String name = uri.getHost();
+            int repositoryIndex = remoteRepositories.size() + 1;
+            String id = repository.getId().orElseGet(() -> repositoryIndex + "|" + uri.getHost());
             String userInfo = uri.getUserInfo();
-            RemoteRepository.Builder builder = new RemoteRepository.Builder(name, "default", repository.getUrl());
+            RemoteRepository.Builder builder = new RemoteRepository.Builder(id, "default", repository.getUrl());
             if (userInfo != null) {
-                LOGGER.finest(() -> "Setting username and password for " + name + " using URI authority");
+                LOGGER.finest(() -> "Setting username and password for " + id + " using URI authority");
                 addUserInfoAuth(uri, userInfo, builder);
             }
             repository.getHttpCredentials().ifPresent(credentials -> addUserInfoAuth(uri, credentials, builder));


Test patch:
diff --git a/smithy-cli/src/it/java/software/amazon/smithy/cli/InitCommandTest.java b/smithy-cli/src/it/java/software/amazon/smithy/cli/InitCommandTest.java
index 46252aca0..349610068 100644
--- a/smithy-cli/src/it/java/software/amazon/smithy/cli/InitCommandTest.java
+++ b/smithy-cli/src/it/java/software/amazon/smithy/cli/InitCommandTest.java
@@ -208,6 +208,30 @@ public class InitCommandTest {
         });
     }
 
+    @Test
+    public void outputDirectoryAlreadyExists() {
+        IntegUtils.withProject(PROJECT_NAME, templatesDir -> {
+            setupTemplatesDirectory(templatesDir);
+
+            IntegUtils.withTempDir("existingOutputDir", dir -> {
+                Path existingPath = null;
+                RunResult result;
+                try {
+                    existingPath = Files.createDirectory(dir.resolve("quickstart-cli"));
+                    result = IntegUtils.run(
+                            dir, ListUtils.of("init", "-t", "quickstart-cli", "-u", templatesDir.toString()));
+                } catch (IOException e) {
+                    throw new RuntimeException(e);
+                } finally {
+                    IoUtils.rmdir(existingPath);
+                }
+
+                assertThat(result.getOutput(), containsString("Output directory `quickstart-cli` already exists."));
+                assertThat(result.getExitCode(), is(1));
+            });
+        });
+    }
+
     private static void run(List<String> args, Path root) {
         StringBuilder output = new StringBuilder();
         int result = IoUtils.runCommand(args, root, output, Collections.emptyMap());
diff --git a/smithy-cli/src/it/java/software/amazon/smithy/cli/MavenResolverMultipleReposTest.java b/smithy-cli/src/it/java/software/amazon/smithy/cli/MavenResolverMultipleReposTest.java
new file mode 100644
index 000000000..87a6eda4a
--- /dev/null
+++ b/smithy-cli/src/it/java/software/amazon/smithy/cli/MavenResolverMultipleReposTest.java
@@ -0,0 +1,158 @@
+package software.amazon.smithy.cli;
+
+import org.apache.commons.codec.digest.DigestUtils;
+import org.junit.jupiter.api.Test;
+import org.mockserver.integration.ClientAndServer;
+import org.mockserver.model.HttpRequest;
+import org.mockserver.model.HttpResponse;
+import software.amazon.smithy.utils.ListUtils;
+
+import java.util.Collections;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.containsString;
+import static org.mockserver.integration.ClientAndServer.startClientAndServer;
+
+public class MavenResolverMultipleReposTest {
+
+    @Test
+    public void multipleRepositoriesOnSameHostName() {
+        ClientAndServer mockServer = null;
+        try {
+            mockServer = startClientAndServer(1234);
+
+            // artifact
+            mockArtifactAndSha(
+                mockServer,
+                "/artifactory-2/com/example/artifact/1.0.0-SNAPSHOT/artifact-1.0.0-20230724.184336-2.jar",
+                "FAKE JAR CONTENT"
+            );
+
+            mockArtifactAndSha(
+                mockServer,
+                "/artifactory-2/com/example/artifact/1.0.0-SNAPSHOT/artifact-1.0.0-20230724.184336-1.pom",
+                pomFileContents()
+            );
+
+            mockArtifactAndSha(
+                    mockServer,
+                    "/artifactory-2/com/example/artifact/1.0.0-SNAPSHOT/maven-metadata.xml",
+                    metadataFileContents()
+            );
+
+            // dependency
+            mockArtifactAndSha(
+                mockServer,
+                "/artifactory-1/com/example/dependency/1.0.0/dependency-1.0.0.jar",
+                "ANOTHER FAKE JAR"
+            );
+
+            mockNotFound(mockServer, "/artifactory-1/.*");
+            mockNotFound(mockServer, "/artifactory-2/.*");
+
+            IntegUtils.runWithEmptyCache("maven-multiple-repos", ListUtils.of("validate", "--debug"),
+                    Collections.emptyMap(), result -> {
+                        assertThat(result.getExitCode(), equalTo(0));
+                        assertThat(result.getOutput(), containsString("software.amazon.smithy.cli.dependencies.DependencyResolver - Resolved Maven dependencies: [com.example:artifact:jar:1.0.0-20230724.184336-2"));
+                    });
+        } finally {
+            if(mockServer!=null) {
+                mockServer.stop();
+            }
+        }
+    }
+
+    private void mockArtifactAndSha(ClientAndServer mockServer, String path, String contents) {
+        mockSuccess(
+            mockServer,
+            path,
+            contents
+        );
+
+        mockSuccess(
+            mockServer,
+        path + ".sha1",
+            DigestUtils.sha1Hex(contents)
+        );
+    }
+
+    private String metadataFileContents() {
+        return
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                "<metadata modelVersion=\"1.1.0\">\n" +
+                "  <groupId>com.example</groupId>\n" +
+                "  <artifactId>artifact</artifactId>\n" +
+                "  <version>1.0.0-SNAPSHOT</version>\n" +
+                "  <versioning>\n" +
+                "    <snapshot>\n" +
+                "      <timestamp>20230724.184336</timestamp>\n" +
+                "      <buildNumber>1</buildNumber>\n" +
+                "    </snapshot>\n" +
+                "    <lastUpdated>20230724184336</lastUpdated>\n" +
+                "    <snapshotVersions>\n" +
+                "      <snapshotVersion>\n" +
+                "        <extension>jar</extension>\n" +
+                "        <value>1.0.0-20230724.184336-2</value>\n" +
+                "        <updated>20230724184336</updated>\n" +
+                "      </snapshotVersion>\n" +
+                "      <snapshotVersion>\n" +
+                "        <extension>pom</extension>\n" +
+                "        <value>1.0.0-20230724.184336-1</value>\n" +
+                "        <updated>20230724184336</updated>\n" +
+                "      </snapshotVersion>\n" +
+                "    </snapshotVersions>\n" +
+                "  </versioning>\n" +
+                "</metadata>\n";
+    }
+
+    private String pomFileContents() {
+        return
+            "<?xml version='1.0' encoding='UTF-8'?>\n" +
+            "<project xsi:schemaLocation=\"http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xmlns=\"http://maven.apache.org/POM/4.0.0\">\n" +
+            "    <modelVersion>4.0.0</modelVersion>\n" +
+            "    <groupId>com.example</groupId>\n" +
+            "    <artifactId>artifact</artifactId>\n" +
+            "    <packaging>jar</packaging>\n" +
+            "    <description>artifact</description>\n" +
+            "    <version>1.0.0-SNAPSHOT</version>\n" +
+            "    <name>artifact</name>\n" +
+            "    <organization>\n" +
+            "        <name>com.example</name>\n" +
+            "    </organization>\n" +
+            "    <properties>\n" +
+            "        <info.versionScheme>early-semver</info.versionScheme>\n" +
+            "    </properties>\n" +
+            "    <dependencies>\n" +
+            "        <dependency>\n" +
+            "            <groupId>com.example</groupId>\n" +
+            "            <artifactId>dependency</artifactId>\n" +
+            "            <version>1.0.0</version>\n" +
+            "        </dependency>\n" +
+            "    </dependencies>\n" +
+            "</project>";
+    }
+
+    private static void mockNotFound(ClientAndServer mockServer, String path) {
+        mockServer.when(
+                HttpRequest
+                        .request()
+                        .withMethod("GET")
+                        .withPath(path)
+        ).respond(HttpResponse.notFoundResponse());
+    }
+
+    private static void mockSuccess(ClientAndServer mockServer, String path, String responseBody) {
+        mockServer.when(
+                HttpRequest
+                        .request()
+                        .withMethod("GET")
+                        .withPath(path)
+        ).respond(
+                HttpResponse
+                        .response()
+                        .withStatusCode(200)
+                        .withBody(responseBody)
+        );
+    }
+}