Fixed cluster group router UseRole ignored bug (#3303)

* fixed bug in UseRoleIgnoredSpec

* close #3294 - fixed usages of GetPaths inside all Group router implementations

src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt

@@ -4061,7 +4061,9 @@ namespace Akka.Routing
     }
     public abstract class Group : Akka.Routing.RouterConfig, System.IEquatable<Akka.Routing.Group>
     {
+        protected readonly string[] InternalPaths;
         protected Group(System.Collections.Generic.IEnumerable<string> paths, string routerDispatcher) { }
+        [System.ObsoleteAttribute("Deprecated since Akka.NET v1.1. Use Paths(ActorSystem) instead.")]
         public System.Collections.Generic.IEnumerable<string> Paths { get; }
         public bool Equals(Akka.Routing.Group other) { }
         public override bool Equals(object obj) { }

src/core/Akka.Cluster.Tests.MultiNode/Routing/UseRoleIgnoredSpec.cs

@@ -272,7 +272,7 @@ private void A_cluster_must_group_local_on_role_b()
                 var router = Sys.ActorOf(
                     new ClusterRouterGroup(
                         new RoundRobinGroup(paths: null),
-                        new ClusterRouterGroupSettings(6, ImmutableHashSet.Create("/user/foo", "/user/bar"), allowLocalRoutees: false, useRole: role)).Props(),
+                        new ClusterRouterGroupSettings(6, ImmutableHashSet.Create("/user/foo", "/user/bar"), allowLocalRoutees: true, useRole: role)).Props(),
                     "router-3b");
 
                 AwaitAssert(() => CurrentRoutees(router).Count().Should().Be(4));

src/core/Akka.Cluster.Tests/ClusterDeployerSpec.cs

@@ -84,7 +84,7 @@ public void RemoteDeployer_must_be_able_to_parse_akka_actor_deployment_with_spec
             deployment.Path.ShouldBe(service);
             deployment.RouterConfig.GetType().ShouldBe(typeof(ClusterRouterGroup));
             deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Local.GetType().ShouldBe(typeof(RoundRobinGroup));
-            deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Local.AsInstanceOf<RoundRobinGroup>().Paths.ShouldBe(new[]{ "/user/myservice" });
+            deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Local.AsInstanceOf<RoundRobinGroup>().GetPaths(Sys).ShouldBe(new[]{ "/user/myservice" });
             deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Settings.TotalInstances.ShouldBe(20);
             deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Settings.AllowLocalRoutees.ShouldBe(false);
             deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Settings.UseRole.ShouldBe("backend");
@@ -104,7 +104,7 @@ public void BugFix2266RemoteDeployer_must_be_able_to_parse_broadcast_group_clust
             deployment.Path.ShouldBe(service);
             deployment.RouterConfig.GetType().ShouldBe(typeof(ClusterRouterGroup));
             deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Local.GetType().ShouldBe(typeof(BroadcastGroup));
-            deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Local.AsInstanceOf<BroadcastGroup>().Paths.ShouldBe(new[] { "/user/myservice" });
+            deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Local.AsInstanceOf<BroadcastGroup>().GetPaths(Sys).ShouldBe(new[] { "/user/myservice" });
             deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Settings.TotalInstances.ShouldBe(10000);
             deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Settings.AllowLocalRoutees.ShouldBe(false);
             deployment.RouterConfig.AsInstanceOf<ClusterRouterGroup>().Settings.UseRole.ShouldBe("backend");

src/core/Akka.Cluster/Routing/ClusterRoutingConfig.cs

