Skip to content

Commit

Permalink
Use SortedMap in GradleResolver, fix property tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Oscar Boykin committed Feb 2, 2024
1 parent f5608bf commit 3043d5e
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 53 deletions.
13 changes: 7 additions & 6 deletions src/scala/com/github/johnynek/bazel_deps/GradleLockFile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.github.johnynek.bazel_deps
import io.circe.Decoder.Result
import io.circe.{Decoder, Error, HCursor, Json, KeyDecoder, Parser}
import io.circe.generic.auto
import scala.collection.immutable.SortedMap

import scala.util.{Failure, Success, Try}

Expand Down Expand Up @@ -69,10 +70,10 @@ object GradleLockFile {
}

case class GradleLockFile(
annotationProcessor: Option[Map[String, GradleLockDependency]],
compileClasspath: Option[Map[String, GradleLockDependency]],
resolutionRules: Option[Map[String, GradleLockDependency]],
runtimeClasspath: Option[Map[String, GradleLockDependency]],
testCompileClasspath: Option[Map[String, GradleLockDependency]],
testRuntimeClasspath: Option[Map[String, GradleLockDependency]]
annotationProcessor: Option[SortedMap[String, GradleLockDependency]],
compileClasspath: Option[SortedMap[String, GradleLockDependency]],
resolutionRules: Option[SortedMap[String, GradleLockDependency]],
runtimeClasspath: Option[SortedMap[String, GradleLockDependency]],
testCompileClasspath: Option[SortedMap[String, GradleLockDependency]],
testRuntimeClasspath: Option[SortedMap[String, GradleLockDependency]]
)
7 changes: 3 additions & 4 deletions src/scala/com/github/johnynek/bazel_deps/GradleResolver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class GradleResolver(
// we just want one, so we merge them all
private def collapseDepTypes(
lockFile: GradleLockFile
): Try[Map[String, GradleLockDependency]] =
): Try[SortedMap[String, GradleLockDependency]] =
TryMerge.tryMergeAll(
None,
NonEmptyList.of(
Expand All @@ -54,7 +54,7 @@ class GradleResolver(
lockFile.testRuntimeClasspath
)
)
.map(_.getOrElse(Map.empty))
.map(_.getOrElse(SortedMap.empty[String, GradleLockDependency]))

private val unit = Validated.valid(())

Expand Down Expand Up @@ -238,7 +238,7 @@ private def cleanUpMap(
}

def buildGraphFromDepMap(
depMap: Map[String, GradleLockDependency]
depMap: SortedMap[String, GradleLockDependency]
): Try[Graph[MavenCoordinate, Unit]] =
assertConnectedDependencyMap(depMap)
.map(_ => cleanUpMap(depMap))
Expand Down Expand Up @@ -274,7 +274,6 @@ private def cleanUpMap(

depMap
.toList
.sortBy(_._1)
.traverse[V, (MavenCoordinate, List[MavenCoordinate])] { case (n, deps) =>
toCoord(n)
.product(
Expand Down
43 changes: 20 additions & 23 deletions src/scala/com/github/johnynek/bazel_deps/TryMerge.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.github.johnynek.bazel_deps
import cats.{Foldable, Reducible}
import cats.data.NonEmptyList
import scala.util.{Failure, Success, Try}
import scala.collection.immutable.SortedMap

import cats.syntax.all._

Expand Down Expand Up @@ -40,33 +41,29 @@ object TryMerge {
}
}

implicit def tryStringMapMerge[T: TryMerge]: TryMerge[Map[String, T]] =
new TryMerge[Map[String, T]] {
private val rev = reverse
implicit def tryStringMapMerge[T: TryMerge]: TryMerge[SortedMap[String, T]] =
new TryMerge[SortedMap[String, T]] {
def tryMerge(
debugName: Option[String],
left: Map[String, T],
right: Map[String, T]
): Try[Map[String, T]] = {
if (right.size > left.size) rev.tryMerge(debugName, right, left)
else {
// right <= left in size
val overlaps = right.exists { case (k, _) => left.contains(k) }
if (!overlaps) Success(left ++ right)
else {
right.toList.sortBy(_._1)
.foldM(left) { case (acc, (k, rv)) =>
acc.get(k) match {
case Some(lv) =>
TryMerge.tryMerge(Some(debugName.fold(k) {p => s"$p:$k"}), lv, rv)
.map(acc.updated(k, _))
case None =>
Success(acc.updated(k, rv))
}
}
left: SortedMap[String, T],
right: SortedMap[String, T]
): Try[SortedMap[String, T]] = {
val tmt = implicitly[TryMerge[T]]
val (big, small, tm) =
if (right.size > left.size) (right, left, tmt.reverse)
else (left, right, tmt)

small.toStream
.foldM(big) { case (acc, (k, rv)) =>
acc.get(k) match {
case Some(lv) =>
val nextDebug = Some(debugName.fold(k) {p => s"$p:$k"})
tm.tryMerge(nextDebug, lv, rv).map(acc.updated(k, _))
case None =>
Success(acc.updated(k, rv))
}
}
}
}
}
}

12 changes: 12 additions & 0 deletions test/scala/com/github/johnynek/bazel_deps/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,16 @@ scala_test(
]
)

scala_test(
name = "trymergetest",
srcs = ["TryMergeTest.scala"],
deps = [
"//src/scala/com/github/johnynek/bazel_deps:trymerge",
"//3rdparty/jvm/org/scalacheck",
"//3rdparty/jvm/org/scalatest:scalatest_propspec",
"//3rdparty/jvm/org/scalatestplus:scalacheck_1_17",
"//3rdparty/jvm/org/typelevel:cats_core",
]
)

test_suite(name = "all_tests")
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import io.circe.jawn.JawnParser
import java.nio.file.Paths
import org.scalatest.funsuite.AnyFunSuite
import scala.util.{Success, Failure}
import scala.collection.immutable.SortedMap

import Decoders._

Expand All @@ -18,8 +19,8 @@ class GradleResolverTest extends AnyFunSuite {
case Left(error) => sys.error(s"Failed to decode $str: ${error}")
}

def deps(str: String): Map[String, GradleLockDependency] =
(new JawnParser).decode[Map[String, GradleLockDependency]](str) match {
def deps(str: String): SortedMap[String, GradleLockDependency] =
(new JawnParser).decode[SortedMap[String, GradleLockDependency]](str) match {
case Right(t) => t
case Left(error) => sys.error(s"Failed to decode $str: ${error}")
}
Expand Down
41 changes: 23 additions & 18 deletions test/scala/com/github/johnynek/bazel_deps/GraphPropTest.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package com.github.johnynek.bazel_deps

import org.scalatest.propspec.AnyPropSpec
import org.scalacheck.Prop.forAll
import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks
import org.scalacheck.{Gen, Arbitrary}

class GraphPropTest extends AnyPropSpec {
class GraphPropTest extends AnyPropSpec with ScalaCheckPropertyChecks {

def graphGen[N, E](g: Gen[(N, Option[(N, E)])]): Gen[Graph[N, E]] =
Gen.sized { s =>
Expand Down Expand Up @@ -89,21 +89,22 @@ class GraphPropTest extends AnyPropSpec {
property("Sanity checks on generated graphs (non-dag) 4") {
forAll(graphGen(genIntNode), nodeGen) { (g, n) =>
val newG = g.removeNode(n)
(!newG.nodes(n)) && (!newG.edgeIterator.exists { case Edge(s, d, _) =>
assert((!newG.nodes(n)) && (!newG.edgeIterator.exists { case Edge(s, d, _) =>
(s == n) || (d == n)
})
}))
}
}

property("Sanity checks on generated graphs (non-dag) 5") {
forAll(graphGen(genIntNode), nodeGen, nodeGen) { (g, s, d) =>
val newEdge = Edge(s, d, ())
val newG = g.addEdge(newEdge)
newG.nodes(s) &&
newG.nodes(d) &&
newG.hasSource(s)(newEdge) &&
newG.hasDestination(d)(newEdge) &&
newG.edgeIterator.toSet(newEdge)

assert(newG.nodes(s) &&
newG.nodes(d) &&
newG.hasSource(s)(newEdge) &&
newG.hasDestination(d)(newEdge) &&
newG.edgeIterator.toSet(newEdge))
}
}

Expand All @@ -114,8 +115,8 @@ class GraphPropTest extends AnyPropSpec {
}) {
case (g, Some(e)) =>
val newG = g.removeEdge(e)
!(newG.edgeIterator.exists(_ == e))
case (g, None) => true
assert(!(newG.edgeIterator.exists(_ == e)))
case (g, None) => ()
}
}

Expand All @@ -126,9 +127,12 @@ class GraphPropTest extends AnyPropSpec {
}) {
case (g, Some(n)) =>
val newG = g.removeNode(n)
newG.hasDestination(n).isEmpty &&
newG.hasSource(n).isEmpty &&
(!newG.nodes(n))

assert(
newG.hasDestination(n).isEmpty &&
newG.hasSource(n).isEmpty &&
(!newG.nodes(n))
)
case (g, None) => true
}
}
Expand All @@ -142,7 +146,7 @@ class GraphPropTest extends AnyPropSpec {
} yield (g, optPair)

forAll(genEnds) {
case (g, Some((s, e))) => g.reflexiveTransitiveClosure(List(s))(e)
case (g, Some((s, e))) => assert(g.reflexiveTransitiveClosure(List(s))(e))
case (g, None) => true
}
}
Expand All @@ -152,6 +156,7 @@ class GraphPropTest extends AnyPropSpec {
g <- graphGen(genIntNode)
e <- edgeFrom(g)
} yield (g, e)

forAll(gen, nodeGen, nodeGen) { case ((g, e), n1, n2) =>
val maybePresent = Edge(n1, n2, ())
val maybeCheck =
Expand All @@ -164,9 +169,9 @@ class GraphPropTest extends AnyPropSpec {
g.hasDestination(n2).contains(maybePresent))
}

g.edgeIterator.forall(g.hasEdge) &&
assert(g.edgeIterator.forall(g.hasEdge) &&
e.forall(g.hasEdge(_)) &&
maybeCheck
maybeCheck)
}
}
}
}
87 changes: 87 additions & 0 deletions test/scala/com/github/johnynek/bazel_deps/TryMergeTest.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package com.github.johnynek.bazel_deps

