Repository: smithy-lang/smithy (java)

Issue:
Resolution for snapshot artifacts is broken when defining multiple repositories with the same hostname
Snapshot dependency resolution is broken if multiple repositories with the same hostname are defined in `smithy-build.json`.

The underlying aether resolution relies on the repository definition to have a discriminator id for de-duplicating repositories.
The current smithy build definition doesn't have that, thus resolution fails.

### Steps to reproduce
Consider the following smithy definition:
```
{
    "version": "1.0",
    "maven": {
        "repositories": [
            {
                "url": "http://localhost:1234/artifactory-1"
            },
            {
                "url": "http://localhost:1234/artifactory-2"
            }
        ],
        "dependencies": [
            // Normally, this could refer to SMITHY_VERSION, but that doesn't work for pre-release builds.
            "com.example:artifact:1.0.0-SNAPSHOT"
        ]
    }
}
```

and the following pom file definition for `com.example:artifact:1.0.0-SNAPSHOT`:
```
<?xml version='1.0' encoding='UTF-8'?>
<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\">
    <modelVersion>4.0.0</modelVersion>
    <groupId>com.example</groupId>
    <artifactId>artifact</artifactId>
    <packaging>jar</packaging>
    <description>artifact</description>
    <version>1.0.0-SNAPSHOT</version>
    <name>artifact</name>
    <organization>
        <name>com.example</name>
    </organization>
    <properties>
        <info.versionScheme>early-semver</info.versionScheme>
    </properties>
    <dependencies>
        <dependency>
            <groupId>com.example</groupId>
            <artifactId>dependency</artifactId>
            <version>1.0.0</version>
        </dependency>
    </dependencies>
</project>
```

After configuring 
- `http://localhost:1234/artifactory-1` to host the release version of `com.example:dependency:1.0.0` 
- `http://localhost:1234/artifactory-2` to host the snapshot version of `com.example:artifact:1.0.0-SNAPSHOT`

We would expect the dependency resolution to succeed, but instead it fails.


Fixed by #1884 and 6e680054bbcdd9a54b9fc0da6078f1dd8ec79e56 adds a test to exemplify the behavior.

Pull request:
Fix snapshot dependency resolution
*Closes #1883 *

*Description of changes:*
Add an optional id for the maven repository definition.

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/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/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/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)
+        );
+    }
+}