Improve handling of differing artifacts

src/main/resources/messages.properties

@@ -51,5 +51,5 @@ lock.status.full.dependencies.changed.multiple=\ \ {0} dependencies changed:\n{1
 
 # lockfile full status - artifacts ============================================
 lock.status.full.artifacts.changed.none=
-lock.status.full.artifacts.changed.singular=\ \ 1 dependency artifacts changed
-lock.status.full.artifacts.changed.multiple=\ \ {0} dependency artifacts changed
+lock.status.full.artifacts.changed.singular=\ \ 1 dependency artifacts changed:\n{1}
+lock.status.full.artifacts.changed.multiple=\ \ {0} dependency artifacts changed:\n{1}

src/main/scala/software/purpledragon/sbt/lock/DependencyUtils.scala

@@ -16,12 +16,11 @@
 
 package software.purpledragon.sbt.lock
 
-import java.time.Instant
-
 import sbt._
 import software.purpledragon.sbt.lock.model.{DependencyLockFile, DependencyRef, ResolvedArtifact, ResolvedDependency}
 
-import scala.collection.{immutable, mutable, SortedSet}
+import java.time.Instant
+import scala.collection.{SortedSet, immutable, mutable}
 
 object DependencyUtils {
   def resolve(updateReport: UpdateReport, configs: Seq[ConfigRef]): DependencyLockFile = {
@@ -64,16 +63,7 @@ object DependencyUtils {
       module: ModuleReport,
       checksumCache: mutable.Map[File, String]): ResolvedDependency = {
 
-    val artifacts: immutable.Seq[ResolvedArtifact] = module.artifacts map { case (artifact, file) =>
-      val hash = checksumCache.getOrElseUpdate(file, hashFile(file))
-
-      val qualifier = artifact.`type` match {
-        case "jar" | "bundle" => ""
-        case q => s"-$q"
-      }
-
-      ResolvedArtifact(s"${artifact.name}$qualifier.${artifact.extension}", hash)
-    }
+    val artifacts = module.artifacts.map(ResolvedArtifact.apply(_, checksumCache))
 
     ResolvedDependency(
       module.module.organization,
@@ -82,6 +72,4 @@ object DependencyUtils {
       artifacts.to[SortedSet],
       SortedSet.empty)
   }
-
-  private def hashFile(file: File): String = s"sha1:${Hash.toHex(Hash(file))}"
 }

src/main/scala/software/purpledragon/sbt/lock/model/ChangedDependency.scala

@@ -17,17 +17,70 @@
 package software.purpledragon.sbt.lock.model
 
 import scala.collection.SortedSet
+import scala.math.Ordered.orderingToOrdered
 
 final case class ChangedDependency(
     org: String,
     name: String,
     oldVersion: String,
     newVersion: String,
-    oldArtifacts: SortedSet[ResolvedArtifact],
-    newArtifacts: SortedSet[ResolvedArtifact],
     oldConfigurations: SortedSet[String],
-    newConfigurations: SortedSet[String]) {
+    newConfigurations: SortedSet[String],
+    addedArtifacts: SortedSet[ResolvedArtifact],
+    removedArtifacts: SortedSet[ResolvedArtifact],
+    changedArtifacts: SortedSet[ChangedArtifact]) {
 
   def versionChanged: Boolean = oldVersion != newVersion
   def configurationsChanged: Boolean = oldConfigurations != newConfigurations
+  def artifactsChanged: Boolean = addedArtifacts.nonEmpty || removedArtifacts.nonEmpty || changedArtifacts.nonEmpty
+}
+
+object ChangedDependency {
+  def apply(
+      org: String,
+      name: String,
+      oldVersion: String,
+      newVersion: String,
+      oldConfigurations: SortedSet[String],
+      newConfigurations: SortedSet[String],
+      oldArtifacts: SortedSet[ResolvedArtifact],
+      newArtifacts: SortedSet[ResolvedArtifact]): ChangedDependency = {
+
+    val oldArtifactsByName = oldArtifacts.map(a => a.name -> a).toMap
+    val newArtifactsByName = newArtifacts.map(a => a.name -> a).toMap
+
+    val addedArtifacts = (newArtifactsByName.keySet -- oldArtifactsByName.keySet).map(newArtifactsByName.apply)
+    val removedArtifacts = (oldArtifactsByName.keySet -- newArtifactsByName.keySet).map(oldArtifactsByName.apply)
+
+    val changedArtifacts =
+      oldArtifactsByName.keySet.intersect(newArtifactsByName.keySet).foldLeft(Seq.empty[ChangedArtifact]) {
+        (changes, name) =>
+          val oldHash = oldArtifactsByName(name).hash
+          val newHash = newArtifactsByName(name).hash
+
+          if (oldHash != newHash) {
+            changes :+ ChangedArtifact(name, oldHash, newHash)
+          } else {
+            changes
+          }
+      }
+
+    ChangedDependency(
+      org,
+      name,
+      oldVersion,
+      newVersion,
+      oldConfigurations,
+      newConfigurations,
+      addedArtifacts.to[SortedSet],
+      removedArtifacts.to[SortedSet],
+      changedArtifacts.to[SortedSet]
+    )
+  }
+}
+
+final case class ChangedArtifact(name: String, oldHash: String, newHash: String) extends Ordered[ChangedArtifact] {
+  override def compare(that: ChangedArtifact): Int = {
+    (name, oldHash, newHash) compare (that.name, that.oldHash, that.newHash)
+  }
 }

src/main/scala/software/purpledragon/sbt/lock/model/DependencyLockFile.scala

@@ -80,10 +80,10 @@ final case class DependencyLockFile(
             depref.name,
             ourDep.version,
             otherDep.version,
-            ourDep.artifacts,
-            otherDep.artifacts,
             ourDep.configurations,
-            otherDep.configurations)
+            otherDep.configurations,
+            ourDep.artifacts,
+            otherDep.artifacts)
         } else {
           changes
         }

src/main/scala/software/purpledragon/sbt/lock/model/LockFileStatus.scala

@@ -166,8 +166,45 @@ final case class LockFileDiffers(
         dumpChanges(otherChanged))
     }
 
