From c337bf52875f85d482d410fceed0bfa10298e1dc Mon Sep 17 00:00:00 2001 From: Arnout Engelen Date: Mon, 16 Dec 2019 13:04:09 +0100 Subject: [PATCH] Remove use of getClass in secondary constructors (#28355) * Remove use of getClass in secondary constructors As this is not allowed anymore on newer versions of scala, and likely didn't work correctly in the past either This might make selecting unique actor system names for test actor systems a bit less reliable, but that didn't seem to be critical anyway. Thanks to @som-snytt for the heads-up and initial implementation in #28353 * Avoid AkkaSpec.getCallerName in MultiNodeClusterShardingConfig * StreamSpec can be abstract * Use more sophisticated test class name logic * scalafmt --- .../MultiNodeClusterShardingConfig.scala | 55 +++++++++++++++++- .../MultiNodeClusterShardingSpec.scala | 4 ++ .../akka/stream/testkit/StreamSpec.scala | 9 ++- .../test/scala/akka/testkit/AkkaSpec.scala | 57 +++++++++++++++---- 4 files changed, 109 insertions(+), 16 deletions(-) diff --git a/akka-cluster-sharding/src/multi-jvm/scala/akka/cluster/sharding/MultiNodeClusterShardingConfig.scala b/akka-cluster-sharding/src/multi-jvm/scala/akka/cluster/sharding/MultiNodeClusterShardingConfig.scala index 57b07ea365..5aa1dbc0b5 100644 --- a/akka-cluster-sharding/src/multi-jvm/scala/akka/cluster/sharding/MultiNodeClusterShardingConfig.scala +++ b/akka-cluster-sharding/src/multi-jvm/scala/akka/cluster/sharding/MultiNodeClusterShardingConfig.scala @@ -4,15 +4,63 @@ package akka.cluster.sharding +import java.lang.reflect.Modifier + import akka.cluster.MultiNodeClusterSpec import akka.persistence.journal.leveldb.SharedLeveldbJournal import akka.remote.testkit.MultiNodeConfig -import akka.testkit.AkkaSpec import com.typesafe.config.{ Config, ConfigFactory } +object MultiNodeClusterShardingConfig { + private[sharding] def testNameFromCallStack(classToStartFrom: Class[_]): String = { + + def isAbstractClass(className: String): Boolean = { + try { + Modifier.isAbstract(Class.forName(className).getModifiers) + } catch { + case _: Throwable => false // yes catch everything, best effort check + } + } + + val startFrom = classToStartFrom.getName + val filteredStack = Thread.currentThread.getStackTrace.iterator + .map(_.getClassName) + // drop until we find the first occurrence of classToStartFrom + .dropWhile(!_.startsWith(startFrom)) + // then continue to the next entry after classToStartFrom that makes sense + .dropWhile { + case `startFrom` => true + case str if str.startsWith(startFrom + "$") => true // lambdas inside startFrom etc + case str if isAbstractClass(str) => true + case _ => false + } + + if (filteredStack.isEmpty) + throw new IllegalArgumentException(s"Couldn't find [${classToStartFrom.getName}] in call stack") + + // sanitize for actor system name + scrubActorSystemName(filteredStack.next()) + } + + /** + * Sanitize the `name` to be used as valid actor system name by + * replacing invalid characters. `name` may for example be a fully qualified + * class name and then the short class name will be used. + */ + def scrubActorSystemName(name: String): String = { + name + .replaceFirst("""^.*\.""", "") // drop package name + .replaceAll("""\$\$?\w+""", "") // drop scala anonymous functions/classes + .replaceAll("[^a-zA-Z_0-9]", "_") + } +} + /** * A MultiNodeConfig for ClusterSharding. Implement the roles, etc. and create with the following: * + * Note that this class is not used anywhere yet, but could be a good starting point + * for new or refactored multi-node sharding specs + * * @param mode the state store mode * @param rememberEntities defaults to off * @param overrides additional config @@ -25,7 +73,10 @@ abstract class MultiNodeClusterShardingConfig( loglevel: String = "INFO") extends MultiNodeConfig { - val targetDir = s"target/ClusterSharding${AkkaSpec.getCallerName(getClass)}Spec-$mode-remember-$rememberEntities" + import MultiNodeClusterShardingConfig._ + + val targetDir = + s"target/ClusterSharding${testNameFromCallStack(classOf[MultiNodeClusterShardingConfig])}Spec-$mode-remember-$rememberEntities" val modeConfig = if (mode == ClusterShardingSettings.StateStoreModeDData) ConfigFactory.empty diff --git a/akka-cluster-sharding/src/multi-jvm/scala/akka/cluster/sharding/MultiNodeClusterShardingSpec.scala b/akka-cluster-sharding/src/multi-jvm/scala/akka/cluster/sharding/MultiNodeClusterShardingSpec.scala index 39975c6e18..c0b67696b0 100644 --- a/akka-cluster-sharding/src/multi-jvm/scala/akka/cluster/sharding/MultiNodeClusterShardingSpec.scala +++ b/akka-cluster-sharding/src/multi-jvm/scala/akka/cluster/sharding/MultiNodeClusterShardingSpec.scala @@ -45,6 +45,10 @@ object MultiNodeClusterShardingSpec { } +/** + * Note that this class is not used anywhere yet, but could be a good starting point + * for new or refactored multi-node sharding specs + */ abstract class MultiNodeClusterShardingSpec(val config: MultiNodeClusterShardingConfig) extends MultiNodeSpec(config) with MultiNodeClusterSpec { diff --git a/akka-stream-testkit/src/test/scala/akka/stream/testkit/StreamSpec.scala b/akka-stream-testkit/src/test/scala/akka/stream/testkit/StreamSpec.scala index b40ae31942..a1b777d771 100644 --- a/akka-stream-testkit/src/test/scala/akka/stream/testkit/StreamSpec.scala +++ b/akka-stream-testkit/src/test/scala/akka/stream/testkit/StreamSpec.scala @@ -14,15 +14,18 @@ import org.scalatest.Failed import scala.concurrent.Future import scala.concurrent.duration._ -class StreamSpec(_system: ActorSystem) extends AkkaSpec(_system) { +abstract class StreamSpec(_system: ActorSystem) extends AkkaSpec(_system) { def this(config: Config) = - this(ActorSystem(AkkaSpec.getCallerName(getClass), ConfigFactory.load(config.withFallback(AkkaSpec.testConf)))) + this( + ActorSystem( + AkkaSpec.testNameFromCallStack(classOf[StreamSpec]), + ConfigFactory.load(config.withFallback(AkkaSpec.testConf)))) def this(s: String) = this(ConfigFactory.parseString(s)) def this(configMap: Map[String, _]) = this(AkkaSpec.mapToConfig(configMap)) - def this() = this(ActorSystem(AkkaSpec.getCallerName(getClass), AkkaSpec.testConf)) + def this() = this(ActorSystem(AkkaSpec.testNameFromCallStack(classOf[StreamSpec]), AkkaSpec.testConf)) override def withFixture(test: NoArgTest) = { super.withFixture(test) match { diff --git a/akka-testkit/src/test/scala/akka/testkit/AkkaSpec.scala b/akka-testkit/src/test/scala/akka/testkit/AkkaSpec.scala index 8c9aff012a..c2d9bd77dc 100644 --- a/akka-testkit/src/test/scala/akka/testkit/AkkaSpec.scala +++ b/akka-testkit/src/test/scala/akka/testkit/AkkaSpec.scala @@ -4,6 +4,8 @@ package akka.testkit +import java.lang.reflect.Modifier + import org.scalactic.{ CanEqual, TypeCheckedTripleEquals } import language.postfixOps @@ -14,6 +16,7 @@ import akka.event.{ Logging, LoggingAdapter } import scala.concurrent.duration._ import scala.concurrent.Future + import com.typesafe.config.{ Config, ConfigFactory } import akka.dispatch.Dispatchers import akka.testkit.TestEvent._ @@ -44,18 +47,47 @@ object AkkaSpec { ConfigFactory.parseMap(map.asJava) } - def getCallerName(clazz: Class[_]): String = { - val s = Thread.currentThread.getStackTrace - .map(_.getClassName) - .drop(1) - .dropWhile(_.matches("(java.lang.Thread|.*AkkaSpec.*|.*\\.StreamSpec.*|.*MultiNodeSpec.*|.*\\.Abstract.*)")) - val reduced = s.lastIndexWhere(_ == clazz.getName) match { - case -1 => s - case z => s.drop(z + 1) + def testNameFromCallStack(classToStartFrom: Class[_]): String = { + + def isAbstractClass(className: String): Boolean = { + try { + Modifier.isAbstract(Class.forName(className).getModifiers) + } catch { + case _: Throwable => false // yes catch everything, best effort check + } } - reduced.head.replaceFirst(""".*\.""", "").replaceAll("[^a-zA-Z_0-9]", "_") + + val startFrom = classToStartFrom.getName + val filteredStack = Thread.currentThread.getStackTrace.iterator + .map(_.getClassName) + // drop until we find the first occurrence of classToStartFrom + .dropWhile(!_.startsWith(startFrom)) + // then continue to the next entry after classToStartFrom that makes sense + .dropWhile { + case `startFrom` => true + case str if str.startsWith(startFrom + "$") => true // lambdas inside startFrom etc + case str if isAbstractClass(str) => true + case _ => false + } + + if (filteredStack.isEmpty) + throw new IllegalArgumentException(s"Couldn't find [${classToStartFrom.getName}] in call stack") + + // sanitize for actor system name + scrubActorSystemName(filteredStack.next()) } + /** + * Sanitize the `name` to be used as valid actor system name by + * replacing invalid characters. `name` may for example be a fully qualified + * class name and then the short class name will be used. + */ + def scrubActorSystemName(name: String): String = { + name + .replaceFirst("""^.*\.""", "") // drop package name + .replaceAll("""\$\$?\w+""", "") // drop scala anonymous functions/classes + .replaceAll("[^a-zA-Z_0-9]", "_") + } } abstract class AkkaSpec(_system: ActorSystem) @@ -70,13 +102,16 @@ abstract class AkkaSpec(_system: ActorSystem) implicit val patience = PatienceConfig(testKitSettings.DefaultTimeout.duration, Span(100, Millis)) def this(config: Config) = - this(ActorSystem(AkkaSpec.getCallerName(getClass), ConfigFactory.load(config.withFallback(AkkaSpec.testConf)))) + this( + ActorSystem( + AkkaSpec.testNameFromCallStack(classOf[AkkaSpec]), + ConfigFactory.load(config.withFallback(AkkaSpec.testConf)))) def this(s: String) = this(ConfigFactory.parseString(s)) def this(configMap: Map[String, _]) = this(AkkaSpec.mapToConfig(configMap)) - def this() = this(ActorSystem(AkkaSpec.getCallerName(getClass), AkkaSpec.testConf)) + def this() = this(ActorSystem(AkkaSpec.testNameFromCallStack(classOf[AkkaSpec]), AkkaSpec.testConf)) val log: LoggingAdapter = Logging(system, this.getClass)