fix: ensure synchronous command execution is lazy (#1274)

fixes #1244
reactiveui · Feb 20, 2017 · d9d36e5 · d9d36e5
1 parent 427885e
commit d9d36e5
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 19 deletions.
diff --git a/src/ReactiveUI.Tests/ObservedChangedMixinTest.cs b/src/ReactiveUI.Tests/ObservedChangedMixinTest.cs
@@ -168,7 +168,7 @@ public void CanAddUpToSevenPlayers()
         {
             foreach (var i in Enumerable.Range(1, 7)) {
                 viewmodel.NewPlayerName = "Player" + i;
-                viewmodel.AddPlayer.Execute();
+                viewmodel.AddPlayer.Execute().Subscribe();
                 Assert.Equal(i, viewmodel.Players.Count);
             }
         }

diff --git a/src/ReactiveUI.Tests/ReactiveCommandTest.cs b/src/ReactiveUI.Tests/ReactiveCommandTest.cs
@@ -218,18 +218,77 @@ public void IsExecutingRemainsTrueAsLongAsExecutionPipelineHasNotCompleted()
         }
 
         [Fact]
-        public void ExecuteOnlyExecutesOnceRegardlessOfNumberOfSubscribers()
+        public void SynchronousCommandExecuteLazily()
         {
             var executionCount = 0;
-            var fixture = ReactiveCommand.Create(() => { ++executionCount; });
-            var execute = fixture.Execute();
+            var fixture1 = ReactiveCommand.Create(() => { ++executionCount; });
+            var fixture2 = ReactiveCommand.Create<int>(_ => { ++executionCount; });
+            var fixture3 = ReactiveCommand.Create(() => { ++executionCount; return 42; });
+            var fixture4 = ReactiveCommand.Create<int, int>(_ => { ++executionCount; return 42; });
+            var execute1 = fixture1.Execute();
+            var execute2 = fixture2.Execute();
+            var execute3 = fixture3.Execute();
+            var execute4 = fixture4.Execute();
+
+            Assert.Equal(0, executionCount);
+
+            execute1.Subscribe();
+            Assert.Equal(1, executionCount);
+
+            execute2.Subscribe();
+            Assert.Equal(2, executionCount);
 
-            execute.Subscribe();
-            execute.Subscribe();
-            execute.Subscribe();
-            execute.Subscribe();
+            execute3.Subscribe();
+            Assert.Equal(3, executionCount);
 
-            Assert.Equal(1, executionCount);
+            execute4.Subscribe();
+            Assert.Equal(4, executionCount);
+        }
+
+        [Fact]
+        public void SynchronousCommandsFailCorrectly()
+        {
+            var fixture1 = ReactiveCommand.Create(() => { throw new InvalidOperationException(); });
+            var fixture2 = ReactiveCommand.Create<int>(_ => { throw new InvalidOperationException(); });
+            var fixture3 = ReactiveCommand.Create(() => { throw new InvalidOperationException(); });
+            var fixture4 = ReactiveCommand.Create<int, int>(_ => { throw new InvalidOperationException(); });
+
+            var failureCount = 0;
+            Observable
+                .Merge(
+                    fixture1.ThrownExceptions,
+                    fixture2.ThrownExceptions,
+                    fixture3.ThrownExceptions,
+                    fixture4.ThrownExceptions)
+                .Subscribe(_ => ++failureCount);
+
+            fixture1
+                .Execute()
+                .Subscribe(
+                    _ => { },
+                    _ => { });
+            Assert.Equal(1, failureCount);
+
+            fixture2
+                .Execute()
+                .Subscribe(
+                    _ => { },
+                    _ => { });
+            Assert.Equal(2, failureCount);
+
+            fixture3
+                .Execute()
+                .Subscribe(
+                    _ => { },
+                    _ => { });
+            Assert.Equal(3, failureCount);
+
+            fixture4
+                .Execute()
+                .Subscribe(
+                    _ => { },
+                    _ => { });
+            Assert.Equal(4, failureCount);
         }
 
         [Fact]

diff --git a/src/ReactiveUI/ReactiveCommand.cs b/src/ReactiveUI/ReactiveCommand.cs
@@ -88,10 +88,13 @@ public static ReactiveCommand<Unit, Unit> Create(
             }
 
             return new ReactiveCommand<Unit, Unit>(
-                _ => {
-                    execute();
-                    return Observable.Return(Unit.Default);
-                },
+                _ => Observable.Create<Unit>(
+                    observer => {
+                        execute();
+                        observer.OnNext(Unit.Default);
+                        observer.OnCompleted();
+                        return Disposable.Empty;
+                    }),
                 canExecute ?? Observable.Return(true),
                 outputScheduler ?? RxApp.MainThreadScheduler);
         }
@@ -125,7 +128,12 @@ public static ReactiveCommand<Unit, TResult> Create<TResult>(
             }
 
             return new ReactiveCommand<Unit, TResult>(
-                _ => Observable.Return(execute()),
+                _ => Observable.Create<TResult>(
+                    observer => {
+                        observer.OnNext(execute());
+                        observer.OnCompleted();
+                        return Disposable.Empty;
+                    }),
                 canExecute ?? Observable.Return(true),
                 outputScheduler ?? RxApp.MainThreadScheduler);
         }
@@ -158,10 +166,13 @@ public static ReactiveCommand<TParam, Unit> Create<TParam>(
             }
 
             return new ReactiveCommand<TParam, Unit>(
-                param => {
-                    execute(param);
-                    return Observable.Return(Unit.Default);
-                },
+                param => Observable.Create<Unit>(
+                    observer => {
+                        execute(param);
+                        observer.OnNext(Unit.Default);
+                        observer.OnCompleted();
+                        return Disposable.Empty;
+                    }),
                 canExecute ?? Observable.Return(true),
                 outputScheduler ?? RxApp.MainThreadScheduler);
         }
@@ -198,7 +209,12 @@ public static ReactiveCommand<TParam, TResult> Create<TParam, TResult>(
             }
 
             return new ReactiveCommand<TParam, TResult>(
-                param => Observable.Return(execute(param)),
+                param => Observable.Create<TResult>(
+                    observer => {
+                        observer.OnNext(execute(param));
+                        observer.OnCompleted();
+                        return Disposable.Empty;
+                    }),
                 canExecute ?? Observable.Return(true),
                 outputScheduler ?? RxApp.MainThreadScheduler);
         }