+    def dumpArtifactChanges(changes: Seq[ChangedDependency]): String = {
+      val changesBuilder = new mutable.StringBuilder()
+
+      changes foreach { change =>
+        changesBuilder ++= "    "
+        changesBuilder ++= change.org
+        changesBuilder ++= ":"
+        changesBuilder ++= change.name
+        changesBuilder ++= "  ("
+        change.oldConfigurations.addString(changesBuilder, ",")
+        changesBuilder ++= ")  "
+        changesBuilder ++= change.oldVersion
+        changesBuilder ++= ":\n"
+
+        val table = new SortedTableFormatter(None, prefix = "      ", stripTrailingNewline = false)
+
+        change.addedArtifacts foreach { added =>
+          table.addRow("(added)", added.name, added.hash)
+        }
+
+        change.removedArtifacts foreach { removed =>
+          table.addRow("(removed)", removed.name, removed.hash)
+        }
+
+        change.changedArtifacts foreach { changed =>
+          table.addRow("(changed)", changed.name, changed.oldHash, s"-> ${changed.newHash}")
+        }
+
+        changesBuilder ++= table.toString()
+      }
+
+      changesBuilder.toString()
+    }
+
     if (artifactChanged.nonEmpty) {
-      errors += MessageUtil.formatPlural("lock.status.full.artifacts.changed", artifactChanged.size)
+      errors += MessageUtil.formatPlural(
+        "lock.status.full.artifacts.changed",
+        artifactChanged.size,
+        dumpArtifactChanges(artifactChanged))
     }
 
     MessageUtil.formatMessage("lock.status.failed.long", errors.mkString("\n"))

src/main/scala/software/purpledragon/sbt/lock/model/ResolvedArtifact.scala

@@ -16,10 +16,33 @@
 
 package software.purpledragon.sbt.lock.model
 
+import java.io.File
+
+import sbt.Hash
+import sbt.librarymanagement.Artifact
+
+import scala.collection.mutable
 import scala.math.Ordered.orderingToOrdered
 
 final case class ResolvedArtifact(name: String, hash: String) extends Ordered[ResolvedArtifact] {
   override def compare(that: ResolvedArtifact): Int = {
     (name, hash) compare (that.name, that.hash)
   }
 }
+
+object ResolvedArtifact {
+  def apply(art: (Artifact, File), checksumCache: mutable.Map[File, String]): ResolvedArtifact = {
+    val (artifact, file) = art
+    val hash = checksumCache.getOrElseUpdate(file, hashFile(file))
+
+    val classifier = artifact.classifier.map(c => s"-$c").getOrElse("")
+    val qualifier = artifact.`type` match {
+      case "jar" | "bundle" => ""
+      case q => s"-$q"
+    }
+
+    ResolvedArtifact(s"${artifact.name}$classifier$qualifier.${artifact.extension}", hash)
+  }
+
+  private def hashFile(file: File): String = s"sha1:${Hash.toHex(Hash(file))}"
+}

src/test/scala/software/purpledragon/sbt/lock/model/DependencyLockFileSpec.scala

@@ -16,28 +16,25 @@
 
 package software.purpledragon.sbt.lock.model
 
-import java.time.Instant
-
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
 