import org.scalatest.propspec.AnyPropSpec
import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks
import org.scalacheck.{Prop, Gen, Arbitrary}
import cats.{Semigroup, Eq}
import scala.util.{Success, Failure}
import scala.collection.immutable.SortedMap

import TryMerge.tryMerge
import java.util.concurrent.Semaphore

class TryMergeTest extends AnyPropSpec with ScalaCheckPropertyChecks {
def associative[A: TryMerge: Eq](a: A, b: A, c: A) = {
val right = tryMerge(None, b, c).flatMap(tryMerge(None, a, _))
val left = tryMerge(None, a, b).flatMap(tryMerge(None, _, c))

(left, right) match {
case (Success(l), Success(r)) => assert(Eq.eqv(l, r))
case (Failure(_), Failure(_)) => ()
case mismatch => fail(s"associative mismatch: $a, $b, $c => $mismatch")
}
}

implicit val tryMergeI: TryMerge[Int] =
new TryMerge[Int] {
def tryMerge(debug: Option[String], a: Int, b: Int) =
// take the high 16 bits of a and low 16 bits of b
// this is associative but not commutative
Success((a & 0xFFFF0000) | (b & 0x0000FFFF))
}

def revTry[A: TryMerge]: TryMerge[A] = implicitly[TryMerge[A]].reverse

property("try merge is associative Int") {
forAll { (a: Int, b: Int, c: Int) =>
associative(a, b, c)
associative(a, b, c)(revTry, implicitly)
}
}


property("try merge is associative Option[Int]") {
forAll { (a: Option[Int], b: Option[Int], c: Option[Int]) =>
associative(a, b, c)
associative(a, b, c)(revTry, implicitly)
}
}

property("try merge is associative SortedMap[String, Int]") {
forAll { (a: SortedMap[String, Int], b: SortedMap[String, Int], c: SortedMap[String, Int]) =>
associative(a, b, c)
associative(a, b, c)(revTry, implicitly)
}
}

property("try merge with taking this rhs is the same as ++ on SortedMap") {
forAll { (a: SortedMap[String, Int], b: SortedMap[String, Int]) =>
implicit val rhs = new TryMerge[Int] {
def tryMerge(d: Option[String], a: Int, b: Int) = Success(b)
}
assert(tryMerge(None, a, b) == Success(a ++ b))
}
}

property("if we always return it's the same as a Semigroup") {
forAll { (a: SortedMap[String, Int], b: SortedMap[String, Int]) =>
implicit val sgBits = new Semigroup[Int] {
def combine(a: Int, b: Int) = (a & 0xFFFF0000) | (b & 0x0000FFFF)
}
val sm = new Semigroup[SortedMap[String, Int]] {
def combine(a: SortedMap[String, Int], b: SortedMap[String, Int]) = {
val allKeys = a.keySet | b.keySet
SortedMap(allKeys.iterator.map { k =>
(a.get(k), b.get(k)) match {
case (Some(l), Some(r)) => (k, sgBits.combine(l, r))
case (Some(l), None) => (k, l)
case (None, Some(r)) => (k, r)
case (None, None) => sys.error(s"unreachable: non-sense key: $k")
}
}.toList: _*)
}
}
assert(tryMerge(None, a, b) == Success(sm.combine(a, b)))
}
}
}

0 comments on commit 3043d5e

Please sign in to comment.