@@ -52,10 +52,12 @@ public ClusterRouterGroupSettings(int totalInstances, bool allowLocalRoutees, st
         /// <summary>
         /// Initializes a new instance of the <see cref="ClusterRouterGroupSettings"/> class.
         /// </summary>
-        /// <param name="totalInstances">TBD</param>
-        /// <param name="routeesPaths">TBD</param>
-        /// <param name="allowLocalRoutees">TBD</param>
-        /// <param name="useRole">TBD</param>
+        /// <param name="totalInstances">The total number of routees. Defaults to 10000.</param>
+        /// <param name="routeesPaths">The actor selection paths to use for each routee.</param>
+        /// <param name="allowLocalRoutees">When <c>true</c>, allows routees to be deployed locally 
+        /// on the node doing the deploying so long as that node also 
+        /// satisfies the useRole setting when used.</param>
+        /// <param name="useRole">The role of the node upon which we are able to create routees.</param>
         /// <exception cref="ArgumentException">
         /// This exception is thrown when either the specified <paramref name="routeesPaths"/> is undefined
         /// or a path defined in the specified <paramref name="routeesPaths"/> is an invalid relative actor path.
@@ -79,7 +81,7 @@ public ClusterRouterGroupSettings(int totalInstances, IEnumerable<string> routee
         }
 
         /// <summary>
-        /// TBD
+        /// The paths of the routees to use on each qualified node.
         /// </summary>
         public IEnumerable<string> RouteesPaths { get; }
 
@@ -150,7 +152,7 @@ public ClusterRouterPoolSettings(int totalInstances, int maxInstancesPerNode, bo
         }
 
         /// <summary>
-        /// TBD
+        /// The maximum number of routee actors that can be deployed per valid node.
         /// </summary>
         public int MaxInstancesPerNode { get; }
 

src/core/Akka.Tests/Routing/ConfiguredLocalRoutingSpec.cs

@@ -234,7 +234,7 @@ public async Task RouterConfig_must_use_routeesPaths_from_config()
 
             routerConfig.Should().BeOfType<RandomGroup>();
             var randomGroup = (RandomGroup)routerConfig;
-            randomGroup.Paths.ShouldAllBeEquivalentTo(new List<string> { "/user/service1", "/user/service2" });
+            randomGroup.GetPaths(Sys).ShouldAllBeEquivalentTo(new List<string> { "/user/service1", "/user/service2" });
 
             var result = await actor.GracefulStop(3.Seconds());
             result.Should().BeTrue();

src/core/Akka/Routing/Broadcast.cs

@@ -301,7 +301,7 @@ public BroadcastGroup(IEnumerable<string> paths, string routerDispatcher) : base
         /// <returns>An enumeration of actor paths used during routee selection</returns>
         public override IEnumerable<string> GetPaths(ActorSystem system)
         {
-            return Paths;
+            return InternalPaths;
         }
 
         /// <summary>
@@ -325,7 +325,7 @@ public override Router CreateRouter(ActorSystem system)
         /// <returns>A new router with the provided dispatcher id.</returns>
         public Group WithDispatcher(string dispatcher)
         {
-            return new BroadcastGroup(Paths, dispatcher);
+            return new BroadcastGroup(InternalPaths, dispatcher);
         }
 
         /// <summary>
@@ -337,7 +337,7 @@ public override ISurrogate ToSurrogate(ActorSystem system)
         {
             return new BroadcastGroupSurrogate
             {
-                Paths = Paths,
+                Paths = InternalPaths,
                 RouterDispatcher = RouterDispatcher
             };
         }

src/core/Akka/Routing/ConsistentHashRouter.cs

@@ -696,7 +696,7 @@ public ConsistentHashingGroup(
         /// <returns>An enumeration of actor paths used during routee selection</returns>
         public override IEnumerable<string> GetPaths(ActorSystem system)
         {
-            return Paths;
+            return InternalPaths;
         }
 
         /// <summary>
@@ -722,7 +722,7 @@ public override Router CreateRouter(ActorSystem system)
         /// <returns>A new router with the provided dispatcher id.</returns>
         public ConsistentHashingGroup WithDispatcher(string dispatcher)
         {
-            return new ConsistentHashingGroup(Paths, VirtualNodesFactor, _hashMapping, dispatcher);
+            return new ConsistentHashingGroup(InternalPaths, VirtualNodesFactor, _hashMapping, dispatcher);
         }
 
         /// <summary>
@@ -736,7 +736,7 @@ public ConsistentHashingGroup WithDispatcher(string dispatcher)
         /// <returns>A new router with the provided <paramref name="vnodes" />.</returns>
         public ConsistentHashingGroup WithVirtualNodesFactor(int vnodes)
         {
-            return new ConsistentHashingGroup(Paths, vnodes, _hashMapping, RouterDispatcher);
+            return new ConsistentHashingGroup(InternalPaths, vnodes, _hashMapping, RouterDispatcher);
         }
 
         /// <summary>
@@ -750,7 +750,7 @@ public ConsistentHashingGroup WithVirtualNodesFactor(int vnodes)
         /// <returns>A new router with the provided <paramref name="mapping"/>.</returns>
         public ConsistentHashingGroup WithHashMapping(ConsistentHashMapping mapping)
         {
-            return new ConsistentHashingGroup(Paths, VirtualNodesFactor, mapping, RouterDispatcher);
+            return new ConsistentHashingGroup(InternalPaths, VirtualNodesFactor, mapping, RouterDispatcher);
         }
 
         /// <summary>
@@ -786,7 +786,7 @@ public override ISurrogate ToSurrogate(ActorSystem system)
         {
             return new ConsistentHashingGroupSurrogate
             {
-                Paths = Paths,
+                Paths = InternalPaths,
                 RouterDispatcher = RouterDispatcher
             };
         }

src/core/Akka/Routing/Random.cs

@@ -290,7 +290,7 @@ public RandomGroup(IEnumerable<string> paths, string routerDispatcher)
         /// <returns>An enumeration of actor paths used during routee selection</returns>
         public override IEnumerable<string> GetPaths(ActorSystem system)
         {
-            return Paths;
+            return InternalPaths;
         }
 
         /// <summary>
@@ -313,7 +313,7 @@ public override Router CreateRouter(ActorSystem system)
         /// <returns>A new router with the provided dispatcher id.</returns>
         public RandomGroup WithDispatcher(string dispatcher)
         {
-            return new RandomGroup(Paths, dispatcher);
+            return new RandomGroup(InternalPaths, dispatcher);
         }
 
         /// <summary>
@@ -325,7 +325,7 @@ public override ISurrogate ToSurrogate(ActorSystem system)
         {
             return new RandomGroupSurrogate
             {
-                Paths = Paths,
+                Paths = InternalPaths,
                 RouterDispatcher = RouterDispatcher
             };
         }

src/core/Akka/Routing/RoundRobin.cs

@@ -354,7 +354,7 @@ public RoundRobinGroup(IEnumerable<string> paths, string routerDispatcher)
         /// <returns>An enumeration of actor paths used during routee selection</returns>
         public override IEnumerable<string> GetPaths(ActorSystem system)
         {
-            return Paths;
+            return InternalPaths;
         }
 
         /// <summary>
@@ -377,7 +377,7 @@ public override Router CreateRouter(ActorSystem system)
         /// <returns>A new router with the provided dispatcher id.</returns>
         public Group WithDispatcher(string dispatcherId)
         {
-            return new RoundRobinGroup(Paths, dispatcherId);
+            return new RoundRobinGroup(InternalPaths, dispatcherId);
         }
 
         /// <summary>
@@ -389,7 +389,7 @@ public override ISurrogate ToSurrogate(ActorSystem system)
         {
             return new RoundRobinGroupSurrogate
             {
-                Paths = Paths,
+                Paths = InternalPaths,
                 RouterDispatcher = RouterDispatcher
             };
         }

src/core/Akka/Routing/RoutedActorCell.cs

@@ -167,7 +167,7 @@ public override void Start()
                 var deprecatedPaths = group.Paths;
 
                 var paths = deprecatedPaths == null
-                        ? group.GetPaths(System).ToArray()
+                        ? group.GetPaths(System)?.ToArray()
                         : deprecatedPaths.ToArray();
 
                 if (paths.NonEmpty())

src/core/Akka/Routing/RouterConfig.cs

@@ -145,13 +145,20 @@ public abstract class Group : RouterConfig, IEquatable<Group>
         /// <param name="routerDispatcher">TBD</param>
         protected Group(IEnumerable<string> paths, string routerDispatcher) : base(routerDispatcher)
         {
-            Paths = paths;
+            // equivalent of turning the paths into an immutable sequence
+            InternalPaths = paths?.ToArray() ?? new string[0];
         }
 
         /// <summary>
-        /// TBD
+        /// Internal property for holding the supplied paths
+        /// </summary>
+        protected readonly string[] InternalPaths;
+
+        /// <summary>
+        /// Retrieves the paths of all routees declared on this router.
         /// </summary>
-        public IEnumerable<string> Paths { get; }
+        [Obsolete("Deprecated since Akka.NET v1.1. Use Paths(ActorSystem) instead.")]
+        public IEnumerable<string> Paths => null;
 
         /// <summary>
         /// Retrieves the actor paths used by this router during routee selection.
@@ -194,7 +201,7 @@ public bool Equals(Group other)
         {
             if (ReferenceEquals(null, other)) return false;
             if (ReferenceEquals(this, other)) return true;
-            return Paths.SequenceEqual(other.Paths);
+            return InternalPaths.SequenceEqual(other.InternalPaths);
         }
 
         /// <inheritdoc/>
@@ -207,7 +214,7 @@ public override bool Equals(object obj)
         }
 
         /// <inheritdoc/>
-        public override int GetHashCode() => Paths?.GetHashCode() ?? 0;
+        public override int GetHashCode() => InternalPaths?.GetHashCode() ?? 0;
     }
 
     /// <summary>