+import java.time.Instant
 import scala.collection.SortedSet
 
 class DependencyLockFileSpec extends AnyFlatSpec with Matchers {
+  private val dependency1Artifact = ResolvedArtifact("package-1.jar", "hash-1")
+  private val dependency1 =
+    ResolvedDependency("com.example", "package-1", "1.0.0", SortedSet(dependency1Artifact), SortedSet("test-1"))
+  private val dependency2 =
+    ResolvedDependency("com.example", "package-2", "1.2.0", SortedSet.empty, SortedSet("test-2"))
+
   private val EmptyLockFile = DependencyLockFile(1, Instant.now(), Nil, Nil)
   private val TestLockFile = DependencyLockFile(
     1,
     Instant.now(),
     Seq("test-1", "test-2"),
-    Seq(
-      ResolvedDependency(
-        "com.example",
-        "package-1",
-        "1.0.0",
-        SortedSet(ResolvedArtifact("package-1.jar", "hash-1")),
-        SortedSet("test-1")),
-      ResolvedDependency("com.example", "package-2", "1.2.0", SortedSet.empty, SortedSet("test-2"))
-    )
+    Seq(dependency1, dependency2)
   )
 
   "findChanges" should "return LockFileMatches for identical lockfiles" in {
@@ -78,12 +75,13 @@ class DependencyLockFileSpec extends AnyFlatSpec with Matchers {
   }
 
   it should "return LockFileDiffers if dependency added" in {
-    val newDependency = ResolvedDependency(
-      "com.example",
-      "package-3",
-      "3.0",
-      SortedSet(ResolvedArtifact("package-3.jar", "hash-3")),
-      SortedSet("test-1"))
+    val newDependency =
+      ResolvedDependency(
+        "com.example",
+        "package-3",
+        "3.0",
+        SortedSet(ResolvedArtifact("package-3.jar", "hash-3")),
+        SortedSet("test-1"))
 
     val left = TestLockFile
     val right = left.copy(dependencies = left.dependencies :+ newDependency)
@@ -101,12 +99,8 @@ class DependencyLockFileSpec extends AnyFlatSpec with Matchers {
   it should "return LockFileDiffers if dependency changed" in {
     val left = TestLockFile
     val right = left.copy(
-      dependencies = left.dependencies.tail :+ ResolvedDependency(
-        "com.example",
-        "package-1",
-        "2.0.0",
-        SortedSet(ResolvedArtifact("package-1.jar", "hash-1a")),
-        SortedSet("test-1", "test-2"))
+      dependencies = left.dependencies.head.copy(version = "2.0.0", configurations = SortedSet("test-1", "test-2")) +:
+        left.dependencies.tail
     )
 
     left.findChanges(right) shouldBe LockFileDiffers(
@@ -120,10 +114,103 @@ class DependencyLockFileSpec extends AnyFlatSpec with Matchers {
           "package-1",
           "1.0.0",
           "2.0.0",
-          SortedSet(ResolvedArtifact("package-1.jar", "hash-1")),
-          SortedSet(ResolvedArtifact("package-1.jar", "hash-1a")),
           SortedSet("test-1"),
-          SortedSet("test-1", "test-2")
+          SortedSet("test-1", "test-2"),
+          SortedSet.empty,
+          SortedSet.empty,
+          SortedSet.empty
+        ))
+    )
+  }
+
+  it should "return LockFileDiffers if artifact changed" in {
+    val left = TestLockFile
+    val right = left.copy(
+      dependencies = left.dependencies map { dep =>
+        dep.copy(artifacts = dep.artifacts map { art =>
+          art.copy(hash = art.hash + "a")
+        })
+      }
+    )
+
+    left.findChanges(right) shouldBe LockFileDiffers(
+      Nil,
+      Nil,
+      Nil,
+      Nil,
+      Seq(
+        ChangedDependency(
+          "com.example",
+          "package-1",
+          "1.0.0",
+          "1.0.0",
+          SortedSet("test-1"),
+          SortedSet("test-1"),
+          SortedSet.empty,
+          SortedSet.empty,
+          SortedSet(ChangedArtifact("package-1.jar", "hash-1", "hash-1a"))
+        ))
+    )
+  }
+
+  it should "return LockFileDiffers if artifact added" in {
+    val left = TestLockFile
+
+    val right = left.copy(
+      dependencies = Seq(
+        dependency1.copy(
+          artifacts = SortedSet(
+            dependency1Artifact,
+            ResolvedArtifact("package-1a.jar", "hash-1a")
+          )),
+        dependency2
+      ))
+
+    left.findChanges(right) shouldBe LockFileDiffers(
+      Nil,
+      Nil,
+      Nil,
+      Nil,
+      Seq(
+        ChangedDependency(
+          "com.example",
+          "package-1",
+          "1.0.0",
+          "1.0.0",
+          SortedSet("test-1"),
+          SortedSet("test-1"),
+          SortedSet(ResolvedArtifact("package-1a.jar", "hash-1a")),
+          SortedSet.empty,
+          SortedSet.empty
+        ))
+    )
+  }
+
+  it should "return LockFileDiffers if artifact removed" in {
+    val left = TestLockFile
+
+    val right = left.copy(
+      dependencies = Seq(
+        dependency1.copy(artifacts = SortedSet.empty),
+        dependency2
+      ))
+
+    left.findChanges(right) shouldBe LockFileDiffers(
+      Nil,
+      Nil,
+      Nil,
+      Nil,
+      Seq(
+        ChangedDependency(
+          "com.example",
+          "package-1",
+          "1.0.0",
+          "1.0.0",
+          SortedSet("test-1"),
+          SortedSet("test-1"),
+          SortedSet.empty,
+          SortedSet(ResolvedArtifact("package-1.jar", "hash-1")),
+          SortedSet.empty
         ))
     )
   }