diff --git a/akka-actor-tests/src/test/java/akka/actor/ActorCreationTest.java b/akka-actor-tests/src/test/java/akka/actor/ActorCreationTest.java new file mode 100644 index 0000000000..b16523af5b --- /dev/null +++ b/akka-actor-tests/src/test/java/akka/actor/ActorCreationTest.java @@ -0,0 +1,68 @@ +/** + * Copyright (C) 2009-2013 Typesafe Inc. + */ + +package akka.actor; + +import static org.junit.Assert.*; +import org.junit.Test; + +import akka.japi.Creator; + +public class ActorCreationTest { + + @Test + public void testWrongCreator() { + try { + Props.create(new Creator() { + @Override + public Actor create() throws Exception { + return null; + } + }); + assert false; + } catch(IllegalArgumentException e) { + assertEquals("cannot use non-static local Creator to create actors; make it static or top-level", e.getMessage()); + } + } + + static class C implements Creator { + @Override + public UntypedActor create() throws Exception { + return null; + } + } + + static class D implements Creator { + @Override + public T create() { + return null; + } + } + + static class E implements Creator { + @Override + public T create() { + return null; + } + } + + @Test + public void testRightCreator() { + final Props p = Props.create(new C()); + assertEquals(UntypedActor.class, p.actorClass()); + } + + @Test + public void testParametricCreator() { + final Props p = Props.create(new D()); + assertEquals(Actor.class, p.actorClass()); + } + + @Test + public void testBoundedCreator() { + final Props p = Props.create(new E()); + assertEquals(UntypedActor.class, p.actorClass()); + } + +} diff --git a/akka-actor/src/main/scala/akka/actor/Props.scala b/akka-actor/src/main/scala/akka/actor/Props.scala index 81c19596ed..46b74bcd26 100644 --- a/akka-actor/src/main/scala/akka/actor/Props.scala +++ b/akka-actor/src/main/scala/akka/actor/Props.scala @@ -12,9 +12,12 @@ import akka.util.Reflect import scala.annotation.varargs import Deploy.{ NoDispatcherGiven, NoMailboxGiven } import scala.collection.immutable - import scala.language.existentials import java.lang.reflect.Constructor +import java.lang.reflect.Modifier +import scala.annotation.tailrec +import java.lang.reflect.ParameterizedType +import java.lang.reflect.TypeVariable /** * Factory for Props instances. @@ -40,16 +43,16 @@ object Props { */ final val defaultDeploy = Deploy() - /** - * The default Props instance, uses the settings from the Props object starting with default*. - */ - final val default = new Props() - /** * A Props instance whose creator will create an actor that doesn't respond to any message */ final val empty = Props[EmptyActor] + /** + * The default Props instance, uses the settings from the Props object starting with default*. + */ + final val default = new Props() + /** * INTERNAL API * @@ -85,7 +88,7 @@ object Props { /** * The deprecated legacy constructor. */ - @deprecated("give class and arguments instead", "2.2") + @deprecated("use Props.withDispatcher and friends", "2.2") def apply( creator: () ⇒ Actor = Props.defaultCreator, dispatcher: String = Dispatchers.DefaultDispatcherId, @@ -98,6 +101,13 @@ object Props { if (d2 != Props.defaultDeploy) p.withDeploy(d2) else p } + /** + * The deprecated legacy extractor. + */ + @deprecated("use three-argument version", "2.2") + def unapply(p: Props)(dummy: Int = 0): Option[(() ⇒ Actor, String, RouterConfig, Deploy)] = + Some((p.creator, p.dispatcher, p.routerConfig, p.deploy)) + /** * Scala API: create a Props given a class and its constructor arguments. */ @@ -108,6 +118,30 @@ object Props { */ @varargs def create(clazz: Class[_], args: AnyRef*): Props = apply(defaultDeploy, clazz, args.toVector) + + /** + * Create new Props from the given [[Creator]]. + */ + def create[T <: Actor](creator: Creator[T]): Props = { + if ((creator.getClass.getModifiers & Modifier.STATIC) == 0) + throw new IllegalArgumentException("cannot use non-static local Creator to create actors; make it static or top-level") + val cc = classOf[Creator[_]] + val ac = classOf[Actor] + @tailrec def findType(c: Class[_]): Class[_] = { + c.getGenericInterfaces collectFirst { + case t: ParameterizedType if cc.isAssignableFrom(t.getRawType.asInstanceOf[Class[_]]) ⇒ + t.getActualTypeArguments.head match { + case c: Class[_] ⇒ c // since T <: Actor + case v: TypeVariable[_] ⇒ + v.getBounds collectFirst { case c: Class[_] if ac.isAssignableFrom(c) && c != ac ⇒ c } getOrElse ac + } + } match { + case Some(x) ⇒ x + case None ⇒ findType(c.getSuperclass) + } + } + apply(defaultDeploy, classOf[CreatorConsumer], findType(creator.getClass) :: creator :: Nil) + } } /** @@ -183,7 +217,7 @@ final case class Props(deploy: Deploy, clazz: Class[_], args: immutable.Seq[Any] * non-serializable inner classes, making them also * non-serializable */ - @deprecated("use Props.create", "2.2") + @deprecated("use Props.create()", "2.2") def this(factory: UntypedActorFactory) = this(Props.defaultDeploy, classOf[UntypedActorFactoryConsumer], Vector(factory)) /** @@ -227,6 +261,7 @@ final case class Props(deploy: Deploy, clazz: Class[_], args: immutable.Seq[Any] * * The creator must not return the same instance multiple times. */ + @deprecated("use Props(...).withDeploy(other.deploy)", "2.2") def withCreator(c: ⇒ Actor): Props = copy(clazz = classOf[CreatorFunctionConsumer], args = (() ⇒ c) :: Nil) /** @@ -239,8 +274,8 @@ final case class Props(deploy: Deploy, clazz: Class[_], args: immutable.Seq[Any] * non-serializable inner classes, making them also * non-serializable */ - @deprecated("use Props.create(clazz, args ...) instead", "2.2") - def withCreator(c: Creator[Actor]): Props = copy(clazz = classOf[CreatorConsumer], args = c :: Nil) + @deprecated("use Props.create(clazz, args ...).withDeploy(other.deploy) instead", "2.2") + def withCreator(c: Creator[Actor]): Props = copy(clazz = classOf[CreatorConsumer], args = classOf[Actor] :: c :: Nil) /** * Returns a new Props with the specified creator set. @@ -248,7 +283,7 @@ final case class Props(deploy: Deploy, clazz: Class[_], args: immutable.Seq[Any] * @deprecated use Props.create(clazz) instead; deprecated since it duplicates * another API */ - @deprecated("use Props(clazz, args).withDeploy(other.deploy)", "2.2") + @deprecated("use Props.create(clazz, args).withDeploy(other.deploy)", "2.2") def withCreator(c: Class[_ <: Actor]): Props = copy(clazz = c, args = Nil) /** @@ -342,8 +377,8 @@ private[akka] class CreatorFunctionConsumer(creator: () ⇒ Actor) extends Indir /** * INTERNAL API */ -private[akka] class CreatorConsumer(creator: Creator[Actor]) extends IndirectActorProducer { - override def actorClass = classOf[Actor] +private[akka] class CreatorConsumer(clazz: Class[_ <: Actor], creator: Creator[Actor]) extends IndirectActorProducer { + override def actorClass = clazz override def produce() = creator.create() } diff --git a/akka-actor/src/main/scala/akka/actor/UntypedActor.scala b/akka-actor/src/main/scala/akka/actor/UntypedActor.scala index 0b88c58089..8d9d61fb28 100644 --- a/akka-actor/src/main/scala/akka/actor/UntypedActor.scala +++ b/akka-actor/src/main/scala/akka/actor/UntypedActor.scala @@ -166,5 +166,5 @@ abstract class UntypedActor extends Actor { /** * Factory closure for an UntypedActor, to be used with 'Actors.actorOf(factory)'. */ -@deprecated("use Props.create(clazz, args) instead", "2.2") +@deprecated("use Creator instead", "2.2") trait UntypedActorFactory extends Creator[Actor] with Serializable diff --git a/akka-actor/src/main/scala/akka/actor/dsl/Creators.scala b/akka-actor/src/main/scala/akka/actor/dsl/Creators.scala index ffc973e491..fce0d0da23 100644 --- a/akka-actor/src/main/scala/akka/actor/dsl/Creators.scala +++ b/akka-actor/src/main/scala/akka/actor/dsl/Creators.scala @@ -14,7 +14,6 @@ import java.util.concurrent.TimeoutException import java.util.concurrent.atomic.AtomicInteger import akka.pattern.ask import scala.reflect.ClassTag -import akka.dispatch.{ UnboundedDequeBasedMailbox, RequiresMessageQueue } trait Creators { this: ActorDSL.type ⇒ diff --git a/akka-actor/src/main/scala/akka/japi/JavaAPI.scala b/akka-actor/src/main/scala/akka/japi/JavaAPI.scala index 66e56cbb6f..2a888836eb 100644 --- a/akka-actor/src/main/scala/akka/japi/JavaAPI.scala +++ b/akka-actor/src/main/scala/akka/japi/JavaAPI.scala @@ -48,7 +48,8 @@ trait Effect { /** * A constructor/factory, takes no parameters but creates a new value of type T every call. */ -trait Creator[T] { +@SerialVersionUID(1L) +trait Creator[T] extends Serializable { /** * This method must return a different instance upon every call. */ diff --git a/akka-docs/rst/java/code/docs/actor/UntypedActorDocTest.java b/akka-docs/rst/java/code/docs/actor/UntypedActorDocTest.java index 7619926409..966be3850a 100644 --- a/akka-docs/rst/java/code/docs/actor/UntypedActorDocTest.java +++ b/akka-docs/rst/java/code/docs/actor/UntypedActorDocTest.java @@ -49,6 +49,7 @@ import akka.actor.IndirectActorProducer; import akka.actor.OneForOneStrategy; //#import-props import akka.actor.Props; +import akka.japi.Creator; //#import-props import akka.actor.SupervisorStrategy; import akka.actor.SupervisorStrategy.Directive; @@ -88,14 +89,35 @@ public class UntypedActorDocTest { private final ActorSystem system = actorSystemResource.getSystem(); + //#creating-props-config + static class MyActorC implements Creator { + @Override public MyActor create() { + return new MyActor("..."); + } + } + + //#creating-props-config + @SuppressWarnings("unused") @Test public void createProps() { //#creating-props-config Props props1 = Props.create(MyUntypedActor.class); Props props2 = Props.create(MyActor.class, "..."); + Props props3 = Props.create(new MyActorC()); //#creating-props-config } + + //#parametric-creator + static class ParametricCreator implements Creator { + @Override public T create() { + // ... fabricate actor here + //#parametric-creator + return null; + //#parametric-creator + } + } + //#parametric-creator @SuppressWarnings("deprecation") @Test diff --git a/akka-docs/rst/java/untyped-actors.rst b/akka-docs/rst/java/untyped-actors.rst index 7483a7cc8d..4457ee9d0a 100644 --- a/akka-docs/rst/java/untyped-actors.rst +++ b/akka-docs/rst/java/untyped-actors.rst @@ -51,12 +51,20 @@ dispatcher to use, see more below). Here are some examples of how to create a .. includecode:: code/docs/actor/UntypedActorDocTest.java#import-props .. includecode:: code/docs/actor/UntypedActorDocTest.java#creating-props-config -The last line shows how to pass constructor arguments to the :class:`Actor` +The second line shows how to pass constructor arguments to the :class:`Actor` being created. The presence of a matching constructor is verified during construction of the :class:`Props` object, resulting in an :class:`IllegalArgumentEception` if no or multiple matching constructors are found. +The third line demonstrates the use of a :class:`Creator`. The +creator class must be static, which is verified during :class:`Props` +construction. The type parameter’s upper bound is used to determine the +produced actor class, falling back to :class:`Actor` if fully erased. An +example of a parametric factory could be: + +.. includecode:: code/docs/actor/UntypedActorDocTest.java#parametric-creator + Deprecated Variants ^^^^^^^^^^^^^^^^^^^ diff --git a/akka-docs/rst/project/migration-guide-2.1.x-2.2.x.rst b/akka-docs/rst/project/migration-guide-2.1.x-2.2.x.rst index 19497c3838..74f074bb5a 100644 --- a/akka-docs/rst/project/migration-guide-2.1.x-2.2.x.rst +++ b/akka-docs/rst/project/migration-guide-2.1.x-2.2.x.rst @@ -18,17 +18,57 @@ Deprecated Closure-Taking Props are usually created in-line and thus carry a reference to their enclosing object; this is not well known among programmers, in particular it can be surprising that innocent-looking actor creation should not be serializable, -e.g. if the enclosing class is an actor. +e.g. if the enclosing class is an actor. Another issue which came up often +during reviews is that these actor creators inadvertedly close over the Actor’s +``this`` reference for calling methods on it, which is inherently unsafe. -Thus we have decided to deprecate ``Props(new MyActor(...))`` and -:class:`UntypedActorFactory` in favor of basing :class:`Props` on a -:class:`Class` and a sequence of constructor arguments. This has the added -benefit of allowing easier integration with dependency injection frameworks, -see :ref:`actor-create-factory`. +Another reason for changing the underlying implementation is that Props now +carries information about which class of actor will be created, allowing the +extraction of mailbox type requirements (e.g. when using the Stash) before +trying to create the actor. Being based on the actor class and a list of +constructor arguments also allows these arguments to be serialized according to +the configured serializer bindings instead of mandating Java serialization +(which was used previously). -The deprecated methods will be retained until the possibility of reintroducing -a similar syntax in a safe fashion has been properly researched (in case of -Scala it might be possible to use macros to this effect). +What changes for Java? +---------------------- + +A new method ``Props.create`` has been introduced with two overloads:: + + Props.create(MyActor.class, arg1, arg2, ...); + // or + Props.create(new MyActorCreator(args ...)); + +In the first case the existence of a constructor signature matching the +supplied arguments is verified at Props construction time. In the second case +it is verified that ``MyActorCreator`` (which must be a ``akka.japi.Creator``) is a static class. In both cases failure is signaled by +throwing a :class:`IllegalArgumentException`. + +The constructors of :class:`Props` have been deprecated to facilitate migration. + +The :meth:`withCreator` methods have been deprecated. The functionality is +available by using ``Props.create(...).withDeploy(oldProps.deploy());``. + +:class:`UntypedActorFactory` has been deprecated in favor of the more precisely +typed :class:`Creator`. + +What changes for Scala? +----------------------- + +The case class signature of Props has been changed to only contain a +:class:`Deploy`, a :class:`Class[_]` and an immutable :class:`Seq[Any]` (the +constructor arguments for the class). The old factory and extractor methods +have been deprecated. + +Properly serializable :class:`Props` can now be created for actors which take +constructor arguments by using ``Props(classOf[MyActor], arg1, arg2, ...)``. +In a future update—possibly within the 2.2.x timeframe—we plan to introduce a +macro which will transform the by-name argument to ``Props(new MyActor(...))`` +into a call to the former. + +The :meth:`withCreator` methods have been deprecated. The functionality is +available by using ``Props(...).withDeploy(oldProps.deploy)``. Immutable everywhere ==================== diff --git a/akka-docs/rst/scala/actors.rst b/akka-docs/rst/scala/actors.rst index 472d94a636..4772899a7a 100644 --- a/akka-docs/rst/scala/actors.rst +++ b/akka-docs/rst/scala/actors.rst @@ -85,18 +85,21 @@ for a migration period): .. includecode:: code/docs/actor/ActorDocSpec.scala#creating-props-deprecated -The last one is deprecated because its functionality is available in full -through :meth:`Props.apply()`. +The first one is deprecated because the case class structure changed between +Akka 2.1 and 2.2. -The first three are deprecated because the captured closure is a local class -which means that it implicitly carries a reference to the enclosing class. This -can easily make the resulting :class:`Props` non-serializable, e.g. when the -enclosing class is an :class:`Actor`. Akka advocates location transparency, -meaning that an application written with actors should just work when it is -deployed over multiple network nodes, and non-serializable actor factories -would break this principle. In case indirect actor creation is needed—for -example when using dependency injection—there is the possibility to use an -:class:`IndirectActorProducer` as described below. +The two variants in the middle are deprecated because :class:`Props` are +primarily concerned with actor creation and thus the “creator” part should be +explicitly set when creating an instance. In case you want to deploy one actor +in the same was as another, simply use +``Props(...).withDeploy(otherProps.deploy)``. + +The last one is not technically deprecated, but it is not recommended because +it encourages to close over the enclosing scope, resulting in non-serializable +:class:`Props` and possibly race conditions (breaking the actor encapsulation). +We will provide a macro-based solution in a future release which allows similar +syntax without the headaches, at which point this variant will be properly +deprecated. There were two use-cases for these methods: passing constructor arguments to the actor—which is solved by the newly introduced diff --git a/akka-docs/rst/scala/code/docs/actor/ActorDocSpec.scala b/akka-docs/rst/scala/code/docs/actor/ActorDocSpec.scala index 0df50cb494..e0bd55593b 100644 --- a/akka-docs/rst/scala/code/docs/actor/ActorDocSpec.scala +++ b/akka-docs/rst/scala/code/docs/actor/ActorDocSpec.scala @@ -243,19 +243,19 @@ class ActorDocSpec extends AkkaSpec(Map("akka.loglevel" -> "INFO")) { //#creating-props //#creating-props-deprecated - // DEPRECATED: encourages to close over enclosing class + // DEPRECATED: old case class signature val props4 = Props( creator = { () ⇒ new MyActor }, dispatcher = "my-dispatcher") - // DEPRECATED: encourages to close over enclosing class + // DEPRECATED due to duplicate functionality with Props.apply() val props5 = props1.withCreator(new MyActor) - // DEPRECATED: encourages to close over enclosing class - val props6 = Props(new MyActor) - // DEPRECATED due to duplicate functionality with Props.apply() - val props7 = props1.withCreator(classOf[MyActor]) + val props6 = props1.withCreator(classOf[MyActor]) + + // NOT RECOMMENDED: encourages to close over enclosing class + val props7 = Props(new MyActor) //#creating-props-deprecated } diff --git a/project/AkkaBuild.scala b/project/AkkaBuild.scala index 6f92e9b5c4..8fab2e7184 100644 --- a/project/AkkaBuild.scala +++ b/project/AkkaBuild.scala @@ -136,7 +136,8 @@ object AkkaBuild extends Build { dependencies = Seq(testkit % "compile;test->test"), settings = defaultSettings ++ scaladocSettings ++ Seq( publishArtifact in Compile := false, - libraryDependencies ++= Dependencies.actorTests + libraryDependencies ++= Dependencies.actorTests, + testOptions += Tests.Argument(TestFrameworks.JUnit, "-v", "-a") ) ) @@ -972,7 +973,7 @@ object Dependencies { val testkit = Seq(Test.junit, Test.scalatest) - val actorTests = Seq(Test.junit, Test.scalatest, Test.commonsCodec, Test.commonsMath, Test.mockito, Test.scalacheck, protobuf) + val actorTests = Seq(Test.junit, Test.scalatest, Test.commonsCodec, Test.commonsMath, Test.mockito, Test.scalacheck, protobuf, Test.junitIntf) val remote = Seq(netty, protobuf, uncommonsMath, Test.junit, Test.scalatest)