Merge pull request #480 from coursier/topic/no-directory-listing-fallback

Add fallback if directory listings are not available

appveyor.yml

@@ -26,7 +26,9 @@ build_script:
   - sbt ++2.10.6 clean compile
   - sbt ++2.10.6 coreJVM/publishLocal cache/publishLocal # to make the scripted tests happy
 test_script:
-  - ps: Start-Job -filepath .\scripts\start-it-server.ps1 -ArgumentList $pwd
+  - ps: Start-Job -filepath .\scripts\start-it-auth-server.ps1 -ArgumentList $pwd
+  - ps: Start-Sleep -s 15 # wait for the first server to have downloaded its dependencies
+  - ps: Start-Job -filepath .\scripts\start-it-no-listing-server.ps1 -ArgumentList $pwd
   - sbt ++2.12.1 testsJVM/test testsJVM/it:test # Would node be around for testsJS/test?
   - sbt ++2.11.8 testsJVM/test testsJVM/it:test
   - sbt ++2.10.6 testsJVM/test testsJVM/it:test sbt-coursier/scripted sbt-coursier/publishLocal sbt-shading/scripted

cache/src/main/scala/coursier/Cache.scala

@@ -319,6 +319,43 @@ object Cache {
   def url(s: String): URL =
     new URL(null, s, handlerFor(s).orNull)
 
+  def urlConnection(url0: String, authentication: Option[Authentication]) = {
+    var conn: URLConnection = null
+
+    try {
+      conn = url(url0).openConnection() // FIXME Should this be closed?
+      // Dummy user-agent instead of the default "Java/...",
+      // so that we are not returned incomplete/erroneous metadata
+      // (Maven 2 compatibility? - happens for snapshot versioning metadata)
+      conn.setRequestProperty("User-Agent", "")
+
+      for (auth <- authentication)
+        conn match {
+          case authenticated: AuthenticatedURLConnection =>
+            authenticated.authenticate(auth)
+          case conn0: HttpURLConnection =>
+            conn0.setRequestProperty(
+              "Authorization",
+              "Basic " + basicAuthenticationEncode(auth.user, auth.password)
+            )
+          case _ =>
+          // FIXME Authentication is ignored
+        }
+
+      conn
+    } catch {
+      case NonFatal(e) =>
+        if (conn != null)
+          conn match {
+            case conn0: HttpURLConnection =>
+              conn0.getInputStream.close()
+              conn0.disconnect()
+            case _ =>
+          }
+        throw e
+    }
+  }
+
   private def download(
     artifact: Artifact,
     cache: File,
@@ -340,45 +377,6 @@ object Cache {
 
     def referenceFileExists: Boolean = referenceFileOpt.exists(_.exists())
 
-    def urlConn(url0: String) = {
-      var conn: URLConnection = null
-
-      try {
-        conn = url(url0).openConnection() // FIXME Should this be closed?
-        // Dummy user-agent instead of the default "Java/...",
-        // so that we are not returned incomplete/erroneous metadata
-        // (Maven 2 compatibility? - happens for snapshot versioning metadata,
-        // this is SO FSCKING CRAZY)
-        conn.setRequestProperty("User-Agent", "")
-
-        for (auth <- artifact.authentication)
-          conn match {
-            case authenticated: AuthenticatedURLConnection =>
-              authenticated.authenticate(auth)
-            case conn0: HttpURLConnection =>
-              conn0.setRequestProperty(
-                "Authorization",
-                "Basic " + basicAuthenticationEncode(auth.user, auth.password)
-              )
-            case _ =>
-            // FIXME Authentication is ignored
-          }
-
-        conn
-      } catch {
-        case NonFatal(e) =>
-          if (conn != null)
-            conn match {
-              case conn0: HttpURLConnection =>
-                conn0.getInputStream.close()
-                conn0.disconnect()
-              case _ =>
-            }
-          throw e
-      }
-    }
-
-
     def fileLastModified(file: File): EitherT[Task, FileError, Option[Long]] =
       EitherT {
         Task {
@@ -402,7 +400,7 @@ object Cache {
           var conn: URLConnection = null
 
           try {
-            conn = urlConn(url)
+            conn = urlConnection(url, artifact.authentication)
 
             conn match {
               case c: HttpURLConnection =>
@@ -558,7 +556,7 @@ object Cache {
               var conn: URLConnection = null
 
               try {
-                conn = urlConn(url)
+                conn = urlConnection(url, artifact.authentication)
 
                 val partialDownload = conn match {
                   case conn0: HttpURLConnection if alreadyDownloaded > 0L =>
@@ -571,7 +569,7 @@ object Cache {
                         // unrecognized Content-Range header -> start a new connection with no resume
                         conn0.getInputStream.close()
                         conn0.disconnect()
-                        conn = urlConn(url)
+                        conn = urlConnection(url, artifact.authentication)
                         false
                       }
                     }

cache/src/main/scala/coursier/Platform.scala

@@ -42,19 +42,12 @@ object Platform {
 
   val artifact: Fetch.Content[Task] = { artifact =>
     EitherT {
-      val url = Cache.url(artifact.url)
+      val conn = Cache.urlConnection(artifact.url, artifact.authentication)
-
-      val conn = url.openConnection()
-      // Dummy user-agent instead of the default "Java/...",
-      // so that we are not returned incomplete/erroneous metadata
-      // (Maven 2 compatibility? - happens for snapshot versioning metadata,
-      // this is SO FUCKING CRAZY)
-      conn.setRequestProperty("User-Agent", "")
       readFully(conn.getInputStream())
     }
   }
 
-  implicit def fetch(
+  def fetch(
     repositories: Seq[core.Repository]
   ): Fetch.Metadata[Task] =
     Fetch.from(repositories, Platform.artifact)

core/shared/src/main/scala/coursier/maven/MavenRepository.scala

@@ -286,35 +286,46 @@ final case class MavenRepository(
 
     for {
       str <- fetch(projectArtifact(module, version, versioningValue))
-      rawListFilesPage <- fetch(artifactFor(listFilesUrl))
+      rawListFilesPageOpt <- EitherT(F.map(fetch(artifactFor(listFilesUrl)).run) {
+        e => \/-(e.toOption): String \/ Option[String]
+      })
       proj0 <- EitherT(F.point[String \/ Project](parseRawPom(str)))
     } yield {
 
-      val files = WebPage.listFiles(listFilesUrl, rawListFilesPage)
+      val foundPublications =
+        rawListFilesPageOpt match {
+          case Some(rawListFilesPage) =>
 
-      val prefix = s"${module.name}-${versioningValue.getOrElse(version)}"
+            val files = WebPage.listFiles(listFilesUrl, rawListFilesPage)
 
-      val packagingTpeMap = proj0.packagingOpt
+            val prefix = s"${module.name}-${versioningValue.getOrElse(version)}"
-        .map { packaging =>
-          (MavenSource.typeDefaultClassifier(packaging), MavenSource.typeExtension(packaging)) -> packaging
-        }
-        .toMap
 
-      val foundPublications = files
+            val packagingTpeMap = proj0.packagingOpt
-        .flatMap(isArtifact(_, prefix))
+              .map { packaging =>
-        .map {
+                (MavenSource.typeDefaultClassifier(packaging), MavenSource.typeExtension(packaging)) -> packaging
-          case (classifier, ext) =>
+              }
-            val tpe = packagingTpeMap.getOrElse(
+              .toMap
-              (classifier, ext),
+
-              MavenSource.classifierExtensionDefaultTypeOpt(classifier, ext).getOrElse(ext)
+            files
-            )
+              .flatMap(isArtifact(_, prefix))
-            val config = MavenSource.typeDefaultConfig(tpe).getOrElse("compile")
+              .map {
-            config -> Publication(
+                case (classifier, ext) =>
-              module.name,
+                  val tpe = packagingTpeMap.getOrElse(
-              tpe,
+                    (classifier, ext),
-              ext,
+                    MavenSource.classifierExtensionDefaultTypeOpt(classifier, ext).getOrElse(ext)
-              classifier
+                  )
-            )
+                  val config = MavenSource.typeDefaultConfig(tpe).getOrElse("compile")
+                  config -> Publication(
+                    module.name,
+                    tpe,
+                    ext,
+                    classifier
+                  )
+              }
+
+          case None =>
+            // Publications can't be listed - MavenSource then handles that
+            Nil
         }
 
       val proj = Pom.addOptionalDependenciesInConfig(

core/shared/src/main/scala/coursier/maven/MavenSource.scala

@@ -65,14 +65,47 @@ final case class MavenSource(
       case Some(classifiers) =>
         val classifiersSet = classifiers.toSet
 
-        project.publications.collect {
+        if (project.publications.isEmpty)
-          case (_, p) if classifiersSet(p.classifier) =>
+          // For repositories not providing directory listings, give a try to some publications anyway
-            p
+          classifiers.map { classifier =>
-        }
+            Publication(
+              dependency.module.name,
+              "jar",
+              "jar",
+              classifier
+            )
+          }
+        else
+          project.publications.collect {
+            case (_, p) if classifiersSet(p.classifier) =>
+              p
+          }
 
       case None =>
 
-        if (dependency.attributes.classifier.nonEmpty)
+        if (project.publications.isEmpty) {
+
+          // For repositories not providing directory listings, give a try to some publications anyway
+
+          val type0 = if (dependency.attributes.`type`.isEmpty) "jar" else dependency.attributes.`type`
+
+          val extension = MavenSource.typeExtension(type0)
+
+          val classifier =
+            if (dependency.attributes.classifier.isEmpty)
+              MavenSource.typeDefaultClassifier(type0)
+            else
+              dependency.attributes.classifier
+
+          Seq(
+            Publication(
+              dependency.module.name,
+              type0,
+              extension,
+              classifier
+            )
+          )
+        } else if (dependency.attributes.classifier.nonEmpty)
           // FIXME We're ignoring dependency.attributes.`type` in this case
           project.publications.collect {
             case (_, p) if p.classifier == dependency.attributes.classifier =>

fetch-js/src/main/scala/coursier/Platform.scala

@@ -88,7 +88,7 @@ object Platform {
     )
   }
 
-  implicit def fetch(
+  def fetch(
     repositories: Seq[core.Repository]
   ): Fetch.Metadata[Task] =
     Fetch.from(repositories, Platform.artifact)

scripts/launch-test-repo.sh

@@ -0,0 +1,15 @@
+#!/usr/bin/env bash
+set -e
+
+VERSION="${VERSION:-1.0.0-SNAPSHOT}"
+
+cd "$(dirname "$0")/.."
+
+./coursier launch \
+  "io.get-coursier:http-server-java7_2.11:$VERSION" \
+  -r https://dl.bintray.com/scalaz/releases \
+  -- \
+    -d tests/jvm/src/test/resources/test-repo/http/abc.com \
+    -u user -P pass -r realm \
+    -v \
+    "$@" &

scripts/start-it-auth-server.ps1

@@ -0,0 +1,3 @@
+# see https://stackoverflow.com/questions/2224350/powershell-start-job-working-directory/2246542#2246542
+Set-Location $args[0]
+& java -jar -noverify coursier launch -r https://dl.bintray.com/scalaz/releases io.get-coursier:http-server-java7_2.11:1.0.0-SNAPSHOT -- -d tests/jvm/src/test/resources/test-repo/http/abc.com -u user -P pass -r realm --port 8080 --list-pages -v

scripts/start-it-no-listing-server.ps1

@@ -1,3 +1,3 @@
 # see https://stackoverflow.com/questions/2224350/powershell-start-job-working-directory/2246542#2246542
 Set-Location $args[0]
-& java -jar -noverify coursier launch -r https://dl.bintray.com/scalaz/releases io.get-coursier:http-server-java7_2.11:1.0.0-SNAPSHOT -- -d tests/jvm/src/test/resources/test-repo/http/abc.com -u user -P pass -r realm --list-pages -v
+& java -jar -noverify coursier launch -r https://dl.bintray.com/scalaz/releases io.get-coursier:http-server-java7_2.11:1.0.0-SNAPSHOT -- -d tests/jvm/src/test/resources/test-repo/http/abc.com -u user -P pass -r realm --port 8081 -v

scripts/travis.sh

@@ -20,19 +20,19 @@ downloadInstallSbtExtras() {
   chmod +x bin/sbt
 }
 
+launchTestRepo() {
+  ./scripts/launch-test-repo.sh "$@"
+}
+
 integrationTestsRequirements() {
   # Required for ~/.ivy2/local repo tests
   sbt ++2.11.8 coreJVM/publishLocal http-server/publishLocal
 
   # Required for HTTP authentication tests
-  coursier launch \
+  launchTestRepo --port 8080 --list-pages
-    io.get-coursier:http-server-java7_2.11:1.0.0-SNAPSHOT \
+
-    -r http://dl.bintray.com/scalaz/releases \
+  # Required for missing directory listing tests (no --list-pages)
-    -- \
+  launchTestRepo --port 8081
-      -d tests/jvm/src/test/resources/test-repo/http/abc.com \
-      -u user -P pass -r realm \
-      --list-pages \
-      -v &
 }
 
 setupCustomJarjar() {

tests/jvm/src/it/scala/coursier/test/DirectoryListingTests.scala

@@ -0,0 +1,71 @@
+package coursier.test
+
+import coursier._
+import coursier.core.Authentication
+import utest._
+
+object DirectoryListingTests extends TestSuite {
+
+  val user = "user"
+  val password = "pass"
+
+  val withListingRepo = MavenRepository(
+    "http://localhost:8080",
+    authentication = Some(Authentication(user, password))
+  )
+
+  val withoutListingRepo = MavenRepository(
+    "http://localhost:8081",
+    authentication = Some(Authentication(user, password))
+  )
+
+  val module = Module("com.abc", "test")
+  val version = "0.1"
+
+  val tests = TestSuite {
+    'withListing - {
+      'jar - CentralTests.withArtifacts(
+        module,
+        version,
+        "jar",
+        extraRepo = Some(withListingRepo)
+      ) {
+        artifacts =>
+          assert(artifacts.length == 1)
+      }
+
+      'jarFoo - CentralTests.withArtifacts(
+        module,
+        version,
+        "jar-foo",
+        extraRepo = Some(withListingRepo)
+      ) {
+        artifacts =>
+          assert(artifacts.length == 1)
+      }
+    }
+
+    'withoutListing - {
+      'jar - CentralTests.withArtifacts(
+        module,
+        version,
+        "jar",
+        extraRepo = Some(withoutListingRepo)
+      ) {
+        artifacts =>
+          assert(artifacts.length == 1)
+      }
+
+      'jarFoo - CentralTests.withArtifacts(
+        module,
+        version,
+        "jar-foo",
+        extraRepo = Some(withoutListingRepo)
+      ) {
+        artifacts =>
+          assert(artifacts.length == 0)
+      }
+    }
+  }
+
+}

tests/jvm/src/it/scala/coursier/test/HttpAuthenticationTests.scala

@@ -23,7 +23,7 @@ object HttpAuthenticationTests extends TestSuite {
             s"basic authentication with user '$user' and password '$password', serving the right " +
             "files.\n" + Console.RESET +
             "Run one from the coursier sources with\n" +
-            "  ./coursier launch -r http://dl.bintray.com/scalaz/releases " +
+            "  ./coursier launch -r https://dl.bintray.com/scalaz/releases " +
             "io.get-coursier:simple-web-server_2.11:1.0.0-M12 -- " +
             "-d tests/jvm/src/test/resources/test-repo/http/abc.com -u user -P pass -r realm -v"
         )

tests/jvm/src/test/resources/test-repo/http/abc.com/com/abc/test/0.1/test-0.1.jar

@@ -0,0 +1 @@
+nothing here

tests/jvm/src/test/resources/test-repo/http/abc.com/com/abc/test/0.1/test-0.1.jar-foo

@@ -0,0 +1 @@
+nothing here

tests/shared/src/test/scala/coursier/test/CentralTests.scala

@@ -23,13 +23,15 @@ object CentralTests extends TestSuite {
   ) = {
     val repositories0 = extraRepo.toSeq ++ repositories
 
+    val fetch = Platform.fetch(repositories0)
+
     Resolution(
       deps,
       filter = filter,
       userActivations = profiles.map(_.iterator.map(_ -> true).toMap)
     )
       .process
-      .run(repositories0)
+      .run(fetch)
       .runF
   }