From 3cb19115cf3ebe1d0e0a8c15154109bc91abb65d Mon Sep 17 00:00:00 2001 From: Max Leuthaeuser <1417198+max-leuthaeuser@users.noreply.github.com> Date: Sun, 10 May 2026 14:29:47 +0200 Subject: [PATCH 1/8] Address review findings for merge safety and errors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../impl/AbstractCompartment.scala | 5 ++-- .../compartment/impl/MultiCompartment.scala | 4 +-- .../scroll/internal/errors/SCROLLErrors.scala | 4 +-- .../graph/impl/CachedScalaRoleGraph.scala | 4 +-- .../AbstractParameterizedSCROLLTest.scala | 14 ++++++++++- .../parameterized/CompartmentMergeTest.scala | 24 +++++++++--------- .../CompartmentRoleFeaturesTest.scala | 25 +++++++++++++++++++ .../MultiCompartmentMergeTest.scala | 24 +++++++++--------- .../MultiCompartmentRoleFeaturesTest.scala | 13 ++++++++++ .../parameterized/MultiCompartmentTest.scala | 1 + 10 files changed, 85 insertions(+), 33 deletions(-) diff --git a/core/src/main/scala/scroll/internal/compartment/impl/AbstractCompartment.scala b/core/src/main/scala/scroll/internal/compartment/impl/AbstractCompartment.scala index 6fd77d09..0e65ebfe 100644 --- a/core/src/main/scala/scroll/internal/compartment/impl/AbstractCompartment.scala +++ b/core/src/main/scala/scroll/internal/compartment/impl/AbstractCompartment.scala @@ -3,6 +3,7 @@ package scroll.internal.compartment.impl import scroll.internal.compartment.CompartmentApi import scroll.internal.dispatch.DispatchQuery import scroll.internal.dispatch.impl.SCROLLDispatchable +import scroll.internal.errors.SCROLLErrors.SCROLLError import scroll.internal.errors.SCROLLErrors.TypeError import scroll.internal.errors.SCROLLErrors.TypeNotFound import scroll.internal.graph.RoleGraphProxyApi @@ -37,8 +38,8 @@ abstract class AbstractCompartment() extends CompartmentApi { override lazy val roleGroups: RoleGroupsApi = new RoleGroups(roleGraph) override lazy val playerEquality: PlayerEqualityApi = new PlayerEquality(roleGraph) - implicit def either2TorException[T](either: Either[?, T]): T = - either.fold(l => throw new RuntimeException(l.toString), r => r) + implicit def either2TorException[E <: SCROLLError, T](either: Either[E, T]): T = + either.fold(err => throw err, identity) protected def applyDispatchQuery(dispatchQuery: DispatchQuery, on: AnyRef): Seq[AnyRef] = roleGraph.plays.coreFor(on).lastOption match { diff --git a/core/src/main/scala/scroll/internal/compartment/impl/MultiCompartment.scala b/core/src/main/scala/scroll/internal/compartment/impl/MultiCompartment.scala index 2819f628..e8d6805a 100644 --- a/core/src/main/scala/scroll/internal/compartment/impl/MultiCompartment.scala +++ b/core/src/main/scala/scroll/internal/compartment/impl/MultiCompartment.scala @@ -14,8 +14,8 @@ import scala.reflect.ClassTag */ trait MultiCompartment extends AbstractCompartment { - implicit def either2SeqTOrException[T](either: Either[?, Seq[Either[?, T]]]): Seq[T] = - either.fold(left => throw new RuntimeException(left.toString), right => right.map(either2TorException)) + implicit def either2SeqTOrException[T](either: Either[SCROLLError, Seq[Either[SCROLLError, T]]]): Seq[T] = + either.fold(err => throw err, _.map(either2TorException)) override def newPlayer[W <: AnyRef: ClassTag](obj: W): MultiPlayer[W] = { require(null != obj) diff --git a/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala b/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala index 55b082b5..e71413b4 100644 --- a/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala +++ b/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala @@ -4,9 +4,9 @@ package scroll.internal.errors */ object SCROLLErrors { - sealed trait SCROLLError extends Throwable + sealed trait SCROLLError extends RuntimeException - sealed trait TypeError extends Throwable + sealed trait TypeError extends SCROLLError final case class TypeNotFound(tpe: Class[?]) extends TypeError { override def toString: String = s"Type '$tpe' could not be found!" diff --git a/core/src/main/scala/scroll/internal/graph/impl/CachedScalaRoleGraph.scala b/core/src/main/scala/scroll/internal/graph/impl/CachedScalaRoleGraph.scala index 7ef69353..27a4f0de 100644 --- a/core/src/main/scala/scroll/internal/graph/impl/CachedScalaRoleGraph.scala +++ b/core/src/main/scala/scroll/internal/graph/impl/CachedScalaRoleGraph.scala @@ -52,7 +52,7 @@ class CachedScalaRoleGraph( override def containsPlayer(player: AnyRef): Boolean = containsCache.get(player) override def detach(other: RoleGraph): Unit = { - require(other.isInstanceOf[CachedScalaRoleGraph], MERGE_MESSAGE) + require(other.isInstanceOf[ScalaRoleGraph], MERGE_MESSAGE) super.detach(other) resetAll() } @@ -64,7 +64,7 @@ class CachedScalaRoleGraph( override def facets(player: AnyRef): Seq[Enumeration#Value] = facetsCache.get(player) override def addPart(other: RoleGraph): Boolean = { - require(other.isInstanceOf[CachedScalaRoleGraph], MERGE_MESSAGE) + require(other.isInstanceOf[ScalaRoleGraph], MERGE_MESSAGE) if (super.addPart(other)) { resetAll() true diff --git a/tests/src/test/scala/scroll/tests/parameterized/AbstractParameterizedSCROLLTest.scala b/tests/src/test/scala/scroll/tests/parameterized/AbstractParameterizedSCROLLTest.scala index a6444443..d4da49d2 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/AbstractParameterizedSCROLLTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/AbstractParameterizedSCROLLTest.scala @@ -5,7 +5,19 @@ import scroll.tests.AbstractSCROLLTest abstract class AbstractParameterizedSCROLLTest extends AbstractSCROLLTest with TableDrivenPropertyChecks { + protected val graphModes = Seq((true, true), (true, false), (false, true), (false, false)) + protected val PARAMS = - Table(("cached", "checkForCycles"), (true, true), (true, false), (false, true), (false, false)) + Table(("cached", "checkForCycles"), graphModes*) + + protected val PARAM_PAIRS = + Table( + ("cachedA", "checkForCyclesA", "cachedB", "checkForCyclesB"), + graphModes.flatMap { case (cachedA, checkForCyclesA) => + graphModes.map { case (cachedB, checkForCyclesB) => + (cachedA, checkForCyclesA, cachedB, checkForCyclesB) + } + }* + ) } diff --git a/tests/src/test/scala/scroll/tests/parameterized/CompartmentMergeTest.scala b/tests/src/test/scala/scroll/tests/parameterized/CompartmentMergeTest.scala index f1c53d18..f3d27b00 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/CompartmentMergeTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/CompartmentMergeTest.scala @@ -5,12 +5,12 @@ import scroll.tests.mocks._ class CompartmentMergeTest extends AbstractParameterizedSCROLLTest { test("union") { - forAll(PARAMS) { (c: Boolean, cc: Boolean) => + forAll(PARAM_PAIRS) { (cachedA: Boolean, checkForCyclesA: Boolean, cachedB: Boolean, checkForCyclesB: Boolean) => val core = new CoreA() - val compA = new CompartmentUnderTest(c, cc) { + val compA = new CompartmentUnderTest(cachedA, checkForCyclesA) { core play new RoleA() } - val compB = new CompartmentUnderTest(c, cc) { + val compB = new CompartmentUnderTest(cachedB, checkForCyclesB) { core play new RoleB() } compA.compartmentRelations.union(compB) @@ -25,12 +25,12 @@ class CompartmentMergeTest extends AbstractParameterizedSCROLLTest { } test("combine") { - forAll(PARAMS) { (c: Boolean, cc: Boolean) => + forAll(PARAM_PAIRS) { (cachedA: Boolean, checkForCyclesA: Boolean, cachedB: Boolean, checkForCyclesB: Boolean) => val core = new CoreA() - val compA = new CompartmentUnderTest(c, cc) { + val compA = new CompartmentUnderTest(cachedA, checkForCyclesA) { core play new RoleA() } - val compB = new CompartmentUnderTest(c, cc) { + val compB = new CompartmentUnderTest(cachedB, checkForCyclesB) { core play new RoleB() } compA.compartmentRelations.combine(compB) @@ -45,12 +45,12 @@ class CompartmentMergeTest extends AbstractParameterizedSCROLLTest { } test("partOf") { - forAll(PARAMS) { (c: Boolean, cc: Boolean) => + forAll(PARAM_PAIRS) { (cachedA: Boolean, checkForCyclesA: Boolean, cachedB: Boolean, checkForCyclesB: Boolean) => val core = new CoreA() - val compA = new CompartmentUnderTest(c, cc) { + val compA = new CompartmentUnderTest(cachedA, checkForCyclesA) { core play new RoleA() } - val compB = new CompartmentUnderTest(c, cc) { + val compB = new CompartmentUnderTest(cachedB, checkForCyclesB) { core play new RoleB() } compA.compartmentRelations.partOf(compB) @@ -62,17 +62,17 @@ class CompartmentMergeTest extends AbstractParameterizedSCROLLTest { } test("notPartOf") { - forAll(PARAMS) { (c: Boolean, cc: Boolean) => + forAll(PARAM_PAIRS) { (cachedA: Boolean, checkForCyclesA: Boolean, cachedB: Boolean, checkForCyclesB: Boolean) => val core = new CoreA() class SomeRoleA class SomeRoleB val roleA = new SomeRoleA val roleB = new SomeRoleB - val compA = new CompartmentUnderTest(c, cc) { + val compA = new CompartmentUnderTest(cachedA, checkForCyclesA) { core play roleA core play roleB } - val compB = new CompartmentUnderTest(c, cc) { + val compB = new CompartmentUnderTest(cachedB, checkForCyclesB) { core play roleB } compA.compartmentRelations.notPartOf(compB) diff --git a/tests/src/test/scala/scroll/tests/parameterized/CompartmentRoleFeaturesTest.scala b/tests/src/test/scala/scroll/tests/parameterized/CompartmentRoleFeaturesTest.scala index c76d74bd..dd8b9729 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/CompartmentRoleFeaturesTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/CompartmentRoleFeaturesTest.scala @@ -3,10 +3,35 @@ package scroll.tests.parameterized import scroll.internal.dispatch.DispatchQuery import scroll.internal.dispatch.DispatchQuery._ import scroll.internal.errors.SCROLLErrors.RoleNotFound +import scroll.internal.errors.SCROLLErrors.TypeNotFound import scroll.tests.mocks._ class CompartmentRoleFeaturesTest extends AbstractParameterizedSCROLLTest { + test("Implicit unwrapping preserves RoleNotFound") { + forAll(PARAMS) { (c: Boolean, cc: Boolean) => + val someCore = new CoreA() + new CompartmentUnderTest(c, cc) { + val err = intercept[RoleNotFound] { + val _: String = (+someCore).missingMethod() + } + err.forCore shouldBe someCore + err.target shouldBe "missingMethod" + } + } + } + + test("Implicit unwrapping preserves TypeNotFound") { + forAll(PARAMS) { (c: Boolean, cc: Boolean) => + new CompartmentUnderTest(c, cc) { + val err = intercept[TypeNotFound] { + val _: RoleA = roleQueries.one[RoleA]() + } + err.tpe shouldBe classOf[RoleA] + } + } + } + test("Dropping role and invoking methods") { forAll(PARAMS) { (c: Boolean, cc: Boolean) => val someCore = new CoreA() diff --git a/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentMergeTest.scala b/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentMergeTest.scala index 0830f65d..056d21ae 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentMergeTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentMergeTest.scala @@ -5,12 +5,12 @@ import scroll.tests.mocks._ class MultiCompartmentMergeTest extends AbstractParameterizedSCROLLTest { test("union") { - forAll(PARAMS) { (c: Boolean, cc: Boolean) => + forAll(PARAM_PAIRS) { (cachedA: Boolean, checkForCyclesA: Boolean, cachedB: Boolean, checkForCyclesB: Boolean) => val core = new CoreA() - val compA = new MultiCompartmentUnderTest(c, cc) { + val compA = new MultiCompartmentUnderTest(cachedA, checkForCyclesA) { core play new RoleA() } - val compB = new MultiCompartmentUnderTest(c, cc) { + val compB = new MultiCompartmentUnderTest(cachedB, checkForCyclesB) { core play new RoleB() } compA.compartmentRelations.union(compB) @@ -25,12 +25,12 @@ class MultiCompartmentMergeTest extends AbstractParameterizedSCROLLTest { } test("combine") { - forAll(PARAMS) { (c: Boolean, cc: Boolean) => + forAll(PARAM_PAIRS) { (cachedA: Boolean, checkForCyclesA: Boolean, cachedB: Boolean, checkForCyclesB: Boolean) => val core = new CoreA() - val compA = new MultiCompartmentUnderTest(c, cc) { + val compA = new MultiCompartmentUnderTest(cachedA, checkForCyclesA) { core play new RoleA() } - val compB = new MultiCompartmentUnderTest(c, cc) { + val compB = new MultiCompartmentUnderTest(cachedB, checkForCyclesB) { core play new RoleB() } compA.compartmentRelations.combine(compB) @@ -45,12 +45,12 @@ class MultiCompartmentMergeTest extends AbstractParameterizedSCROLLTest { } test("partOf") { - forAll(PARAMS) { (c: Boolean, cc: Boolean) => + forAll(PARAM_PAIRS) { (cachedA: Boolean, checkForCyclesA: Boolean, cachedB: Boolean, checkForCyclesB: Boolean) => val core = new CoreA() - val compA = new MultiCompartmentUnderTest(c, cc) { + val compA = new MultiCompartmentUnderTest(cachedA, checkForCyclesA) { core play new RoleA() } - val compB = new MultiCompartmentUnderTest(c, cc) { + val compB = new MultiCompartmentUnderTest(cachedB, checkForCyclesB) { core play new RoleB() } compA.compartmentRelations.partOf(compB) @@ -62,17 +62,17 @@ class MultiCompartmentMergeTest extends AbstractParameterizedSCROLLTest { } test("notPartOf") { - forAll(PARAMS) { (c: Boolean, cc: Boolean) => + forAll(PARAM_PAIRS) { (cachedA: Boolean, checkForCyclesA: Boolean, cachedB: Boolean, checkForCyclesB: Boolean) => val core = new CoreA() class SomeRoleA class SomeRoleB val roleA = new SomeRoleA val roleB = new SomeRoleB - val compA = new MultiCompartmentUnderTest(c, cc) { + val compA = new MultiCompartmentUnderTest(cachedA, checkForCyclesA) { core play roleA core play roleB } - val compB = new MultiCompartmentUnderTest(c, cc) { + val compB = new MultiCompartmentUnderTest(cachedB, checkForCyclesB) { core play roleB } compA.compartmentRelations.notPartOf(compB) diff --git a/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentRoleFeaturesTest.scala b/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentRoleFeaturesTest.scala index c663b95d..cc4548bf 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentRoleFeaturesTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentRoleFeaturesTest.scala @@ -7,6 +7,19 @@ import scroll.tests.mocks._ class MultiCompartmentRoleFeaturesTest extends AbstractParameterizedSCROLLTest { + test("Implicit unwrapping preserves RoleNotFound") { + forAll(PARAMS) { (c: Boolean, cc: Boolean) => + val someCore = new CoreA() + new MultiCompartmentUnderTest(c, cc) { + val err = intercept[RoleNotFound] { + val _: Seq[String] = (+someCore).missingMethod() + } + err.forCore shouldBe someCore + err.target shouldBe "missingMethod" + } + } + } + test("Dropping role and invoking methods") { forAll(PARAMS) { (c: Boolean, cc: Boolean) => val someCore = new CoreA() diff --git a/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentTest.scala b/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentTest.scala index 2bd1c9b1..d13ed883 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentTest.scala @@ -13,6 +13,7 @@ class MultiCompartmentTest extends AbstractParameterizedSCROLLTest { (+p).i() match { case Right(_) => fail("There should be no Right here") case Left(f @ IllegalRoleInvocationDispatch(_, _, _)) => fail(f.toString) + case Left(TypeNotFound(tpe)) => fail(s"Unexpected type lookup failure for '$tpe'") case Left(RoleNotFound(core, _, _)) => core shouldBe p } From ac3c8a15043e3e2c330f075d917821be54f6892b Mon Sep 17 00:00:00 2001 From: Max Leuthaeuser <1417198+max-leuthaeuser@users.noreply.github.com> Date: Sun, 10 May 2026 14:29:47 +0200 Subject: [PATCH 2/8] Improve reflective overload resolution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../internal/util/ReflectiveHelper.scala | 97 ++++++++++++++++--- .../tests/other/ReflectiveHelperTest.scala | 43 ++++++++ 2 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 tests/src/test/scala/scroll/tests/other/ReflectiveHelperTest.scala diff --git a/core/src/main/scala/scroll/internal/util/ReflectiveHelper.scala b/core/src/main/scala/scroll/internal/util/ReflectiveHelper.scala index c720718c..dd27ff47 100644 --- a/core/src/main/scala/scroll/internal/util/ReflectiveHelper.scala +++ b/core/src/main/scala/scroll/internal/util/ReflectiveHelper.scala @@ -16,6 +16,17 @@ object ReflectiveHelper { import Memoiser._ + private val primitiveWrappers: Map[Class[?], Class[?]] = Map( + java.lang.Boolean.TYPE -> classOf[java.lang.Boolean], + java.lang.Character.TYPE -> classOf[java.lang.Character], + java.lang.Short.TYPE -> classOf[java.lang.Short], + java.lang.Integer.TYPE -> classOf[java.lang.Integer], + java.lang.Long.TYPE -> classOf[java.lang.Long], + java.lang.Float.TYPE -> classOf[java.lang.Float], + java.lang.Double.TYPE -> classOf[java.lang.Double], + java.lang.Byte.TYPE -> classOf[java.lang.Byte] + ) + private lazy val methodCache = buildCache[Class[?], Seq[Method]](allMethods) @@ -145,26 +156,90 @@ object ReflectiveHelper { private def isSameNumberOfParameters(m: Method, size: Int): Boolean = m.getParameterCount == size + private def wrappedType(tpe: Class[?]): Class[?] = primitiveWrappers.getOrElse(tpe, tpe) + private def isSameArgumentTypes(m: Method, args: Seq[Any]): Boolean = args.zip(m.getParameterTypes).forall { case (arg, paramType) => - paramType match { - case java.lang.Boolean.TYPE => arg.isInstanceOf[Boolean] - case java.lang.Character.TYPE => arg.isInstanceOf[Char] - case java.lang.Short.TYPE => arg.isInstanceOf[Short] - case java.lang.Integer.TYPE => arg.isInstanceOf[Integer] - case java.lang.Long.TYPE => arg.isInstanceOf[Long] - case java.lang.Float.TYPE => arg.isInstanceOf[Float] - case java.lang.Double.TYPE => arg.isInstanceOf[Double] - case java.lang.Byte.TYPE => arg.isInstanceOf[Byte] - case _ => arg == null || paramType.isAssignableFrom(arg.getClass) + if (arg == null) { + !paramType.isPrimitive + } else { + wrappedType(paramType).isAssignableFrom(arg.getClass) } } private def matchMethod(m: Method, args: Seq[Any]): Boolean = isSameNumberOfParameters(m, args.size) && isSameArgumentTypes(m, args) + private def inheritanceDistance(from: Class[?], to: Class[?]): Int = + if (from == to) { + 0 + } else { + val directParents = Option(from.getSuperclass).toSeq ++ from.getInterfaces.toSeq + directParents + .filter(to.isAssignableFrom) + .map(parent => inheritanceDistance(parent, to)) + .collect { case distance if distance != Int.MaxValue => distance + 1 } + .minOption + .getOrElse(Int.MaxValue) + } + + private def compareParameterSpecificity(arg: Any, left: Class[?], right: Class[?]): Int = { + val leftType = wrappedType(left) + val rightType = wrappedType(right) + + if (leftType == rightType) { + 0 + } else if (arg != null) { + val argType = arg.getClass + val leftDistance = inheritanceDistance(argType, leftType) + val rightDistance = inheritanceDistance(argType, rightType) + + if (leftDistance < rightDistance) { + -1 + } else if (rightDistance < leftDistance) { + 1 + } else if (leftType.isAssignableFrom(rightType)) { + 1 + } else if (rightType.isAssignableFrom(leftType)) { + -1 + } else { + 0 + } + } else if (leftType.isAssignableFrom(rightType)) { + 1 + } else if (rightType.isAssignableFrom(leftType)) { + -1 + } else { + 0 + } + } + + private def declaringClassDistance(on: Class[?], declaringClass: Class[?]): Int = + inheritanceDistance(on, declaringClass) + + private def selectMostSpecificMethod(on: Class[?], methods: Seq[Method], args: Seq[Any]): Option[Method] = + methods.reduceLeftOption { (best, candidate) => + val comparisons = + args.zip(best.getParameterTypes.zip(candidate.getParameterTypes)).map { case (arg, (bestType, candidateType)) => + compareParameterSpecificity(arg, bestType, candidateType) + } + + val candidateBetter = comparisons.contains(1) + val bestBetter = comparisons.contains(-1) + + if (candidateBetter && !bestBetter) { + candidate + } else if (bestBetter && !candidateBetter) { + best + } else { + val bestDeclaringDistance = declaringClassDistance(on, best.getDeclaringClass) + val candidateDeclaringDistance = declaringClassDistance(on, candidate.getDeclaringClass) + if (candidateDeclaringDistance < bestDeclaringDistance) candidate else best + } + } + private def cachedFindMethod(on: Class[?], name: String, args: Seq[Any]): Option[Method] = - findMethods(on, name).find(matchMethod(_, args)) + selectMostSpecificMethod(on, findMethods(on, name).filter(matchMethod(_, args)), args) /** Find a method of the wrapped object by its name and argument list given. * diff --git a/tests/src/test/scala/scroll/tests/other/ReflectiveHelperTest.scala b/tests/src/test/scala/scroll/tests/other/ReflectiveHelperTest.scala new file mode 100644 index 00000000..35ba9ded --- /dev/null +++ b/tests/src/test/scala/scroll/tests/other/ReflectiveHelperTest.scala @@ -0,0 +1,43 @@ +package scroll.tests.other + +import scroll.internal.util.ReflectiveHelper +import scroll.tests.AbstractSCROLLTest + +class ReflectiveHelperTest extends AbstractSCROLLTest { + + test("findMethod prefers the most specific overload for a subclass argument") { + val output = new java.io.ByteArrayOutputStream() + val target = new java.io.PrintStream(output) + val method = ReflectiveHelper.findMethod(target, "print", Seq("value")) + + method.map(_.getParameterTypes.toSeq) shouldBe Some(Seq(classOf[String])) + ReflectiveHelper.resultOf[Unit](target, method.get, Seq("value")) + output.toString shouldBe "value" + } + + test("findMethod prefers the most specific reference overload for null arguments") { + class Overloaded { + def dispatch(value: CharSequence): String = "char-sequence" + def dispatch(value: String): String = "string" + } + + val target = new Overloaded() + val method = ReflectiveHelper.findMethod(target, "dispatch", Seq(null)) + + method.map(_.getParameterTypes.toSeq) shouldBe Some(Seq(classOf[String])) + ReflectiveHelper.resultOf[String](target, method.get, Seq(null)) shouldBe "string" + } + + test("findMethod matches primitive parameters against boxed Scala arguments") { + class PrimitiveTarget { + def increment(value: Int): Int = value + 1 + } + + val target = new PrimitiveTarget() + val method = ReflectiveHelper.findMethod(target, "increment", Seq(1)) + + method.map(_.getParameterTypes.toSeq) shouldBe Some(Seq(java.lang.Integer.TYPE)) + ReflectiveHelper.resultOf[Int](target, method.get, Seq(1)) shouldBe 2 + } + +} From 23a77a10d738c7561666c15ee0e36eae781dc622 Mon Sep 17 00:00:00 2001 From: Max Leuthaeuser <1417198+max-leuthaeuser@users.noreply.github.com> Date: Sun, 10 May 2026 14:29:47 +0200 Subject: [PATCH 3/8] Harden mutable state handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../internal/rpa/RolePlayingAutomaton.scala | 76 ++++++++----- .../internal/support/impl/RoleGroups.scala | 100 +++++++++--------- .../support/impl/RoleRestrictions.scala | 33 +++--- .../RoleRestrictionsConcurrencyTest.scala | 66 ++++++++++++ .../tests/parameterized/RoleGroupsTest.scala | 25 +++++ .../RolePlayingAutomatonTest.scala | 46 ++++++++ 6 files changed, 260 insertions(+), 86 deletions(-) create mode 100644 tests/src/test/scala/scroll/tests/other/RoleRestrictionsConcurrencyTest.scala diff --git a/core/src/main/scala/scroll/internal/rpa/RolePlayingAutomaton.scala b/core/src/main/scala/scroll/internal/rpa/RolePlayingAutomaton.scala index cf8df429..bc6e1fe9 100644 --- a/core/src/main/scala/scroll/internal/rpa/RolePlayingAutomaton.scala +++ b/core/src/main/scala/scroll/internal/rpa/RolePlayingAutomaton.scala @@ -119,7 +119,8 @@ object RolePlayingAutomaton { */ trait RolePlayingAutomaton { - private val runtime = Runtime.default + private val runtime = Runtime.default + private val stateLock = new AnyRef private val queue: Queue[RPAData] = Unsafe.unsafe { implicit u => runtime.unsafe.run(Queue.unbounded[RPAData]).getOrThrow() @@ -139,11 +140,15 @@ trait RolePlayingAutomaton { /** Register event handler for a state. */ def when(state: RPAState)(handler: PartialFunction[Event, Transition]): Unit = - handlers = handlers.updated(state, handler) + stateLock.synchronized { + handlers = handlers.updated(state, handler) + } /** Register transition handler. Multiple handlers can be chained. */ def onTransition(handler: PartialFunction[(RPAState, RPAState), Unit]): Unit = - transitionHandler = transitionHandler.orElse(handler) + stateLock.synchronized { + transitionHandler = transitionHandler.orElse(handler) + } def goto(state: RPAState, data: RPAData = Uninitialized): Transition = Transition(state, data) @@ -154,24 +159,30 @@ trait RolePlayingAutomaton { /** Stops this automaton by interrupting the processing fiber. */ def halt(): Unit = { - currentState = Stop - fiber.foreach { f => + val activeFiber = stateLock.synchronized { + currentState = Stop + val runningFiber = fiber + fiber = None + runningFiber + } + activeFiber.foreach { f => Unsafe.unsafe { implicit u => runtime.unsafe.run(f.interrupt).getOrThrow() } } - fiber = None } private def start(): Unit = - if (fiber.isEmpty) { - currentState = Start - currentData = Uninitialized - val loop = processLoop - val startedFiber = Unsafe.unsafe { implicit u => - runtime.unsafe.run(loop.forkDaemon).getOrThrow() + stateLock.synchronized { + if (fiber.isEmpty) { + currentState = Start + currentData = Uninitialized + val loop = processLoop + val startedFiber = Unsafe.unsafe { implicit u => + runtime.unsafe.run(loop.forkDaemon).getOrThrow() + } + fiber = Some(startedFiber) } - fiber = Some(startedFiber) } private def enqueue(data: RPAData): Unit = @@ -181,19 +192,36 @@ trait RolePlayingAutomaton { private def processLoop: ZIO[Any, Nothing, Unit] = queue.take.flatMap { message => - val state = currentState - val data = currentData - handlers.get(state) match { - case Some(handler) if handler.isDefinedAt(Event(message, data)) => - val Transition(nextState, nextData) = handler(Event(message, data)) - currentState = nextState - currentData = nextData - if (transitionHandler.isDefinedAt(state -> nextState)) { - transitionHandler(state -> nextState) + val snapshot = stateLock.synchronized { + val state = currentState + val data = currentData + handlers.get(state) match { + case Some(handler) if handler.isDefinedAt(Event(message, data)) => Some((state, data, handler)) + case _ => None + } + } + + snapshot match { + case Some((state, data, handler)) => + val Transition(nextState, nextData) = handler(Event(message, data)) + val (shouldContinue, transitionAction) = stateLock.synchronized { + if (fiber.isEmpty || currentState == Stop) { + (false, Option.empty[() => Unit]) + } else { + currentState = nextState + currentData = nextData + val action = transitionHandler.lift(state -> nextState).map(handler => () => handler) + (nextState != Stop, action) + } } - if (nextState == Stop) ZIO.unit else processLoop + + transitionAction.foreach(_()) + if (shouldContinue) processLoop else ZIO.unit case _ => - processLoop + val shouldContinue = stateLock.synchronized { + fiber.nonEmpty && currentState != Stop + } + if (shouldContinue) processLoop else ZIO.unit } } diff --git a/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala b/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala index 8be6bb63..a6e0f916 100644 --- a/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala +++ b/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala @@ -8,6 +8,7 @@ import scroll.internal.support.RoleGroupsApi import scroll.internal.util.ReflectiveHelper import scala.collection.mutable +import scala.collection.concurrent.TrieMap import scala.reflect.ClassTag import scala.reflect.classTag @@ -15,17 +16,18 @@ class RoleGroups(private val roleGraph: RoleGraphProxyApi) extends RoleGroupsApi import RoleGroupsApi._ - private lazy val roleGroups = mutable.HashMap.empty[String, RoleGroup] + private lazy val roleGroups = TrieMap.empty[String, RoleGroup] override def checked(func: => Unit): Unit = { func - validate() + validate(roleGroups.values.toSeq) } override def create(name: String): RoleGroupApi = RoleGroup(name, Seq.empty, (0, 0), (0, 0)) - private def validateOccurrenceCardinality(): Unit = - roleGroups.foreach { case (name, rg) => + private def validateOccurrenceCardinality(groups: Seq[RoleGroup]): Unit = + groups.foreach { rg => + val name = rg.name val min = rg.occ._1 val max = rg.occ._2 val types = rg.types @@ -102,7 +104,6 @@ class RoleGroups(private val roleGraph: RoleGraphProxyApi) extends RoleGroupsApi } } ) { - rg.evaluated = true return resultRoleTypeSet.toSeq // scalastyle:ignore } @@ -113,59 +114,63 @@ class RoleGroups(private val roleGraph: RoleGraphProxyApi) extends RoleGroupsApi throw new RuntimeException(s"Constraint set for inner cardinality of role group '${rg.name}' violated!") } - private def eval(rg: RoleGroup): Seq[String] = { - val model = new Model(s"MODEL$$${rg.hashCode()}") - val types = rg.types - val numOfTypes = types.size - val sumName = "SUM$" + rg.name - - val (min, max) = (rg.limit._1, rg.limit._2) - - val (sum, op) = (min, max) match { - case _ if max.compare(min) == 0 && min == numOfTypes => - (model.intVar(sumName, numOfTypes), AND) - case _ if min == 1 && max.compare(numOfTypes) == 0 => - (model.intVar(sumName, 1, numOfTypes), OR) - case _ if min == 1 && max.compare(1) == 0 => (model.intVar(sumName, 1), XOR) - case _ if min == 0 && max.compare(0) == 0 => (model.intVar(sumName, 0), NOT) - case _ => - throw new RuntimeException(s"Role group constraint of ($min, $max) for role group '${rg.name}' not possible!") + private def resolveTypes(rg: RoleGroup, evaluatedGroups: mutable.Map[String, Seq[String]]): Seq[String] = + rg.entries.flatMap { + case ts: Types => ts.types + case nested: RoleGroup => eval(nested, evaluatedGroups) + case _ => + throw new RuntimeException("Role groups can only contain a list of types or role groups itself!") } - val constraintsMap = buildConstraintsMap(types, op, model, numOfTypes, min, max, rg) - model.post(model.sum(constraintsMap.values.toArray, "=", sum)) - solve(model, types, constraintsMap, rg) - } + private def eval(rg: RoleGroup, evaluatedGroups: mutable.Map[String, Seq[String]]): Seq[String] = + evaluatedGroups.getOrElseUpdate( + rg.name, { + val model = new Model(s"MODEL$$${rg.hashCode()}") + val types = resolveTypes(rg, evaluatedGroups) + val numOfTypes = types.size + val sumName = "SUM$" + rg.name + + val (min, max) = (rg.limit._1, rg.limit._2) + + val (sum, op) = (min, max) match { + case _ if max.compare(min) == 0 && min == numOfTypes => + (model.intVar(sumName, numOfTypes), AND) + case _ if min == 1 && max.compare(numOfTypes) == 0 => + (model.intVar(sumName, 1, numOfTypes), OR) + case _ if min == 1 && max.compare(1) == 0 => (model.intVar(sumName, 1), XOR) + case _ if min == 0 && max.compare(0) == 0 => (model.intVar(sumName, 0), NOT) + case _ => + throw new RuntimeException( + s"Role group constraint of ($min, $max) for role group '${rg.name}' not possible!" + ) + } + + val constraintsMap = buildConstraintsMap(types, op, model, numOfTypes, min, max, rg) + model.post(model.sum(constraintsMap.values.toArray, "=", sum)) + solve(model, types, constraintsMap, rg) + } + ) - private def validateInnerCardinality(): Unit = - try - roleGroups.values - .filter(!_.evaluated) - .foreach(eval) - finally roleGroups.values.foreach(_.evaluated = false) + private def validateInnerCardinality(groups: Seq[RoleGroup]): Unit = { + val evaluatedGroups = mutable.Map.empty[String, Seq[String]] + groups.foreach(eval(_, evaluatedGroups)) + } /** Checks all role groups. Will throw a RuntimeException if a role group constraint is violated! */ - private def validate(): Unit = { - validateOccurrenceCardinality() - validateInnerCardinality() + private def validate(groups: Seq[RoleGroup]): Unit = { + validateOccurrenceCardinality(groups) + validateInnerCardinality(groups) } private def addRoleGroup(rg: RoleGroup): RoleGroup = - if (roleGroups.exists { case (n, _) => n == rg.name }) { + if (roleGroups.putIfAbsent(rg.name, rg).nonEmpty) { throw new RuntimeException(s"The RoleGroup ${rg.name} was already added!") } else { - roleGroups(rg.name) = rg rg } - case class RoleGroup( - name: String, - entries: Seq[Entry], - limit: (Int, CInt), - occ: (Int, CInt), - var evaluated: Boolean = false - ) extends RoleGroupApi { + case class RoleGroup(name: String, entries: Seq[Entry], limit: (Int, CInt), occ: (Int, CInt)) extends RoleGroupApi { assert(occ._1 >= 0 && occ._2 >= occ._1) assert(limit._1 >= 0 && limit._2 >= limit._1) @@ -173,12 +178,7 @@ class RoleGroups(private val roleGraph: RoleGraphProxyApi) extends RoleGroupsApi ReflectiveHelper.simpleName(m.toString) override def types: Seq[String] = - entries.flatMap { - case ts: Types => ts.types - case rg: RoleGroup => eval(rg) - case _ => - throw new RuntimeException("Role groups can only contain a list of types or role groups itself!") - } + resolveTypes(this, mutable.Map.empty[String, Seq[String]]) override def containing( rg: RoleGroupApi* diff --git a/core/src/main/scala/scroll/internal/support/impl/RoleRestrictions.scala b/core/src/main/scala/scroll/internal/support/impl/RoleRestrictions.scala index 84e53ae0..59425de3 100644 --- a/core/src/main/scala/scroll/internal/support/impl/RoleRestrictions.scala +++ b/core/src/main/scala/scroll/internal/support/impl/RoleRestrictions.scala @@ -3,34 +3,43 @@ package scroll.internal.support.impl import scroll.internal.support.RoleRestrictionsApi import scroll.internal.util.ReflectiveHelper -import scala.collection.mutable +import scala.annotation.tailrec +import scala.collection.concurrent.TrieMap import scala.reflect.ClassTag import scala.reflect.classTag class RoleRestrictions() extends RoleRestrictionsApi { - private lazy val restrictions = mutable.HashMap.empty[String, List[Class[?]]] - - private def addToMap(m: mutable.Map[String, List[Class[?]]], elem: (String, Class[?])): Unit = { - val key = elem._1 - val value = elem._2 - m.update(key, m.getOrElseUpdate(key, List(value)) ++ List(value)) - } + private lazy val restrictions = TrieMap.empty[String, List[Class[?]]] + + @tailrec + private def addToMap(key: String, value: Class[?]): Unit = + restrictions.get(key) match { + case Some(existing) => + if (!restrictions.replace(key, existing, existing :+ value)) { + addToMap(key, value) + } + case None => + if (restrictions.putIfAbsent(key, List(value)).nonEmpty) { + addToMap(key, value) + } + } override def addRoleRestriction[A <: AnyRef: ClassTag, B <: AnyRef: ClassTag](): Unit = - addToMap(restrictions, (classTag[A].toString, classTag[B].runtimeClass)) + addToMap(classTag[A].toString, classTag[B].runtimeClass) override def replaceRoleRestriction[A <: AnyRef: ClassTag, B <: AnyRef: ClassTag](): Unit = - restrictions(classTag[A].toString) = List(classTag[B].runtimeClass) + restrictions.put(classTag[A].toString, List(classTag[B].runtimeClass)) override def removeRoleRestriction[A <: AnyRef: ClassTag](): Unit = { val _ = restrictions.remove(classTag[A].toString) } override def validate[R <: AnyRef: ClassTag](player: AnyRef, role: R): Unit = - if (restrictions.nonEmpty) { + val currentRestrictions = restrictions.toList + if (currentRestrictions.nonEmpty) { val roleInterface = classTag[R].runtimeClass.getDeclaredMethods if ( - restrictions.exists { case (pt, rts) => + currentRestrictions.exists { case (pt, rts) => ReflectiveHelper.isInstanceOf(pt, player.getClass.toString) && !rts .exists(r => ReflectiveHelper.isSameInterface(roleInterface, r.getDeclaredMethods)) } diff --git a/tests/src/test/scala/scroll/tests/other/RoleRestrictionsConcurrencyTest.scala b/tests/src/test/scala/scroll/tests/other/RoleRestrictionsConcurrencyTest.scala new file mode 100644 index 00000000..5a3a3920 --- /dev/null +++ b/tests/src/test/scala/scroll/tests/other/RoleRestrictionsConcurrencyTest.scala @@ -0,0 +1,66 @@ +package scroll.tests.other + +import scroll.internal.support.impl.RoleRestrictions +import scroll.tests.AbstractSCROLLTest +import scroll.tests.mocks.CoreA + +import java.util.concurrent.CountDownLatch +import scala.concurrent.Await +import scala.concurrent.ExecutionContext.Implicits.global +import scala.concurrent.Future +import scala.concurrent.duration.DurationInt + +class RoleRestrictionsConcurrencyTest extends AbstractSCROLLTest { + + class RoleOne { def one(): Int = 1 } + + class RoleTwo { def two(): Int = 2 } + + class RoleThree { def three(): Int = 3 } + + class RoleFour { def four(): Int = 4 } + + class RoleFive { def five(): Int = 5 } + + class RoleSix { def six(): Int = 6 } + + private def registerRestrictionsConcurrently(restrictions: RoleRestrictions): Unit = { + val start = new CountDownLatch(1) + val registrations = Seq[() => Unit]( + () => restrictions.addRoleRestriction[CoreA, RoleOne](), + () => restrictions.addRoleRestriction[CoreA, RoleTwo](), + () => restrictions.addRoleRestriction[CoreA, RoleThree](), + () => restrictions.addRoleRestriction[CoreA, RoleFour](), + () => restrictions.addRoleRestriction[CoreA, RoleFive](), + () => restrictions.addRoleRestriction[CoreA, RoleSix]() + ) + + val tasks = registrations.map { register => + Future { + start.await() + register() + } + } + + start.countDown() + Await.result(Future.sequence(tasks), 10.seconds) + } + + test("Concurrent restriction updates do not lose allowed role types") { + val player = new CoreA() + + (1 to 50).foreach { _ => + val restrictions = new RoleRestrictions() + + registerRestrictionsConcurrently(restrictions) + + noException should be thrownBy restrictions.validate(player, new RoleOne()) + noException should be thrownBy restrictions.validate(player, new RoleTwo()) + noException should be thrownBy restrictions.validate(player, new RoleThree()) + noException should be thrownBy restrictions.validate(player, new RoleFour()) + noException should be thrownBy restrictions.validate(player, new RoleFive()) + noException should be thrownBy restrictions.validate(player, new RoleSix()) + } + } + +} diff --git a/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala b/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala index 0c5e3c56..cc3566ee 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala @@ -41,4 +41,29 @@ class RoleGroupsTest extends AbstractParameterizedSCROLLTest { } } + test("Validating nested role groups repeatedly") { + forAll(PARAMS) { (c: Boolean, cc: Boolean) => + val acc1 = new CoreA() + val acc2 = new CoreA() + new CompartmentUnderTest(c, cc) { + + class Source + + class Target + + val source = new Source + val target = new Target + val pair = roleGroups.create("Pair").containing[Source, Target](1, 1)(2, 2) + roleGroups.create("Transaction").containing(pair)(1, 1)(2, 2) + + roleGroups.checked { + acc1 play source + acc2 play target + } + + roleGroups.checked {} + } + } + } + } diff --git a/tests/src/test/scala/scroll/tests/parameterized/RolePlayingAutomatonTest.scala b/tests/src/test/scala/scroll/tests/parameterized/RolePlayingAutomatonTest.scala index 9a5972a5..907a9319 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/RolePlayingAutomatonTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/RolePlayingAutomatonTest.scala @@ -6,6 +6,14 @@ import scroll.internal.rpa.RolePlayingAutomaton import scroll.internal.rpa.RolePlayingAutomaton._ import scroll.tests.mocks._ +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger +import scala.concurrent.Await +import scala.concurrent.ExecutionContext.Implicits.global +import scala.concurrent.Future +import scala.concurrent.duration.DurationInt + class RolePlayingAutomatonTest extends AbstractParameterizedSCROLLTest { class ACompartment(c: Boolean, cc: Boolean) extends CompartmentUnderTest(c, cc) { @@ -55,4 +63,42 @@ class RolePlayingAutomatonTest extends AbstractParameterizedSCROLLTest { } } + test("Concurrent run calls only start one processing loop") { + class CountingRPA extends RolePlayingAutomaton { + + private case object Running extends RPAState + + private val runningTransitionCount = new AtomicInteger(0) + private val runningSignal = new CountDownLatch(1) + + when(Start) { case Event(BindRole, _) => + Thread.sleep(50) + goto(Running) + } + + when(Running) { case Event(BindRole, _) => goto(Running) } + + onTransition { case Start -> Running => + runningTransitionCount.incrementAndGet() + runningSignal.countDown() + } + + def send(data: RPAData): Unit = self ! data + + def awaitRunning(): Boolean = runningSignal.await(5, TimeUnit.SECONDS) + + def transitionCount: Int = runningTransitionCount.get() + } + + val rpa = new CountingRPA() + Await.result(Future.sequence((1 to 8).map(_ => Future(rpa.run()))), 10.seconds) + + (1 to 8).foreach(_ => rpa.send(BindRole)) + + rpa.awaitRunning() shouldBe true + Thread.sleep(200) + rpa.transitionCount shouldBe 1 + rpa.halt() + } + } From 556efb73a2959559718bc5d1ea86092c5cc5a1b3 Mon Sep 17 00:00:00 2001 From: Max Leuthaeuser <1417198+max-leuthaeuser@users.noreply.github.com> Date: Sun, 10 May 2026 14:29:47 +0200 Subject: [PATCH 4/8] Introduce typed validation errors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../scroll/internal/errors/SCROLLErrors.scala | 74 +++++++++++++++++++ .../support/impl/RoleConstraints.scala | 15 ++-- .../internal/support/impl/RoleGroups.scala | 35 ++++----- .../support/impl/RoleRestrictions.scala | 3 +- .../parameterized/RoleConstraintsTest.scala | 21 +++--- .../tests/parameterized/RoleGroupsTest.scala | 5 +- .../parameterized/RoleRestrictionsTest.scala | 5 +- 7 files changed, 114 insertions(+), 44 deletions(-) diff --git a/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala b/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala index e71413b4..6cf83e94 100644 --- a/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala +++ b/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala @@ -8,6 +8,80 @@ object SCROLLErrors { sealed trait TypeError extends SCROLLError + sealed trait ValidationError extends RuntimeException { + override def getMessage: String = toString + } + + sealed trait ConstraintError extends ValidationError + + final case class RoleImplicationConstraintViolation(player: AnyRef, requiredRole: String) extends ConstraintError { + + override def toString: String = + s"Role implication constraint violation: '$player' should play role '$requiredRole', but it does not!" + + } + + final case class RoleEquivalenceConstraintViolation(player: AnyRef, requiredRole: String) extends ConstraintError { + + override def toString: String = + s"Role equivalence constraint violation: '$player' should play role '$requiredRole', but it does not!" + + } + + final case class RoleProhibitionConstraintViolation(player: AnyRef, prohibitedRole: String) extends ConstraintError { + + override def toString: String = + s"Role prohibition constraint violation: '$player' plays role '$prohibitedRole', but it is not allowed to do so!" + + } + + final case class RoleRestrictionViolation(player: AnyRef, role: AnyRef) extends ValidationError { + + override def toString: String = + s"Role '$role' can not be played by '$player' due to the active role restrictions!" + + } + + sealed trait RoleGroupError extends ValidationError + + final case class RoleGroupOccurrenceCardinalityViolation( + groupName: String, + roleTypes: Seq[String], + actual: Int, + min: Int, + max: String + ) extends RoleGroupError { + + override def toString: String = + s"Occurrence cardinality in role group '$groupName' violated! " + + s"Roles '$roleTypes' are played $actual times but should be between $min and $max." + + } + + final case class MissingRoleGroupConstraint(groupName: String, roleType: String) extends RoleGroupError { + override def toString: String = s"Constraints for role group '$groupName' do not contain '$roleType'!" + } + + final case class UnsolvableRoleGroupConstraint(groupName: String) extends RoleGroupError { + override def toString: String = s"Constraint set of role group '$groupName' unsolvable!" + } + + final case class RoleGroupInnerCardinalityViolation(groupName: String) extends RoleGroupError { + override def toString: String = s"Constraint set for inner cardinality of role group '$groupName' violated!" + } + + final case class InvalidRoleGroupEntry() extends RoleGroupError { + override def toString: String = "Role groups can only contain a list of types or role groups itself!" + } + + final case class InvalidRoleGroupConstraint(groupName: String, min: Int, max: String) extends RoleGroupError { + override def toString: String = s"Role group constraint of ($min, $max) for role group '$groupName' not possible!" + } + + final case class DuplicateRoleGroup(groupName: String) extends RoleGroupError { + override def toString: String = s"The RoleGroup $groupName was already added!" + } + final case class TypeNotFound(tpe: Class[?]) extends TypeError { override def toString: String = s"Type '$tpe' could not be found!" } diff --git a/core/src/main/scala/scroll/internal/support/impl/RoleConstraints.scala b/core/src/main/scala/scroll/internal/support/impl/RoleConstraints.scala index 2ddb8a2e..2ffd15c2 100644 --- a/core/src/main/scala/scroll/internal/support/impl/RoleConstraints.scala +++ b/core/src/main/scala/scroll/internal/support/impl/RoleConstraints.scala @@ -3,6 +3,9 @@ package scroll.internal.support.impl import com.google.common.graph.GraphBuilder import com.google.common.graph.Graphs import com.google.common.graph.MutableGraph +import scroll.internal.errors.SCROLLErrors.RoleEquivalenceConstraintViolation +import scroll.internal.errors.SCROLLErrors.RoleImplicationConstraintViolation +import scroll.internal.errors.SCROLLErrors.RoleProhibitionConstraintViolation import scroll.internal.graph.RoleGraphProxyApi import scroll.internal.support.RoleConstraintsApi import scroll.internal.util.ReflectiveHelper @@ -29,9 +32,7 @@ class RoleConstraints(private val roleGraph: RoleGraphProxyApi) extends RoleCons val allRoles = roleGraph.plays.roles(player) allImplicitRoles.foreach(r => if (!allRoles.exists(ReflectiveHelper.isInstanceOf(r, _))) { - throw new RuntimeException( - s"Role implication constraint violation: '$player' should play role '$r', but it does not!" - ) + throw RoleImplicationConstraintViolation(player, r) } ) } @@ -44,9 +45,7 @@ class RoleConstraints(private val roleGraph: RoleGraphProxyApi) extends RoleCons val allRoles = roleGraph.plays.roles(player) allEquivalentRoles.foreach(r => if (!allRoles.exists(ReflectiveHelper.isInstanceOf(r, _))) { - throw new RuntimeException( - s"Role equivalence constraint violation: '$player' should play role '$r', but it does not!" - ) + throw RoleEquivalenceConstraintViolation(player, r) } ) } @@ -68,9 +67,7 @@ class RoleConstraints(private val roleGraph: RoleGraphProxyApi) extends RoleCons .diff(list.toSet) .foreach(r => if (allRoles.exists(ReflectiveHelper.isInstanceOf(r, _))) { - throw new RuntimeException( - s"Role prohibition constraint violation: '$player' plays role '$r', but it is not allowed to do so!" - ) + throw RoleProhibitionConstraintViolation(player, r) } ) } diff --git a/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala b/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala index a6e0f916..7c1cccf8 100644 --- a/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala +++ b/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala @@ -3,6 +3,13 @@ package scroll.internal.support.impl import org.chocosolver.solver.Model import org.chocosolver.solver.Solution import org.chocosolver.solver.variables.IntVar +import scroll.internal.errors.SCROLLErrors.DuplicateRoleGroup +import scroll.internal.errors.SCROLLErrors.InvalidRoleGroupConstraint +import scroll.internal.errors.SCROLLErrors.InvalidRoleGroupEntry +import scroll.internal.errors.SCROLLErrors.MissingRoleGroupConstraint +import scroll.internal.errors.SCROLLErrors.RoleGroupInnerCardinalityViolation +import scroll.internal.errors.SCROLLErrors.RoleGroupOccurrenceCardinalityViolation +import scroll.internal.errors.SCROLLErrors.UnsolvableRoleGroupConstraint import scroll.internal.graph.RoleGraphProxyApi import scroll.internal.support.RoleGroupsApi import scroll.internal.util.ReflectiveHelper @@ -35,10 +42,7 @@ class RoleGroups(private val roleGraph: RoleGraphProxyApi) extends RoleGroupsApi .map(ts => roleGraph.plays.allPlayers.count(r => ts == ReflectiveHelper.simpleName(r.getClass.toString))) .sum if (actual < min || max < actual) { - throw new RuntimeException( - s"Occurrence cardinality in role group '$name' violated! " + - s"Roles '$types' are played $actual times but should be between $min and $max." - ) + throw RoleGroupOccurrenceCardinalityViolation(name, types, actual, min, max.toString) } } @@ -87,14 +91,7 @@ class RoleGroups(private val roleGraph: RoleGraphProxyApi) extends RoleGroupsApi val numRole = roleGraph.plays .roles(p) .count(r => t == ReflectiveHelper.simpleName(r.getClass.toString)) - if ( - numRole == s.getIntVal( - constraintsMap.getOrElse( - t, - throw new RuntimeException(s"Constraints for role group '${rg.name}' do not contain '$t'!") - ) - ) - ) { + if (numRole == s.getIntVal(constraintsMap.getOrElse(t, throw MissingRoleGroupConstraint(rg.name, t)))) { resultRoleTypeSet.add(t) true } else { @@ -108,18 +105,17 @@ class RoleGroups(private val roleGraph: RoleGraphProxyApi) extends RoleGroupsApi } } else { - throw new RuntimeException(s"Constraint set of role group '${rg.name}' unsolvable!") + throw UnsolvableRoleGroupConstraint(rg.name) } // give up - throw new RuntimeException(s"Constraint set for inner cardinality of role group '${rg.name}' violated!") + throw RoleGroupInnerCardinalityViolation(rg.name) } private def resolveTypes(rg: RoleGroup, evaluatedGroups: mutable.Map[String, Seq[String]]): Seq[String] = rg.entries.flatMap { case ts: Types => ts.types case nested: RoleGroup => eval(nested, evaluatedGroups) - case _ => - throw new RuntimeException("Role groups can only contain a list of types or role groups itself!") + case _ => throw InvalidRoleGroupEntry() } private def eval(rg: RoleGroup, evaluatedGroups: mutable.Map[String, Seq[String]]): Seq[String] = @@ -139,10 +135,7 @@ class RoleGroups(private val roleGraph: RoleGraphProxyApi) extends RoleGroupsApi (model.intVar(sumName, 1, numOfTypes), OR) case _ if min == 1 && max.compare(1) == 0 => (model.intVar(sumName, 1), XOR) case _ if min == 0 && max.compare(0) == 0 => (model.intVar(sumName, 0), NOT) - case _ => - throw new RuntimeException( - s"Role group constraint of ($min, $max) for role group '${rg.name}' not possible!" - ) + case _ => throw InvalidRoleGroupConstraint(rg.name, min, max.toString) } val constraintsMap = buildConstraintsMap(types, op, model, numOfTypes, min, max, rg) @@ -165,7 +158,7 @@ class RoleGroups(private val roleGraph: RoleGraphProxyApi) extends RoleGroupsApi private def addRoleGroup(rg: RoleGroup): RoleGroup = if (roleGroups.putIfAbsent(rg.name, rg).nonEmpty) { - throw new RuntimeException(s"The RoleGroup ${rg.name} was already added!") + throw DuplicateRoleGroup(rg.name) } else { rg } diff --git a/core/src/main/scala/scroll/internal/support/impl/RoleRestrictions.scala b/core/src/main/scala/scroll/internal/support/impl/RoleRestrictions.scala index 59425de3..286569ca 100644 --- a/core/src/main/scala/scroll/internal/support/impl/RoleRestrictions.scala +++ b/core/src/main/scala/scroll/internal/support/impl/RoleRestrictions.scala @@ -1,5 +1,6 @@ package scroll.internal.support.impl +import scroll.internal.errors.SCROLLErrors.RoleRestrictionViolation import scroll.internal.support.RoleRestrictionsApi import scroll.internal.util.ReflectiveHelper @@ -44,7 +45,7 @@ class RoleRestrictions() extends RoleRestrictionsApi { .exists(r => ReflectiveHelper.isSameInterface(roleInterface, r.getDeclaredMethods)) } ) { - throw new RuntimeException(s"Role '$role' can not be played by '$player' due to the active role restrictions!") + throw RoleRestrictionViolation(player, role) } } diff --git a/tests/src/test/scala/scroll/tests/parameterized/RoleConstraintsTest.scala b/tests/src/test/scala/scroll/tests/parameterized/RoleConstraintsTest.scala index 34077498..1b36b93d 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/RoleConstraintsTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/RoleConstraintsTest.scala @@ -1,5 +1,8 @@ package scroll.tests.parameterized +import scroll.internal.errors.SCROLLErrors.RoleEquivalenceConstraintViolation +import scroll.internal.errors.SCROLLErrors.RoleImplicationConstraintViolation +import scroll.internal.errors.SCROLLErrors.RoleProhibitionConstraintViolation import scroll.tests.mocks._ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { @@ -17,7 +20,7 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player play roleA play roleB } - the[RuntimeException] thrownBy { + the[RoleImplicationConstraintViolation] thrownBy { roleConstraints.checked { player drop roleB } @@ -33,13 +36,13 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player play roleC } - the[RuntimeException] thrownBy { + the[RoleImplicationConstraintViolation] thrownBy { roleConstraints.checked { player drop roleB } } should have message s"Role implication constraint violation: '$player' should play role '${roleB.getClass.getName}', but it does not!" - the[RuntimeException] thrownBy { + the[RoleImplicationConstraintViolation] thrownBy { roleConstraints.checked { player drop roleC } @@ -71,7 +74,7 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player play roleB } - the[RuntimeException] thrownBy { + the[RoleProhibitionConstraintViolation] thrownBy { roleConstraints.checked { player play roleA } @@ -84,7 +87,7 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player drop roleB } - the[RuntimeException] thrownBy { + the[RoleProhibitionConstraintViolation] thrownBy { roleConstraints.checked { player play roleA player play roleB @@ -109,7 +112,7 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { val roleC = new RoleC() roleConstraints.addRoleEquivalence[RoleA, RoleB]() - the[RuntimeException] thrownBy { + the[RoleEquivalenceConstraintViolation] thrownBy { roleConstraints.checked { player play roleA } @@ -119,7 +122,7 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player play roleB } - the[RuntimeException] thrownBy { + the[RoleEquivalenceConstraintViolation] thrownBy { roleConstraints.checked { player drop roleA } @@ -137,7 +140,7 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player play roleC } - the[RuntimeException] thrownBy { + the[RoleEquivalenceConstraintViolation] thrownBy { roleConstraints.checked { player drop roleB } @@ -156,7 +159,7 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { roleConstraints.addRoleImplication[RoleA, RoleB]() roleConstraints.addRoleProhibition[RoleA, RoleB]() - the[RuntimeException] thrownBy { + the[RoleProhibitionConstraintViolation] thrownBy { roleConstraints.checked { player play roleA player play roleB diff --git a/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala b/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala index cc3566ee..03ccc924 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala @@ -1,5 +1,6 @@ package scroll.tests.parameterized +import scroll.internal.errors.SCROLLErrors.RoleGroupInnerCardinalityViolation import scroll.tests.mocks.CompartmentUnderTest import scroll.tests.mocks.CoreA @@ -25,13 +26,13 @@ class RoleGroupsTest extends AbstractParameterizedSCROLLTest { acc2 play target } - the[RuntimeException] thrownBy { + the[RoleGroupInnerCardinalityViolation] thrownBy { roleGroups.checked { acc2 drop target } } should have message s"Constraint set for inner cardinality of role group '$roleGroupName' violated!" - the[RuntimeException] thrownBy { + the[RoleGroupInnerCardinalityViolation] thrownBy { roleGroups.checked { acc1 play target } diff --git a/tests/src/test/scala/scroll/tests/parameterized/RoleRestrictionsTest.scala b/tests/src/test/scala/scroll/tests/parameterized/RoleRestrictionsTest.scala index d839ed2c..8c998762 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/RoleRestrictionsTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/RoleRestrictionsTest.scala @@ -1,5 +1,6 @@ package scroll.tests.parameterized +import scroll.internal.errors.SCROLLErrors.RoleRestrictionViolation import scroll.tests.mocks._ class RoleRestrictionsTest extends AbstractParameterizedSCROLLTest { @@ -15,7 +16,7 @@ class RoleRestrictionsTest extends AbstractParameterizedSCROLLTest { player drop roleA roleRestrictions.replaceRoleRestriction[CoreA, RoleD]() - the[RuntimeException] thrownBy { + the[RoleRestrictionViolation] thrownBy { player play roleA } should have message s"Role '$roleA' can not be played by '$player' due to the active role restrictions!" @@ -35,7 +36,7 @@ class RoleRestrictionsTest extends AbstractParameterizedSCROLLTest { player play roleA player play roleD - the[RuntimeException] thrownBy { + the[RoleRestrictionViolation] thrownBy { player play roleB } should have message s"Role '$roleB' can not be played by '$player' due to the active role restrictions!" From 1e3d295d4db3625677133f51101ed6e52e80b5be Mon Sep 17 00:00:00 2001 From: Max Leuthaeuser <1417198+max-leuthaeuser@users.noreply.github.com> Date: Sun, 10 May 2026 14:29:47 +0200 Subject: [PATCH 5/8] Add public scroll facade Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 ++ core/src/main/scala/scroll/DispatchQuery.scala | 5 +++++ core/src/main/scala/scroll/Many.scala | 5 +++++ core/src/main/scala/scroll/package.scala | 11 +++++++++++ .../main/scala/scroll/examples/APICallsExample.scala | 6 +++--- .../src/main/scala/scroll/examples/BankExample.scala | 8 ++++---- .../scroll/examples/ExpressionProblemExample.scala | 2 +- .../src/main/scala/scroll/examples/RobotExample.scala | 2 +- .../scala/scroll/examples/UniversityExample.scala | 2 +- 9 files changed, 33 insertions(+), 10 deletions(-) create mode 100644 core/src/main/scala/scroll/DispatchQuery.scala create mode 100644 core/src/main/scala/scroll/Many.scala create mode 100644 core/src/main/scala/scroll/package.scala diff --git a/README.md b/README.md index d414e56b..d067db8e 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,8 @@ To provide a DSL for the pure embedding of roles in structured contexts, *SCROLL ## Example ## ```scala +import scroll.Compartment + // A Natural type, the player: class Person(val firstName: String) diff --git a/core/src/main/scala/scroll/DispatchQuery.scala b/core/src/main/scala/scroll/DispatchQuery.scala new file mode 100644 index 00000000..81e357c3 --- /dev/null +++ b/core/src/main/scala/scroll/DispatchQuery.scala @@ -0,0 +1,5 @@ +package scroll + +object DispatchQuery { + export internal.dispatch.DispatchQuery.* +} diff --git a/core/src/main/scala/scroll/Many.scala b/core/src/main/scala/scroll/Many.scala new file mode 100644 index 00000000..c12f70e9 --- /dev/null +++ b/core/src/main/scala/scroll/Many.scala @@ -0,0 +1,5 @@ +package scroll + +object Many { + export internal.util.Many.* +} diff --git a/core/src/main/scala/scroll/package.scala b/core/src/main/scala/scroll/package.scala new file mode 100644 index 00000000..9eab1d56 --- /dev/null +++ b/core/src/main/scala/scroll/package.scala @@ -0,0 +1,11 @@ +package object scroll { + + type Compartment = internal.compartment.impl.Compartment + + type MultiCompartment = internal.compartment.impl.MultiCompartment + + type DispatchQuery = internal.dispatch.DispatchQuery + + type Many = internal.util.Many + +} diff --git a/examples/src/main/scala/scroll/examples/APICallsExample.scala b/examples/src/main/scala/scroll/examples/APICallsExample.scala index d81d94ef..1b450d3d 100644 --- a/examples/src/main/scala/scroll/examples/APICallsExample.scala +++ b/examples/src/main/scala/scroll/examples/APICallsExample.scala @@ -1,8 +1,8 @@ package scroll.examples -import scroll.internal.compartment.impl.Compartment -import scroll.internal.dispatch.DispatchQuery -import scroll.internal.dispatch.DispatchQuery.Bypassing +import scroll.Compartment +import scroll.DispatchQuery +import scroll.DispatchQuery.Bypassing object APICallsExample { diff --git a/examples/src/main/scala/scroll/examples/BankExample.scala b/examples/src/main/scala/scroll/examples/BankExample.scala index 16ea9314..4bdb485e 100644 --- a/examples/src/main/scala/scroll/examples/BankExample.scala +++ b/examples/src/main/scala/scroll/examples/BankExample.scala @@ -1,9 +1,9 @@ package scroll.examples -import scroll.internal.compartment.impl.Compartment -import scroll.internal.dispatch.DispatchQuery -import scroll.internal.dispatch.DispatchQuery.Bypassing -import scroll.internal.util.Many.* +import scroll.Compartment +import scroll.DispatchQuery +import scroll.DispatchQuery.Bypassing +import scroll.Many.* object BankExample { diff --git a/examples/src/main/scala/scroll/examples/ExpressionProblemExample.scala b/examples/src/main/scala/scroll/examples/ExpressionProblemExample.scala index 8f48530a..8b47573f 100644 --- a/examples/src/main/scala/scroll/examples/ExpressionProblemExample.scala +++ b/examples/src/main/scala/scroll/examples/ExpressionProblemExample.scala @@ -1,6 +1,6 @@ package scroll.examples -import scroll.internal.compartment.impl.Compartment +import scroll.Compartment object ExpressionProblemExample { diff --git a/examples/src/main/scala/scroll/examples/RobotExample.scala b/examples/src/main/scala/scroll/examples/RobotExample.scala index f9cc5f11..a293f7cb 100644 --- a/examples/src/main/scala/scroll/examples/RobotExample.scala +++ b/examples/src/main/scala/scroll/examples/RobotExample.scala @@ -4,7 +4,7 @@ import scroll.examples.RobotExample.ActorView.DriveableRole import scroll.examples.RobotExample.BehavioralView.ServiceRole import scroll.examples.RobotExample.NavigationView.NavigationRole import scroll.examples.RobotExample.SensorView.ObservingEnvironmentRole -import scroll.internal.compartment.impl.Compartment +import scroll.Compartment object RobotExample { diff --git a/examples/src/main/scala/scroll/examples/UniversityExample.scala b/examples/src/main/scala/scroll/examples/UniversityExample.scala index 938a5127..fb4bef85 100644 --- a/examples/src/main/scala/scroll/examples/UniversityExample.scala +++ b/examples/src/main/scala/scroll/examples/UniversityExample.scala @@ -1,6 +1,6 @@ package scroll.examples -import scroll.internal.compartment.impl.Compartment +import scroll.Compartment object UniversityExample { From a90f460bd702bd8e9f4f61a81e405bd12d9fd8c3 Mon Sep 17 00:00:00 2001 From: Max Leuthaeuser <1417198+max-leuthaeuser@users.noreply.github.com> Date: Sun, 10 May 2026 14:29:47 +0200 Subject: [PATCH 6/8] Refresh README and Scaladoc Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 3 +++ .../src/main/scala/scroll/DispatchQuery.scala | 2 ++ core/src/main/scala/scroll/Many.scala | 2 ++ .../scroll/internal/errors/SCROLLErrors.scala | 24 +++++++++++++++++++ .../internal/support/RoleConstraintsApi.scala | 2 +- .../internal/support/RoleGroupsApi.scala | 2 +- .../support/RoleRestrictionsApi.scala | 4 ++-- core/src/main/scala/scroll/package.scala | 9 +++++++ 8 files changed, 44 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index d067db8e..4e99214c 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,9 @@ new Compartment { } ``` +For user-facing imports, prefer the public `scroll` facade such as `scroll.Compartment`, `scroll.DispatchQuery`, and +`scroll.Many.*` instead of importing from `scroll.internal.*`. + A more elaborated example can be found [here](https://github.com/max-leuthaeuser/SCROLL/wiki/The-Bank-Example-%28Overview%29) and [here](https://github.com/max-leuthaeuser/SCROLL/wiki/The-Bank-Example-%28Advanced-Role-features%29). You can find even more examples [here](https://github.com/max-leuthaeuser/SCROLL/tree/master/examples/src/main/scala/scroll/examples). diff --git a/core/src/main/scala/scroll/DispatchQuery.scala b/core/src/main/scala/scroll/DispatchQuery.scala index 81e357c3..78515692 100644 --- a/core/src/main/scala/scroll/DispatchQuery.scala +++ b/core/src/main/scala/scroll/DispatchQuery.scala @@ -1,5 +1,7 @@ package scroll +/** Public entry point for dispatch-query builders and helpers such as [[Bypassing]], [[From]], and [[Through]]. + */ object DispatchQuery { export internal.dispatch.DispatchQuery.* } diff --git a/core/src/main/scala/scroll/Many.scala b/core/src/main/scala/scroll/Many.scala index c12f70e9..1ad66cc7 100644 --- a/core/src/main/scala/scroll/Many.scala +++ b/core/src/main/scala/scroll/Many.scala @@ -1,5 +1,7 @@ package scroll +/** Public entry point for the unbounded role-group cardinality marker `*`. + */ object Many { export internal.util.Many.* } diff --git a/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala b/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala index 6cf83e94..6f273fb4 100644 --- a/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala +++ b/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala @@ -8,12 +8,16 @@ object SCROLLErrors { sealed trait TypeError extends SCROLLError + /** Base type for validation failures raised by constraints, restrictions, and role groups. + */ sealed trait ValidationError extends RuntimeException { override def getMessage: String = toString } sealed trait ConstraintError extends ValidationError + /** Raised when a role implication constraint requires another role that is currently missing. + */ final case class RoleImplicationConstraintViolation(player: AnyRef, requiredRole: String) extends ConstraintError { override def toString: String = @@ -21,6 +25,8 @@ object SCROLLErrors { } + /** Raised when an equivalence constraint requires another role that is currently missing. + */ final case class RoleEquivalenceConstraintViolation(player: AnyRef, requiredRole: String) extends ConstraintError { override def toString: String = @@ -28,6 +34,8 @@ object SCROLLErrors { } + /** Raised when a player holds a role that is forbidden by a prohibition constraint. + */ final case class RoleProhibitionConstraintViolation(player: AnyRef, prohibitedRole: String) extends ConstraintError { override def toString: String = @@ -35,6 +43,8 @@ object SCROLLErrors { } + /** Raised when a player attempts to play a role that is blocked by active role restrictions. + */ final case class RoleRestrictionViolation(player: AnyRef, role: AnyRef) extends ValidationError { override def toString: String = @@ -44,6 +54,8 @@ object SCROLLErrors { sealed trait RoleGroupError extends ValidationError + /** Raised when the occurrence cardinality of a role group is outside the configured bounds. + */ final case class RoleGroupOccurrenceCardinalityViolation( groupName: String, roleTypes: Seq[String], @@ -58,26 +70,38 @@ object SCROLLErrors { } + /** Raised when solving a role-group constraint references an unknown role type variable. + */ final case class MissingRoleGroupConstraint(groupName: String, roleType: String) extends RoleGroupError { override def toString: String = s"Constraints for role group '$groupName' do not contain '$roleType'!" } + /** Raised when a role-group constraint model cannot be solved at all. + */ final case class UnsolvableRoleGroupConstraint(groupName: String) extends RoleGroupError { override def toString: String = s"Constraint set of role group '$groupName' unsolvable!" } + /** Raised when the inner cardinality of a role group is violated by the current role assignments. + */ final case class RoleGroupInnerCardinalityViolation(groupName: String) extends RoleGroupError { override def toString: String = s"Constraint set for inner cardinality of role group '$groupName' violated!" } + /** Raised when a role-group definition contains unsupported entries. + */ final case class InvalidRoleGroupEntry() extends RoleGroupError { override def toString: String = "Role groups can only contain a list of types or role groups itself!" } + /** Raised when a role-group cardinality definition is not representable by the supported constraint kinds. + */ final case class InvalidRoleGroupConstraint(groupName: String, min: Int, max: String) extends RoleGroupError { override def toString: String = s"Role group constraint of ($min, $max) for role group '$groupName' not possible!" } + /** Raised when a role group with the same name is registered more than once. + */ final case class DuplicateRoleGroup(groupName: String) extends RoleGroupError { override def toString: String = s"The RoleGroup $groupName was already added!" } diff --git a/core/src/main/scala/scroll/internal/support/RoleConstraintsApi.scala b/core/src/main/scala/scroll/internal/support/RoleConstraintsApi.scala index 27345779..7d3f3c86 100644 --- a/core/src/main/scala/scroll/internal/support/RoleConstraintsApi.scala +++ b/core/src/main/scala/scroll/internal/support/RoleConstraintsApi.scala @@ -37,7 +37,7 @@ trait RoleConstraintsApi { def addRoleProhibition[A <: AnyRef: ClassTag, B <: AnyRef: ClassTag](): Unit /** Wrapping function that checks all available role constraints for all core objects and its roles after the given - * function was executed. Throws a RuntimeException if a role constraint is violated! + * function was executed. Throws a typed constraint validation error if a role constraint is violated. * * @param func * the function to execute and check role constraints afterwards diff --git a/core/src/main/scala/scroll/internal/support/RoleGroupsApi.scala b/core/src/main/scala/scroll/internal/support/RoleGroupsApi.scala index 3b01cbd1..a6938f84 100644 --- a/core/src/main/scala/scroll/internal/support/RoleGroupsApi.scala +++ b/core/src/main/scala/scroll/internal/support/RoleGroupsApi.scala @@ -37,7 +37,7 @@ trait RoleGroupsApi { import RoleGroupsApi._ /** Wrapping function that checks all available role group constraints for all core objects and its roles after the - * given function was executed. Throws a RuntimeException if a role group constraint is violated! + * given function was executed. Throws a typed role-group validation error if a role group constraint is violated. * * @param func * the function to execute and check role group constraints afterwards diff --git a/core/src/main/scala/scroll/internal/support/RoleRestrictionsApi.scala b/core/src/main/scala/scroll/internal/support/RoleRestrictionsApi.scala index bfbb20d4..40bfd40b 100644 --- a/core/src/main/scala/scroll/internal/support/RoleRestrictionsApi.scala +++ b/core/src/main/scala/scroll/internal/support/RoleRestrictionsApi.scala @@ -32,8 +32,8 @@ trait RoleRestrictionsApi { */ def removeRoleRestriction[A <: AnyRef: ClassTag](): Unit - /** Checks all role restriction between the given player and a role type. Will throw a RuntimeException if a - * restriction is violated! + /** Checks all role restriction between the given player and a role type. Throws a typed restriction validation error + * if a restriction is violated. * * @param player * the player instance to check diff --git a/core/src/main/scala/scroll/package.scala b/core/src/main/scala/scroll/package.scala index 9eab1d56..dfaced6b 100644 --- a/core/src/main/scala/scroll/package.scala +++ b/core/src/main/scala/scroll/package.scala @@ -1,11 +1,20 @@ +/** Public SCROLL facade. + * + * Import types and helpers from this package when using the library directly. The implementation continues to live + * under `scroll.internal`, but user code should prefer the stable aliases and re-exports defined here. + */ package object scroll { + /** Public alias for the standard single-dispatch compartment DSL. */ type Compartment = internal.compartment.impl.Compartment + /** Public alias for the multi-dispatch compartment DSL. */ type MultiCompartment = internal.compartment.impl.MultiCompartment + /** Public alias for composed dispatch queries. */ type DispatchQuery = internal.dispatch.DispatchQuery + /** Public alias for the unbounded role-group cardinality marker. */ type Many = internal.util.Many } From 8110b90d890edc4b27da588075da93826e59784d Mon Sep 17 00:00:00 2001 From: Max Leuthaeuser <1417198+max-leuthaeuser@users.noreply.github.com> Date: Sun, 10 May 2026 14:29:47 +0200 Subject: [PATCH 7/8] Complete remaining review cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 4 + .../impl/AbstractCompartment.scala | 3 +- .../scroll/internal/ecore/ECoreImporter.scala | 3 +- .../scroll/internal/errors/SCROLLErrors.scala | 96 ++++++++++++++++++- .../scala/scroll/internal/formal/CROM.scala | 88 +++++++---------- .../scroll/internal/formal/FormalCROI.scala | 32 +++---- .../internal/graph/impl/ScalaRoleGraph.scala | 6 +- .../internal/support/RelationshipsApi.scala | 6 ++ .../internal/support/impl/Relationships.scala | 33 +++---- .../support/impl/RoleConstraints.scala | 5 +- .../internal/support/impl/RoleGroups.scala | 4 +- .../internal/util/ReflectiveHelper.scala | 28 ++++-- core/src/main/scala/scroll/package.scala | 27 ++++++ .../tests/other/ReflectiveHelperTest.scala | 26 +++++ .../CompartmentRoleFeaturesTest.scala | 4 +- .../MultiCompartmentRoleFeaturesTest.scala | 20 +++- .../parameterized/MultiCompartmentTest.scala | 1 + .../parameterized/RelationshipTest.scala | 6 +- .../parameterized/RoleConstraintsTest.scala | 45 +++++---- .../tests/parameterized/RoleGroupsTest.scala | 8 +- .../parameterized/RoleRestrictionsTest.scala | 12 ++- 21 files changed, 326 insertions(+), 131 deletions(-) diff --git a/README.md b/README.md index 4e99214c..f8a163b5 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,10 @@ new Compartment { For user-facing imports, prefer the public `scroll` facade such as `scroll.Compartment`, `scroll.DispatchQuery`, and `scroll.Many.*` instead of importing from `scroll.internal.*`. +When working with `scroll.MultiCompartment`, the public `scroll.MultiDispatchResult` alias and `sequenceResults` +extension help flatten the nested `Either[SCROLLError, Seq[Either[SCROLLError, E]]]` return shape into +`Either[SCROLLError, Seq[E]]` when all dispatched role invocations succeed. + A more elaborated example can be found [here](https://github.com/max-leuthaeuser/SCROLL/wiki/The-Bank-Example-%28Overview%29) and [here](https://github.com/max-leuthaeuser/SCROLL/wiki/The-Bank-Example-%28Advanced-Role-features%29). You can find even more examples [here](https://github.com/max-leuthaeuser/SCROLL/tree/master/examples/src/main/scala/scroll/examples). diff --git a/core/src/main/scala/scroll/internal/compartment/impl/AbstractCompartment.scala b/core/src/main/scala/scroll/internal/compartment/impl/AbstractCompartment.scala index 0e65ebfe..166963df 100644 --- a/core/src/main/scala/scroll/internal/compartment/impl/AbstractCompartment.scala +++ b/core/src/main/scala/scroll/internal/compartment/impl/AbstractCompartment.scala @@ -3,6 +3,7 @@ package scroll.internal.compartment.impl import scroll.internal.compartment.CompartmentApi import scroll.internal.dispatch.DispatchQuery import scroll.internal.dispatch.impl.SCROLLDispatchable +import scroll.internal.errors.SCROLLErrors.InvalidRolePlayer import scroll.internal.errors.SCROLLErrors.SCROLLError import scroll.internal.errors.SCROLLErrors.TypeError import scroll.internal.errors.SCROLLErrors.TypeNotFound @@ -105,7 +106,7 @@ abstract class AbstractCompartment() extends CompartmentApi { case p: IPlayer[?, ?] => rolePlaying.addPlaysRelation[W, R](p.wrapped.asInstanceOf[W], role) case p: AnyRef => rolePlaying.addPlaysRelation[W, R](p.asInstanceOf[W], role) case null => - throw new RuntimeException(s"Only instances of 'IPlayer' or 'AnyRef' are allowed to play roles!") + throw InvalidRolePlayer() } this } diff --git a/core/src/main/scala/scroll/internal/ecore/ECoreImporter.scala b/core/src/main/scala/scroll/internal/ecore/ECoreImporter.scala index 09f1c4d0..62cdfac2 100644 --- a/core/src/main/scala/scroll/internal/ecore/ECoreImporter.scala +++ b/core/src/main/scala/scroll/internal/ecore/ECoreImporter.scala @@ -8,6 +8,7 @@ import org.eclipse.emf.ecore.util.BasicExtendedMetaData import org.eclipse.emf.ecore.xmi.XMLResource import org.eclipse.emf.ecore.xmi.impl.EcoreResourceFactoryImpl import org.eclipse.emf.ecore.xmi.impl.XMIResourceFactoryImpl +import scroll.internal.errors.SCROLLErrors.CROMMetaModelLoadFailure /** Trait providing functionality for importing ecore models. */ @@ -25,7 +26,7 @@ trait ECoreImporter { val eObject = r.getContents.get(0) eObject match { case p: EPackage => val _ = rs.getPackageRegistry.put(p.getNsURI, p) - case _ => throw new IllegalStateException("Meta-Model for CROM could not be loaded!") + case _ => throw CROMMetaModelLoadFailure() } } diff --git a/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala b/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala index 6f273fb4..4a13b8d0 100644 --- a/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala +++ b/core/src/main/scala/scroll/internal/errors/SCROLLErrors.scala @@ -4,7 +4,9 @@ package scroll.internal.errors */ object SCROLLErrors { - sealed trait SCROLLError extends RuntimeException + sealed trait SCROLLError extends RuntimeException { + override def getMessage: String = toString + } sealed trait TypeError extends SCROLLError @@ -54,6 +56,18 @@ object SCROLLErrors { sealed trait RoleGroupError extends ValidationError + sealed trait RelationshipError extends ValidationError + + sealed trait ReflectionError extends SCROLLError + + sealed trait FormalModelError extends RuntimeException { + override def getMessage: String = toString + } + + sealed trait GraphError extends SCROLLError + + sealed trait PlayerError extends SCROLLError + /** Raised when the occurrence cardinality of a role group is outside the configured bounds. */ final case class RoleGroupOccurrenceCardinalityViolation( @@ -106,6 +120,86 @@ object SCROLLErrors { override def toString: String = s"The RoleGroup $groupName was already added!" } + /** Raised when a relationship expects at least one match but none can be found. + */ + final case class EmptyRelationshipMultiplicityViolation(relationshipName: String) extends RelationshipError { + + override def toString: String = + s"With left multiplicity for '$relationshipName' of '*', the resulting role set should not be empty!" + + } + + /** Raised when a relationship expects an exact number of matches but gets a different size. + */ + final case class ConcreteRelationshipMultiplicityViolation(relationshipName: String, expectedSize: Ordered[Int]) + extends RelationshipError { + + override def toString: String = + s"With a concrete multiplicity for '$relationshipName' of '$expectedSize' the resulting role set should have the same size!" + + } + + /** Raised when a relationship expects a bounded range of matches but gets a size outside that range. + */ + final case class RangeRelationshipMultiplicityViolation( + relationshipName: String, + lowerBound: Ordered[Int], + upperBound: String + ) extends RelationshipError { + + override def toString: String = + s"With a multiplicity for '$relationshipName' from '$lowerBound' to '$upperBound', the resulting role set size should be in between!" + + } + + /** Raised when a relationship uses a multiplicity shape that is not supported by the runtime. + */ + final case class UnsupportedRelationshipMultiplicity() extends RelationshipError { + override def toString: String = "This multiplicity is not allowed!" + } + + /** Raised when reflective field lookup fails. + */ + final case class ReflectiveFieldNotFound(owner: Class[?], fieldName: String) extends ReflectionError { + override def toString: String = s"Field '$fieldName' not found on '$owner'!" + } + + /** Raised when reflective method lookup by name fails. + */ + final case class ReflectiveMethodNotFound(target: AnyRef, methodName: String) extends ReflectionError { + override def toString: String = s"Function with name '$methodName' not found on '$target'!" + } + + /** Raised when a role graph operation would introduce a cycle. + */ + final case class CyclicRolePlayingRelationshipFound() extends GraphError { + override def toString: String = "Cyclic role-playing relationship found!" + } + + /** Raised when a specific player would become part of a cyclic role graph. + */ + final case class CyclicRolePlayingRelationshipForPlayer(player: AnyRef) extends GraphError { + override def toString: String = s"Cyclic role-playing relationship for player '$player' found!" + } + + /** Raised when the CROM meta-model cannot be loaded. + */ + final case class CROMMetaModelLoadFailure() extends FormalModelError { + override def toString: String = "Meta-Model for CROM could not be loaded!" + } + + /** Raised when a CROI relationship references a role without a corresponding plays entry. + */ + final case class RoleNotPlayedInCROI(role: AnyRef) extends FormalModelError { + override def toString: String = s"The given role '$role' is not played in the CROI!" + } + + /** Raised when something other than an object/player tries to participate in role playing. + */ + final case class InvalidRolePlayer() extends PlayerError { + override def toString: String = "Only instances of 'IPlayer' or 'AnyRef' are allowed to play roles!" + } + final case class TypeNotFound(tpe: Class[?]) extends TypeError { override def toString: String = s"Type '$tpe' could not be found!" } diff --git a/core/src/main/scala/scroll/internal/formal/CROM.scala b/core/src/main/scala/scroll/internal/formal/CROM.scala index b6d66721..5f225f0c 100644 --- a/core/src/main/scala/scroll/internal/formal/CROM.scala +++ b/core/src/main/scala/scroll/internal/formal/CROM.scala @@ -39,26 +39,22 @@ trait CROM extends ECoreImporter { .map(of.eGet(_).toString) .getOrElse("-") - private def constructNT[NT >: Null <: AnyRef](elem: EObject): NT = - instanceName(elem).asInstanceOf[NT] - - private def constructRT[RT >: Null <: AnyRef](elem: EObject): RT = - instanceName(elem).asInstanceOf[RT] - - private def constructCT[CT >: Null <: AnyRef](elem: EObject): CT = - instanceName(elem).asInstanceOf[CT] - - private def constructRST[RST >: Null <: AnyRef](elem: EObject): RST = - instanceName(elem).asInstanceOf[RST] - - private def constructFills[NT >: Null <: AnyRef, RT >: Null <: AnyRef](elem: EObject): List[(NT, RT)] = { - val obj = elem.asInstanceOf[DynamicEObjectImpl] - val filler = obj.dynamicGet(1).asInstanceOf[DynamicEObjectImpl].dynamicGet(0).asInstanceOf[NT] - val filledObj = obj.dynamicGet(0).asInstanceOf[DynamicEObjectImpl] + private def dynamic(obj: EObject): DynamicEObjectImpl = + obj.asInstanceOf[DynamicEObjectImpl] + + private def dynamicRoleList(obj: DynamicEObjectImpl, index: Int): List[DynamicEObjectImpl] = + Option(obj.dynamicGet(index).asInstanceOf[EcoreEList[DynamicEObjectImpl]]) + .map(_.asScala.toList) + .getOrElse(List.empty) + + private def constructFills(elem: EObject): List[(String, String)] = { + val obj = dynamic(elem) + val filler = instanceName(dynamic(obj.dynamicGet(1).asInstanceOf[DynamicEObjectImpl])) + val filledObj = dynamic(obj.dynamicGet(0).asInstanceOf[DynamicEObjectImpl]) if (filledObj.eClass().getName == ROLEGROUP) { - collectRoles(filledObj).map(r => (filler, instanceName(r).asInstanceOf[RT])) + collectRoles(filledObj).map(r => (filler, instanceName(r))) } else { - val filled = obj.dynamicGet(0).asInstanceOf[DynamicEObjectImpl].dynamicGet(0).asInstanceOf[RT] + val filled = instanceName(dynamic(obj.dynamicGet(0).asInstanceOf[DynamicEObjectImpl])) List((filler, filled)) } } @@ -76,39 +72,24 @@ trait CROM extends ECoreImporter { } ) - private def constructParts[CT >: Null <: AnyRef, RT >: Null <: AnyRef](elem: EObject): (CT, List[RT]) = { - val ct = instanceName(elem.eContainer()).asInstanceOf[CT] - val roles = collectRoles(elem).map(r => instanceName(r).asInstanceOf[RT]) + private def constructParts(elem: EObject): (String, List[String]) = { + val ct = instanceName(elem.eContainer()) + val roles = collectRoles(elem).map(instanceName) (ct, roles) } - private def constructRel[RST >: Null <: AnyRef, RT >: Null <: AnyRef](elem: EObject): (RST, List[RT]) = { + private def constructRel(elem: EObject): (String, List[String]) = { val rstName = instanceName(elem) val roles = collectRoles(elem.eContainer()) val rsts = roles .filter { role => - val incoming = role - .asInstanceOf[DynamicEObjectImpl] - .dynamicGet(1) - .asInstanceOf[EcoreEList[DynamicEObjectImpl]] - .asScala - val inCond = incoming match { - case null => false - case _ => incoming.exists(e => e.dynamicGet(0).asInstanceOf[String] == rstName) - } - val outgoing = role - .asInstanceOf[DynamicEObjectImpl] - .dynamicGet(2) - .asInstanceOf[EcoreEList[DynamicEObjectImpl]] - .asScala - val outCond = outgoing match { - case null => false - case _ => outgoing.exists(e => e.dynamicGet(0).asInstanceOf[String] == rstName) - } + val dynamicRole = dynamic(role) + val inCond = dynamicRoleList(dynamicRole, 1).exists(_.dynamicGet(0).asInstanceOf[String] == rstName) + val outCond = dynamicRoleList(dynamicRole, 2).exists(_.dynamicGet(0).asInstanceOf[String] == rstName) inCond || outCond } - .map(instanceName(_).asInstanceOf[RT]) - (rstName.asInstanceOf[RST], rsts) + .map(instanceName) + (rstName, rsts) } private def addToMap(m: mutable.Map[String, List[String]], elem: (String, List[String])): Unit = { @@ -117,9 +98,7 @@ trait CROM extends ECoreImporter { m.update(key, m.getOrElseUpdate(key, value) ++ value) } - protected def construct[NT >: Null <: AnyRef, RT >: Null <: AnyRef, CT >: Null <: AnyRef, RST >: Null <: AnyRef]( - path: String - ): FormalCROM[NT, RT, CT, RST] = { + private def constructStrings(path: String): FormalCROM[String, String, String, String] = { val nt = mutable.ListBuffer[String]() val rt = mutable.ListBuffer[String]() val ct = mutable.ListBuffer[String]() @@ -132,19 +111,24 @@ trait CROM extends ECoreImporter { .filter(e => validTypes.contains(e.eClass().getName)) .foreach { curr => curr.eClass().getName match { - case NATURALTYPE => nt += constructNT(curr) - case ROLETYPE => rt += constructRT(curr) - case COMPARTMENTTYPE => ct += constructCT(curr) + case NATURALTYPE => nt += instanceName(curr) + case ROLETYPE => rt += instanceName(curr) + case COMPARTMENTTYPE => ct += instanceName(curr) case RELATIONSHIP => - val _ = rst += constructRST[String](curr) - addToMap(rel, constructRel[String, String](curr)) + rst += instanceName(curr) + addToMap(rel, constructRel(curr)) case FULFILLMENT => fills ++= constructFills(curr) - case PART => addToMap(parts, constructParts[String, String](curr)) + case PART => addToMap(parts, constructParts(curr)) case _ => } } + FormalCROM(nt.result(), rt.result(), ct.result(), rst.result(), fills.result(), parts.toMap, rel.toMap) - .asInstanceOf[FormalCROM[NT, RT, CT, RST]] } + protected def construct[NT >: Null <: AnyRef, RT >: Null <: AnyRef, CT >: Null <: AnyRef, RST >: Null <: AnyRef]( + path: String + ): FormalCROM[NT, RT, CT, RST] = + constructStrings(path).asInstanceOf[FormalCROM[NT, RT, CT, RST]] + } diff --git a/core/src/main/scala/scroll/internal/formal/FormalCROI.scala b/core/src/main/scala/scroll/internal/formal/FormalCROI.scala index 7ce35d9c..d9f554f2 100644 --- a/core/src/main/scala/scroll/internal/formal/FormalCROI.scala +++ b/core/src/main/scala/scroll/internal/formal/FormalCROI.scala @@ -1,5 +1,7 @@ package scroll.internal.formal +import scroll.internal.errors.SCROLLErrors.RoleNotPlayedInCROI + /** Companion object for the formal representation of the Compartment Role Object Instance (CROI). */ object FormalCROI { @@ -117,36 +119,30 @@ final case class FormalCROI[NT >: Null <: AnyRef, RT >: Null <: AnyRef, CT >: Nu private def o: List[AnyRef] = n.concat(c) + private def linksFor(rst: RST, c: CT): List[(RT, RT)] = links.getOrElse((rst, c), List.empty) + def o_c(c: CT): List[NT] = plays.filter(_._2 == c).map(_._1) private def repsilon: List[RT] = r :+ null def pred(rst: RST, c: CT, r: RT): List[RT] = - if (links.contains((rst, c))) { - links((rst, c)).filter(_._2 == r).map(_._1) - } else { - List.empty[RT] - } + linksFor(rst, c).filter(_._2 == r).map(_._1) def succ(rst: RST, c: CT, r: RT): List[RT] = - if (links.contains((rst, c))) { - links((rst, c)).filter(_._1 == r).map(_._2) - } else { - List.empty[RT] - } + linksFor(rst, c).filter(_._1 == r).map(_._2) + + private def playerOption(r: RT): Option[NT] = + Option(r).flatMap(role => plays.find(_._3 == role).map(_._1)) private def player(r: RT): NT = - r match { - case null => null - case _ => - plays.find(_._3 == r) match { - case Some(p) => p._1 - case _ => throw new RuntimeException(s"The given role '$r' is not played in the CROI!") - } + if (r == null) { + null.asInstanceOf[NT] + } else { + playerOption(r).getOrElse(throw RoleNotPlayedInCROI(r)) } def overline_links(rst: RST, c: CT): List[(NT, NT)] = - links((rst, c)).map { case (r_1, r_2) => + linksFor(rst, c).map { case (r_1, r_2) => (player(r_1), player(r_2)) } diff --git a/core/src/main/scala/scroll/internal/graph/impl/ScalaRoleGraph.scala b/core/src/main/scala/scroll/internal/graph/impl/ScalaRoleGraph.scala index ef1c1765..e632cb31 100644 --- a/core/src/main/scala/scroll/internal/graph/impl/ScalaRoleGraph.scala +++ b/core/src/main/scala/scroll/internal/graph/impl/ScalaRoleGraph.scala @@ -5,6 +5,8 @@ import com.google.common.graph.GraphBuilder import com.google.common.graph.Graphs import com.google.common.graph.MutableGraph import scroll.internal.compartment.impl.AbstractCompartment +import scroll.internal.errors.SCROLLErrors.CyclicRolePlayingRelationshipFound +import scroll.internal.errors.SCROLLErrors.CyclicRolePlayingRelationshipForPlayer import scroll.internal.graph.RoleGraph import scala.annotation.tailrec @@ -61,7 +63,7 @@ class ScalaRoleGraph( private def checkCycles(): Unit = if (checkForCycles && Graphs.hasCycle(root)) { - throw new RuntimeException("Cyclic role-playing relationship found!") + throw CyclicRolePlayingRelationshipFound() } override def addBinding(player: AnyRef, role: AnyRef): Unit = { @@ -69,7 +71,7 @@ class ScalaRoleGraph( require(null != role) val _ = root.putEdge(player, role) if (checkForCycles && Graphs.hasCycle(root)) { - throw new RuntimeException(s"Cyclic role-playing relationship for player '$player' found!") + throw CyclicRolePlayingRelationshipForPlayer(player) } } diff --git a/core/src/main/scala/scroll/internal/support/RelationshipsApi.scala b/core/src/main/scala/scroll/internal/support/RelationshipsApi.scala index f0bae2c0..fbae5027 100644 --- a/core/src/main/scala/scroll/internal/support/RelationshipsApi.scala +++ b/core/src/main/scala/scroll/internal/support/RelationshipsApi.scala @@ -37,6 +37,9 @@ trait RelationshipsApi { /** Get all instances of the left side of the relationship w.r.t. the provided matching function and checking the * multiplicity. * + * Implementations raise typed [[scroll.internal.errors.SCROLLErrors.RelationshipError]] instances when the + * configured multiplicity is violated or not supported. + * * @param matcher * a matching function to select the appropriate instances * @return @@ -47,6 +50,9 @@ trait RelationshipsApi { /** Get all instances of the right side of the relationship w.r.t. the provided matching function and checking the * multiplicity. * + * Implementations raise typed [[scroll.internal.errors.SCROLLErrors.RelationshipError]] instances when the + * configured multiplicity is violated or not supported. + * * @param matcher * a matching function to select the appropriate instances * @return diff --git a/core/src/main/scala/scroll/internal/support/impl/Relationships.scala b/core/src/main/scala/scroll/internal/support/impl/Relationships.scala index c781816c..23b0a9b7 100644 --- a/core/src/main/scala/scroll/internal/support/impl/Relationships.scala +++ b/core/src/main/scala/scroll/internal/support/impl/Relationships.scala @@ -1,5 +1,9 @@ package scroll.internal.support.impl +import scroll.internal.errors.SCROLLErrors.ConcreteRelationshipMultiplicityViolation +import scroll.internal.errors.SCROLLErrors.EmptyRelationshipMultiplicityViolation +import scroll.internal.errors.SCROLLErrors.RangeRelationshipMultiplicityViolation +import scroll.internal.errors.SCROLLErrors.UnsupportedRelationshipMultiplicity import scroll.internal.support.RelationshipsApi import scroll.internal.support.RoleQueriesApi @@ -51,31 +55,28 @@ class Relationships(private val roleQueries: RoleQueriesApi) extends Relationshi rightMul: Multiplicity ) extends RelationshipApi[L, R] { - protected val MULT_NOT_ALLOWED: String = "This multiplicity is not allowed!" - private def checkMul[T](m: Multiplicity, on: Seq[T]): Seq[T] = { m match { case MMany() => - assert(on.nonEmpty, s"With left multiplicity for '$name' of '*', the resulting role set should not be empty!") + if (on.isEmpty) { + throw EmptyRelationshipMultiplicityViolation(name) + } case ConcreteValue(v) => - assert( - v.compare(on.size) == 0, - s"With a concrete multiplicity for '$name' of '$v' the resulting role set should have the same size!" - ) + if (v.compare(on.size) != 0) { + throw ConcreteRelationshipMultiplicityViolation(name, v) + } case RangeMultiplicity(f, t) => (f, t) match { case (ConcreteValue(v1), ConcreteValue(v2)) => - assert( - v1 <= on.size && v2 >= on.size, - s"With a multiplicity for '$name' from '$v1' to '$v2', the resulting role set size should be in between!" - ) + if (!(v1 <= on.size && v2 >= on.size)) { + throw RangeRelationshipMultiplicityViolation(name, v1, v2.toString) + } case (ConcreteValue(v), MMany()) => - assert( - v <= on.size, - s"With a multiplicity for '$name' from '$v' to '*', the resulting role set size should be in between!" - ) + if (!(v <= on.size)) { + throw RangeRelationshipMultiplicityViolation(name, v, "*") + } case _ => - throw new RuntimeException(MULT_NOT_ALLOWED) // default case + throw UnsupportedRelationshipMultiplicity() } } on diff --git a/core/src/main/scala/scroll/internal/support/impl/RoleConstraints.scala b/core/src/main/scala/scroll/internal/support/impl/RoleConstraints.scala index 2ffd15c2..748fbc09 100644 --- a/core/src/main/scala/scroll/internal/support/impl/RoleConstraints.scala +++ b/core/src/main/scala/scroll/internal/support/impl/RoleConstraints.scala @@ -96,8 +96,9 @@ class RoleConstraints(private val roleGraph: RoleGraphProxyApi) extends RoleCons roleGraph.plays.allPlayers.foreach(p => roleGraph.plays.roles(p).foreach(r => validateConstraints(p, r))) } - /** Checks all role constraints between the given player and role instance. Will throw a RuntimeException if a - * constraint is violated! + /** Checks all role constraints between the given player and role instance. + * + * Raises typed constraint violations when a configured implication, equivalence, or prohibition is broken. * * @param player * the player instance to check diff --git a/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala b/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala index 7c1cccf8..8497c233 100644 --- a/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala +++ b/core/src/main/scala/scroll/internal/support/impl/RoleGroups.scala @@ -149,7 +149,9 @@ class RoleGroups(private val roleGraph: RoleGraphProxyApi) extends RoleGroupsApi groups.foreach(eval(_, evaluatedGroups)) } - /** Checks all role groups. Will throw a RuntimeException if a role group constraint is violated! + /** Checks all role groups. + * + * Raises typed role-group validation errors when an occurrence or inner-cardinality constraint is violated. */ private def validate(groups: Seq[RoleGroup]): Unit = { validateOccurrenceCardinality(groups) diff --git a/core/src/main/scala/scroll/internal/util/ReflectiveHelper.scala b/core/src/main/scala/scroll/internal/util/ReflectiveHelper.scala index dd27ff47..49578906 100644 --- a/core/src/main/scala/scroll/internal/util/ReflectiveHelper.scala +++ b/core/src/main/scala/scroll/internal/util/ReflectiveHelper.scala @@ -1,5 +1,9 @@ package scroll.internal.util +import com.google.common.util.concurrent.UncheckedExecutionException +import scroll.internal.errors.SCROLLErrors.ReflectiveFieldNotFound +import scroll.internal.errors.SCROLLErrors.ReflectiveMethodNotFound + import java.lang.reflect.Field import java.lang.reflect.Method import scala.collection.immutable.ArraySeq @@ -64,6 +68,14 @@ object ReflectiveHelper { private def cachedSimpleName(t: String): String = simpleClassName(simpleClassName(t, "."), "$") + private def getCachedValue[T](load: => T): T = + try + load + catch { + case err: UncheckedExecutionException if err.getCause.isInstanceOf[RuntimeException] => + throw err.getCause.asInstanceOf[RuntimeException] + } + /** Translates a Class or Type name to a String, i.e. removing anything before the last occurrence of "$" * or ".". * @@ -72,7 +84,7 @@ object ReflectiveHelper { * @return * anything after the last occurrence of "$" or "." */ - def simpleName(t: String): String = classNameCache.get(t) + def simpleName(t: String): String = getCachedValue(classNameCache.get(t)) /** Compares two class names. * @@ -114,16 +126,17 @@ object ReflectiveHelper { .get(of) .find(_.getName == name) .getOrElse { - throw new RuntimeException(s"Field '$name' not found on '$of'!") + throw ReflectiveFieldNotFound(of, name) } - private def findField(of: Class[?], name: String): Field = fieldByNameCache.get((of, name)) + private def findField(of: Class[?], name: String): Field = + getCachedValue(fieldByNameCache.get((of, name))) private def cachedFindMethods(of: Class[?], name: String): Seq[Method] = methodCache.get(of).filter(_.getName == name) private def findMethods(of: Class[?], name: String): Seq[Method] = - methodsByNameCache.get((of, name)) + getCachedValue(methodsByNameCache.get((of, name))) private def allMethods(of: Class[?]): Seq[Method] = { def getAccessibleMethods(c: Class[?]): Seq[Method] = @@ -253,7 +266,7 @@ object ReflectiveHelper { * Some(Method) if the wrapped object provides the function/method in question, None otherwise */ def findMethod(on: AnyRef, name: String, args: Seq[Any]): Option[Method] = - methodMatchCache.get((on.getClass, name, args)) + getCachedValue(methodMatchCache.get((on.getClass, name, args))) private def cachedHasMember(on: Class[?], name: String): java.lang.Boolean = { lazy val fields = fieldCache.get(on) @@ -270,7 +283,8 @@ object ReflectiveHelper { * @return * true if the wrapped object provides the given member, false otherwise */ - def hasMember(on: AnyRef, name: String): Boolean = hasMemberCache.get((on.getClass, name)) + def hasMember(on: AnyRef, name: String): Boolean = + getCachedValue(hasMemberCache.get((on.getClass, name))) /** Returns the runtime content of type T of the field with the given name of the wrapped object. * @@ -333,7 +347,7 @@ object ReflectiveHelper { case elem +: _ => elem.invoke(on).asInstanceOf[T] case Nil => - throw new RuntimeException(s"Function with name '$name' not found on '$on'!") + throw ReflectiveMethodNotFound(on, name) } /** Checks if the wrapped object is of type T. diff --git a/core/src/main/scala/scroll/package.scala b/core/src/main/scala/scroll/package.scala index dfaced6b..e16a1258 100644 --- a/core/src/main/scala/scroll/package.scala +++ b/core/src/main/scala/scroll/package.scala @@ -5,6 +5,16 @@ */ package object scroll { + /** Public alias for core SCROLL dispatch and lookup errors. */ + type SCROLLError = internal.errors.SCROLLErrors.SCROLLError + + /** Public alias for raw multi-dispatch return values. + * + * A multi-dispatch call can fail before dispatch starts or per dispatched role invocation, so the result keeps both + * error layers intact. + */ + type MultiDispatchResult[E] = Either[SCROLLError, Seq[Either[SCROLLError, E]]] + /** Public alias for the standard single-dispatch compartment DSL. */ type Compartment = internal.compartment.impl.Compartment @@ -17,4 +27,21 @@ package object scroll { /** Public alias for the unbounded role-group cardinality marker. */ type Many = internal.util.Many + /** Convenience helpers for consuming multi-dispatch results without changing the underlying dynamic API. + */ + extension [E](result: MultiDispatchResult[E]) + + /** Sequence all per-role results into a single `Either`. + * + * Returns the first encountered [[SCROLLError]] or the collected successful values if all dispatched role + * invocations succeed. + */ + def sequenceResults: Either[SCROLLError, Seq[E]] = + result.flatMap(_.foldRight(Right(Seq.empty[E]): Either[SCROLLError, Seq[E]]) { (entry, acc) => + for { + value <- entry + values <- acc + } yield value +: values + }) + } diff --git a/tests/src/test/scala/scroll/tests/other/ReflectiveHelperTest.scala b/tests/src/test/scala/scroll/tests/other/ReflectiveHelperTest.scala index 35ba9ded..116423c5 100644 --- a/tests/src/test/scala/scroll/tests/other/ReflectiveHelperTest.scala +++ b/tests/src/test/scala/scroll/tests/other/ReflectiveHelperTest.scala @@ -1,5 +1,7 @@ package scroll.tests.other +import scroll.internal.errors.SCROLLErrors.ReflectiveFieldNotFound +import scroll.internal.errors.SCROLLErrors.ReflectiveMethodNotFound import scroll.internal.util.ReflectiveHelper import scroll.tests.AbstractSCROLLTest @@ -40,4 +42,28 @@ class ReflectiveHelperTest extends AbstractSCROLLTest { ReflectiveHelper.resultOf[Int](target, method.get, Seq(1)) shouldBe 2 } + test("propertyOf reports typed missing-field errors") { + class Target { + val value: String = "ok" + } + + val err = intercept[ReflectiveFieldNotFound] { + ReflectiveHelper.propertyOf[String](new Target(), "missing") + } + + err.fieldName shouldBe "missing" + } + + test("resultOf reports typed missing-method errors") { + class Target { + def value: String = "ok" + } + + val err = intercept[ReflectiveMethodNotFound] { + ReflectiveHelper.resultOf[String](new Target(), "missing") + } + + err.methodName shouldBe "missing" + } + } diff --git a/tests/src/test/scala/scroll/tests/parameterized/CompartmentRoleFeaturesTest.scala b/tests/src/test/scala/scroll/tests/parameterized/CompartmentRoleFeaturesTest.scala index dd8b9729..6523100f 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/CompartmentRoleFeaturesTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/CompartmentRoleFeaturesTest.scala @@ -2,6 +2,7 @@ package scroll.tests.parameterized import scroll.internal.dispatch.DispatchQuery import scroll.internal.dispatch.DispatchQuery._ +import scroll.internal.errors.SCROLLErrors.CyclicRolePlayingRelationshipForPlayer import scroll.internal.errors.SCROLLErrors.RoleNotFound import scroll.internal.errors.SCROLLErrors.TypeNotFound import scroll.tests.mocks._ @@ -410,9 +411,10 @@ class CompartmentRoleFeaturesTest extends AbstractParameterizedSCROLLTest { someRoleA play someRoleB someRoleB play someRoleC - a[RuntimeException] should be thrownBy { + val err = the[CyclicRolePlayingRelationshipForPlayer] thrownBy { someRoleC play someRoleA } + err.player shouldBe someRoleC } } diff --git a/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentRoleFeaturesTest.scala b/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentRoleFeaturesTest.scala index cc4548bf..b56fb1ef 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentRoleFeaturesTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentRoleFeaturesTest.scala @@ -1,7 +1,9 @@ package scroll.tests.parameterized +import scroll.sequenceResults import scroll.internal.dispatch.DispatchQuery import scroll.internal.dispatch.DispatchQuery._ +import scroll.internal.errors.SCROLLErrors.CyclicRolePlayingRelationshipForPlayer import scroll.internal.errors.SCROLLErrors.RoleNotFound import scroll.tests.mocks._ @@ -46,6 +48,21 @@ class MultiCompartmentRoleFeaturesTest extends AbstractParameterizedSCROLLTest { } } + test("Multi-dispatch results can be sequenced") { + forAll(PARAMS) { (c: Boolean, cc: Boolean) => + val someCore = new CoreA() + new MultiCompartmentUnderTest(c, cc) { + someCore play new RoleA() + someCore play new RoleD() + + (+someCore).i[Int]().sequenceResults match { + case Right(values) => values should contain theSameElementsAs Seq(1, 4) + case Left(error) => fail(error.toString) + } + } + } + } + test("Transferring a role") { forAll(PARAMS) { (c: Boolean, cc: Boolean) => val someCoreA = new CoreA() @@ -429,9 +446,10 @@ class MultiCompartmentRoleFeaturesTest extends AbstractParameterizedSCROLLTest { someRoleA play someRoleB someRoleB play someRoleC - a[RuntimeException] should be thrownBy { + val err = the[CyclicRolePlayingRelationshipForPlayer] thrownBy { someRoleC play someRoleA } + err.player shouldBe someRoleC } } diff --git a/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentTest.scala b/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentTest.scala index d13ed883..86049fd9 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/MultiCompartmentTest.scala @@ -15,6 +15,7 @@ class MultiCompartmentTest extends AbstractParameterizedSCROLLTest { case Left(f @ IllegalRoleInvocationDispatch(_, _, _)) => fail(f.toString) case Left(TypeNotFound(tpe)) => fail(s"Unexpected type lookup failure for '$tpe'") case Left(RoleNotFound(core, _, _)) => core shouldBe p + case Left(error) => fail("Unexpected dispatch failure: ", error) } val rA = new RoleA() diff --git a/tests/src/test/scala/scroll/tests/parameterized/RelationshipTest.scala b/tests/src/test/scala/scroll/tests/parameterized/RelationshipTest.scala index dce29173..6e46999c 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/RelationshipTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/RelationshipTest.scala @@ -1,5 +1,6 @@ package scroll.tests.parameterized +import scroll.internal.errors.SCROLLErrors.ConcreteRelationshipMultiplicityViolation import scroll.tests.mocks._ class RelationshipTest extends AbstractParameterizedSCROLLTest { @@ -20,9 +21,10 @@ class RelationshipTest extends AbstractParameterizedSCROLLTest { val rel2 = roleRelationships.create(rel2Name).from[RoleA](relSize).to[RoleC](relSize) rel2.left() should contain only rA - the[AssertionError] thrownBy { + val violation = the[ConcreteRelationshipMultiplicityViolation] thrownBy rel2.right() - } should have message s"assertion failed: With a concrete multiplicity for '$rel2Name' of '$relSize' the resulting role set should have the same size!" + violation.relationshipName shouldBe rel2Name + violation.expectedSize.compare(relSize) shouldBe 0 import scroll.internal.util.Many._ diff --git a/tests/src/test/scala/scroll/tests/parameterized/RoleConstraintsTest.scala b/tests/src/test/scala/scroll/tests/parameterized/RoleConstraintsTest.scala index 1b36b93d..97fa6ecb 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/RoleConstraintsTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/RoleConstraintsTest.scala @@ -20,11 +20,12 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player play roleA play roleB } - the[RoleImplicationConstraintViolation] thrownBy { + val implicationViolation1 = the[RoleImplicationConstraintViolation] thrownBy roleConstraints.checked { player drop roleB } - } should have message s"Role implication constraint violation: '$player' should play role '${roleB.getClass.getName}', but it does not!" + implicationViolation1.player shouldBe player + implicationViolation1.requiredRole shouldBe roleB.getClass.getName roleConstraints.checked { player play roleB @@ -36,17 +37,19 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player play roleC } - the[RoleImplicationConstraintViolation] thrownBy { + val implicationViolation2 = the[RoleImplicationConstraintViolation] thrownBy roleConstraints.checked { player drop roleB } - } should have message s"Role implication constraint violation: '$player' should play role '${roleB.getClass.getName}', but it does not!" + implicationViolation2.player shouldBe player + implicationViolation2.requiredRole shouldBe roleB.getClass.getName - the[RoleImplicationConstraintViolation] thrownBy { + val implicationViolation3 = the[RoleImplicationConstraintViolation] thrownBy roleConstraints.checked { player drop roleC } - } should have message s"Role implication constraint violation: '$player' should play role '${roleB.getClass.getName}', but it does not!" + implicationViolation3.player shouldBe player + implicationViolation3.requiredRole shouldBe roleB.getClass.getName roleConstraints.checked { player play roleC play roleB @@ -74,11 +77,12 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player play roleB } - the[RoleProhibitionConstraintViolation] thrownBy { + val prohibitionViolation1 = the[RoleProhibitionConstraintViolation] thrownBy roleConstraints.checked { player play roleA } - } should have message s"Role prohibition constraint violation: '$player' plays role '${roleB.getClass.getName}', but it is not allowed to do so!" + prohibitionViolation1.player shouldBe player + prohibitionViolation1.prohibitedRole shouldBe roleB.getClass.getName roleConstraints.addRoleProhibition[RoleB, RoleC]() @@ -87,13 +91,14 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player drop roleB } - the[RoleProhibitionConstraintViolation] thrownBy { + val prohibitionViolation2 = the[RoleProhibitionConstraintViolation] thrownBy roleConstraints.checked { player play roleA player play roleB player play roleC } - } should have message s"Role prohibition constraint violation: '$player' plays role '${roleB.getClass.getName}', but it is not allowed to do so!" + prohibitionViolation2.player shouldBe player + prohibitionViolation2.prohibitedRole shouldBe roleB.getClass.getName roleConstraints.checked { player drop roleB @@ -112,21 +117,23 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { val roleC = new RoleC() roleConstraints.addRoleEquivalence[RoleA, RoleB]() - the[RoleEquivalenceConstraintViolation] thrownBy { + val equivalenceViolation1 = the[RoleEquivalenceConstraintViolation] thrownBy roleConstraints.checked { player play roleA } - } should have message s"Role equivalence constraint violation: '$player' should play role '${roleB.getClass.getName}', but it does not!" + equivalenceViolation1.player shouldBe player + equivalenceViolation1.requiredRole shouldBe roleB.getClass.getName roleConstraints.checked { player play roleB } - the[RoleEquivalenceConstraintViolation] thrownBy { + val equivalenceViolation2 = the[RoleEquivalenceConstraintViolation] thrownBy roleConstraints.checked { player drop roleA } - } should have message s"Role equivalence constraint violation: '$player' should play role '${roleA.getClass.getName}', but it does not!" + equivalenceViolation2.player shouldBe player + equivalenceViolation2.requiredRole shouldBe roleA.getClass.getName roleConstraints.checked { player drop roleB @@ -140,11 +147,12 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { player play roleC } - the[RoleEquivalenceConstraintViolation] thrownBy { + val equivalenceViolation3 = the[RoleEquivalenceConstraintViolation] thrownBy roleConstraints.checked { player drop roleB } - } should have message s"Role equivalence constraint violation: '$player' should play role '${roleB.getClass.getName}', but it does not!" + equivalenceViolation3.player shouldBe player + equivalenceViolation3.requiredRole shouldBe roleB.getClass.getName } } @@ -159,12 +167,13 @@ class RoleConstraintsTest extends AbstractParameterizedSCROLLTest { roleConstraints.addRoleImplication[RoleA, RoleB]() roleConstraints.addRoleProhibition[RoleA, RoleB]() - the[RoleProhibitionConstraintViolation] thrownBy { + val prohibitionViolation = the[RoleProhibitionConstraintViolation] thrownBy roleConstraints.checked { player play roleA player play roleB } - } should have message s"Role prohibition constraint violation: '$player' plays role '${roleB.getClass.getName}', but it is not allowed to do so!" + prohibitionViolation.player shouldBe player + prohibitionViolation.prohibitedRole shouldBe roleB.getClass.getName } } diff --git a/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala b/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala index 03ccc924..7027da23 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/RoleGroupsTest.scala @@ -26,17 +26,17 @@ class RoleGroupsTest extends AbstractParameterizedSCROLLTest { acc2 play target } - the[RoleGroupInnerCardinalityViolation] thrownBy { + val innerViolation1 = the[RoleGroupInnerCardinalityViolation] thrownBy roleGroups.checked { acc2 drop target } - } should have message s"Constraint set for inner cardinality of role group '$roleGroupName' violated!" + innerViolation1.groupName shouldBe roleGroupName - the[RoleGroupInnerCardinalityViolation] thrownBy { + val innerViolation2 = the[RoleGroupInnerCardinalityViolation] thrownBy roleGroups.checked { acc1 play target } - } should have message s"Constraint set for inner cardinality of role group '$roleGroupName' violated!" + innerViolation2.groupName shouldBe roleGroupName } } diff --git a/tests/src/test/scala/scroll/tests/parameterized/RoleRestrictionsTest.scala b/tests/src/test/scala/scroll/tests/parameterized/RoleRestrictionsTest.scala index 8c998762..59f696bc 100644 --- a/tests/src/test/scala/scroll/tests/parameterized/RoleRestrictionsTest.scala +++ b/tests/src/test/scala/scroll/tests/parameterized/RoleRestrictionsTest.scala @@ -16,9 +16,11 @@ class RoleRestrictionsTest extends AbstractParameterizedSCROLLTest { player drop roleA roleRestrictions.replaceRoleRestriction[CoreA, RoleD]() - the[RoleRestrictionViolation] thrownBy { + val violation = the[RoleRestrictionViolation] thrownBy { player play roleA - } should have message s"Role '$roleA' can not be played by '$player' due to the active role restrictions!" + } + violation.player shouldBe player + violation.role shouldBe roleA } } @@ -36,9 +38,11 @@ class RoleRestrictionsTest extends AbstractParameterizedSCROLLTest { player play roleA player play roleD - the[RoleRestrictionViolation] thrownBy { + val violation = the[RoleRestrictionViolation] thrownBy { player play roleB - } should have message s"Role '$roleB' can not be played by '$player' due to the active role restrictions!" + } + violation.player shouldBe player + violation.role shouldBe roleB } } From 9215638ba6ceee749845d66d3532bb4e4d6ed156 Mon Sep 17 00:00:00 2001 From: Max Leuthaeuser <1417198+max-leuthaeuser@users.noreply.github.com> Date: Sun, 10 May 2026 14:29:48 +0200 Subject: [PATCH 8/8] Refresh benchmark documentation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- benchmark/README.md | 14 +++++++++++++- .../scroll/benchmarks/AbstractBenchmark.scala | 5 +++++ .../scala/scroll/benchmarks/BankBenchmark.scala | 4 ++++ .../main/scala/scroll/benchmarks/BankExample.scala | 4 ++++ .../scala/scroll/benchmarks/CachingBenchmark.scala | 4 ++++ .../scala/scroll/benchmarks/CachingExample.scala | 2 ++ .../scala/scroll/benchmarks/NoopBenchmark.scala | 4 ++++ .../main/scala/scroll/benchmarks/NoopExample.scala | 2 ++ .../main/scala/scroll/benchmarks/RunnerApp.scala | 4 ++++ 9 files changed, 42 insertions(+), 1 deletion(-) diff --git a/benchmark/README.md b/benchmark/README.md index d6739310..2e5feffc 100644 --- a/benchmark/README.md +++ b/benchmark/README.md @@ -1,12 +1,21 @@ SCROLL Benchmark ================ +This subproject contains the JMH benchmarks used to measure dispatch, caching, and larger end-to-end role scenarios. +It depends on the main `core` module and is compiled as the `benchmark` sbt subproject. + ## How to run ## -1) Run `sbt` and switch to the `benchmark` module with `project benchmark`. +1) Run `sbt` and switch to the `benchmark` module with `project benchmark`, or compile it directly with + `benchmark / compile`. 2) Launch `JMH` e.g., for the `BankExample` benchmark with: `Jmh / run -jvmArgs "-Xms10g -Xmx12g -XX:+UseG1GC -XX:ParallelGCThreads=4 -XX:ConcGCThreads=1 -XX:InitiatingHeapOccupancyPercent=70" .*BankBenchmark` +3) To run through the dedicated benchmark runner that also exports results as `.csv` and `.json`, use: + `benchmark / Jmh / run .*BankBenchmark` + +The main library now exposes the public `scroll` facade for user-facing code. The benchmark sources intentionally stay +close to the lower-level runtime APIs they measure, so they may still import selected `scroll.internal.*` types. For GC parameters see: https://www.oracle.com/technetwork/articles/java/g1gc-1984535.html @@ -17,6 +26,9 @@ For GC parameters see: https://www.oracle.com/technetwork/articles/java/g1gc-198 Use e.g.: http://jmh.morethan.io/ +When benchmarks are started through `RunnerApp`, result files are written to the working directory as +`benchmark_.csv` and `benchmark_.json`. + ## Enable Profiling ## Use e.g., YourKit with: `Jmh / run -jvmArgs "-agentpath:/Applications/YourKit-Java-Profiler-2019.1.app/Contents/Resources/bin/mac/libyjpagent.jnilib=onexit=memory,onexit=snapshot -Xms10g -Xmx12g -XX:+UseG1GC -XX:ParallelGCThreads=4 -XX:ConcGCThreads=1 -XX:InitiatingHeapOccupancyPercent=70" .*BankBenchmark` diff --git a/benchmark/src/main/scala/scroll/benchmarks/AbstractBenchmark.scala b/benchmark/src/main/scala/scroll/benchmarks/AbstractBenchmark.scala index 5fc7cdd4..ac5c10a0 100644 --- a/benchmark/src/main/scala/scroll/benchmarks/AbstractBenchmark.scala +++ b/benchmark/src/main/scala/scroll/benchmarks/AbstractBenchmark.scala @@ -4,6 +4,11 @@ import org.openjdk.jmh.annotations._ import java.util.concurrent.TimeUnit +/** Shared JMH configuration for all SCROLL benchmarks in this subproject. + * + * Concrete benchmarks inherit the same mode, warmup, measurement, fork, and state settings so their results are easier + * to compare across different scenarios. + */ @BenchmarkMode(Array(Mode.AverageTime)) @OutputTimeUnit(TimeUnit.MILLISECONDS) @Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) diff --git a/benchmark/src/main/scala/scroll/benchmarks/BankBenchmark.scala b/benchmark/src/main/scala/scroll/benchmarks/BankBenchmark.scala index 092c5f97..0c5a3036 100644 --- a/benchmark/src/main/scala/scroll/benchmarks/BankBenchmark.scala +++ b/benchmark/src/main/scala/scroll/benchmarks/BankBenchmark.scala @@ -5,6 +5,8 @@ import org.openjdk.jmh.annotations.Scope import org.openjdk.jmh.annotations.Setup import org.openjdk.jmh.annotations.State +/** Benchmarks for the bank example with configurable player, role, transaction, and caching settings. + */ object BankBenchmark { @State(Scope.Benchmark) @@ -26,6 +28,8 @@ object BankBenchmark { } +/** Measures both building and executing the larger bank benchmark scenario. + */ class BankBenchmark extends AbstractBenchmark with BankBenchmarkConfig.Exhaustive { import BankBenchmark._ diff --git a/benchmark/src/main/scala/scroll/benchmarks/BankExample.scala b/benchmark/src/main/scala/scroll/benchmarks/BankExample.scala index 1b8797df..70be0ee5 100644 --- a/benchmark/src/main/scala/scroll/benchmarks/BankExample.scala +++ b/benchmark/src/main/scala/scroll/benchmarks/BankExample.scala @@ -7,6 +7,10 @@ import scroll.internal.dispatch.DispatchQuery.Bypassing import scala.collection.mutable.ArrayBuffer import scala.util.Random +/** End-to-end benchmark fixture modelling a bank with customers, accounts, and money transfers. + * + * The setup exercises role binding, deep dispatch chains, compartment composition, and optional graph caching. + */ class BankExample { class Person(val name: String) diff --git a/benchmark/src/main/scala/scroll/benchmarks/CachingBenchmark.scala b/benchmark/src/main/scala/scroll/benchmarks/CachingBenchmark.scala index d940289f..9ed88d3f 100644 --- a/benchmark/src/main/scala/scroll/benchmarks/CachingBenchmark.scala +++ b/benchmark/src/main/scala/scroll/benchmarks/CachingBenchmark.scala @@ -3,6 +3,8 @@ package scroll.benchmarks import org.openjdk.jmh.annotations._ import org.openjdk.jmh.annotations.Benchmark +/** Benchmarks for comparing cached versus non-cached dispatch over repeated invocations. + */ object CachingBenchmark { @State(Scope.Benchmark) @@ -27,6 +29,8 @@ object CachingBenchmark { } +/** Measures the cost of repeated dispatches through cached and non-cached role graphs. + */ class CachingBenchmark extends AbstractBenchmark { import CachingBenchmark._ diff --git a/benchmark/src/main/scala/scroll/benchmarks/CachingExample.scala b/benchmark/src/main/scala/scroll/benchmarks/CachingExample.scala index 3d538a9a..6578ea9c 100644 --- a/benchmark/src/main/scala/scroll/benchmarks/CachingExample.scala +++ b/benchmark/src/main/scala/scroll/benchmarks/CachingExample.scala @@ -2,6 +2,8 @@ package scroll.benchmarks import scroll.internal.compartment.impl.Compartment +/** Benchmark fixture comparing cached and non-cached dispatch lookups across a role chain of configurable depth. + */ class CachingExample { class Core diff --git a/benchmark/src/main/scala/scroll/benchmarks/NoopBenchmark.scala b/benchmark/src/main/scala/scroll/benchmarks/NoopBenchmark.scala index 67c1734c..dd4210bc 100644 --- a/benchmark/src/main/scala/scroll/benchmarks/NoopBenchmark.scala +++ b/benchmark/src/main/scala/scroll/benchmarks/NoopBenchmark.scala @@ -6,6 +6,8 @@ import scroll.internal.dispatch.SCROLLDynamic import java.util.concurrent.TimeUnit import scala.util.Random +/** Benchmarks for the lightweight no-op forwarding scenario. + */ object NoopBenchmark { @State(Scope.Thread) @@ -16,6 +18,8 @@ object NoopBenchmark { @Param(Array("true", "false")) var cached: Boolean = scala.compiletime.uninitialized + /** Rebuilds the no-op scenario for the current cache mode before each iteration. + */ @Setup(Level.Iteration) def setup(): Unit = { player = new NoopExample(cached).compartment.player diff --git a/benchmark/src/main/scala/scroll/benchmarks/NoopExample.scala b/benchmark/src/main/scala/scroll/benchmarks/NoopExample.scala index 9706c7de..d8fe4354 100644 --- a/benchmark/src/main/scala/scroll/benchmarks/NoopExample.scala +++ b/benchmark/src/main/scala/scroll/benchmarks/NoopExample.scala @@ -4,6 +4,8 @@ import scroll.internal.compartment.impl.Compartment import scroll.internal.dispatch.DispatchQuery import scroll.internal.dispatch.DispatchQuery._ +/** Minimal benchmark fixture for measuring dispatch overhead when a role only forwards to the base object. + */ class NoopExample(cached: Boolean) { val compartment = new NoopCompartment() diff --git a/benchmark/src/main/scala/scroll/benchmarks/RunnerApp.scala b/benchmark/src/main/scala/scroll/benchmarks/RunnerApp.scala index 246e5d94..b22feabc 100644 --- a/benchmark/src/main/scala/scroll/benchmarks/RunnerApp.scala +++ b/benchmark/src/main/scala/scroll/benchmarks/RunnerApp.scala @@ -8,6 +8,10 @@ import org.openjdk.jmh.runner.options.CommandLineOptions import java.text.SimpleDateFormat import java.util.Date +/** Runs JMH from the command line and writes the collected results in machine-readable formats. + * + * Output files are created in the current working directory using a timestamped `benchmark_` prefix. + */ object RunnerApp { def main(args: Array[String]): Unit = {