src/core/Akka/Routing/ScatterGatherFirstCompleted.cs

@@ -441,7 +441,7 @@ public override Router CreateRouter(ActorSystem system)
         /// <returns>An enumeration of actor paths used during routee selection</returns>
         public override IEnumerable<string> GetPaths(ActorSystem system)
         {
-            return Paths;
+            return InternalPaths;
         }
 
         /// <summary>
@@ -454,7 +454,7 @@ public override IEnumerable<string> GetPaths(ActorSystem system)
         /// <returns>A new router with the provided dispatcher id.</returns>
         public ScatterGatherFirstCompletedGroup WithDispatcher(string dispatcher)
         {
-            return new ScatterGatherFirstCompletedGroup(Paths, Within, RouterDispatcher);
+            return new ScatterGatherFirstCompletedGroup(InternalPaths, Within, RouterDispatcher);
         }
 
         #region Surrogate
@@ -467,7 +467,7 @@ public override ISurrogate ToSurrogate(ActorSystem system)
         {
             return new ScatterGatherFirstCompletedGroupSurrogate
             {
-                Paths = Paths,
+                Paths = InternalPaths,
                 Within = Within,
                 RouterDispatcher = RouterDispatcher
             };

src/core/Akka/Routing/TailChopping.cs

@@ -456,7 +456,7 @@ public override Router CreateRouter(ActorSystem system)
         /// <returns>An enumeration of actor paths used during routee selection</returns>
         public override IEnumerable<string> GetPaths(ActorSystem system)
         {
-            return Paths;
+            return InternalPaths;
         }
 
         /// <summary>
@@ -469,7 +469,7 @@ public override IEnumerable<string> GetPaths(ActorSystem system)
         /// <returns>A new router with the provided dispatcher id.</returns>
         public TailChoppingGroup WithDispatcher(string dispatcher)
         {
-            return new TailChoppingGroup(Paths, Within, Interval, dispatcher);
+            return new TailChoppingGroup(InternalPaths, Within, Interval, dispatcher);
         }
 
         /// <summary>
@@ -481,7 +481,7 @@ public override ISurrogate ToSurrogate(ActorSystem system)
         {
             return new TailChoppingGroupSurrogate
             {
-                Paths = Paths,
+                Paths = InternalPaths,
                 Within = Within,
                 Interval = Interval,
                 RouterDispatcher = RouterDispatcher