Merge pull request #1459 from glopesdev/issue-1458

Require user defined conversion when multicasting to subject with non-matching type
bonsai-rx · Jul 6, 2023 · 4ada1bb · 4ada1bb
2 parents cab951e + 54ffac5
commit 4ada1bb
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 3 deletions.
diff --git a/Bonsai.Core.Tests/CombinatorBuilderTests.cs b/Bonsai.Core.Tests/CombinatorBuilderTests.cs
@@ -22,8 +22,10 @@ Expression CreateObservableExpression<TSource>(IObservable<TSource> source)
 
         IObservable<TSource> TestCombinatorBuilder<TSource>(object combinator, params Expression[] arguments)
         {
+            Expression buildResult;
             var builder = new CombinatorBuilder { Combinator = combinator };
-            var buildResult = builder.Build(arguments);
+            try { buildResult = builder.Build(arguments); }
+            catch (Exception ex) { throw new WorkflowBuildException(ex.Message, builder, ex); }
             var lambda = Expression.Lambda<Func<IObservable<TSource>>>(buildResult);
             var resultFactory = lambda.Compile();
             var result = resultFactory();

diff --git a/Bonsai.Core.Tests/OverloadedCombinatorBuilderTests.cs b/Bonsai.Core.Tests/OverloadedCombinatorBuilderTests.cs
@@ -188,8 +188,8 @@ public void Build_ListTupleOverloadedMethodCalledWithIntTuple_ReturnsIntValue()
         }
 
         [TestMethod]
-        [ExpectedException(typeof(InvalidOperationException))]
-        public void Build_AmbiguousOverloadedMethodCalledWithIntTuple_ThrowsInvalidOperationException()
+        [ExpectedException(typeof(WorkflowBuildException))]
+        public void Build_AmbiguousOverloadedMethodCalledWithIntTuple_ThrowsWorkflowBuildException()
         {
             var value = 5;
             var combinator = new AmbiguousOverloadedCombinatorMock();

diff --git a/Bonsai.Core.Tests/SubjectTests.cs b/Bonsai.Core.Tests/SubjectTests.cs
@@ -9,6 +9,30 @@ namespace Bonsai.Core.Tests
     [TestClass]
     public class SubjectTests
     {
+        [Combinator]
+        class TypeCombinatorMock<T> : Combinator<T, T>
+        {
+            public override IObservable<T> Process(IObservable<T> source)
+            {
+                return source;
+            }
+        }
+
+        [TestMethod]
+        [ExpectedException(typeof(WorkflowBuildException))]
+        public void Build_MulticastInterfaceToSubjectOfDifferentInterface_ThrowsBuildException()
+        {
+            var builder = new WorkflowBuilder();
+            builder.Workflow.Add(new BehaviorSubject<IDisposable> { Name = nameof(BehaviorSubject) });
+            var source = builder.Workflow.Add(new CombinatorBuilder { Combinator = new DoubleProperty { Value = 5.5 } });
+            var convert1 = builder.Workflow.Add(new CombinatorBuilder { Combinator = new TypeCombinatorMock<IComparable>() });
+            var convert2 = builder.Workflow.Add(new MulticastSubject { Name = nameof(BehaviorSubject) });
+            builder.Workflow.AddEdge(source, convert1, new ExpressionBuilderArgument());
+            builder.Workflow.AddEdge(convert1, convert2, new ExpressionBuilderArgument());
+            var expression = builder.Workflow.Build();
+            Assert.IsNotNull(expression);
+        }
+
         [TestMethod]
         public void ResourceSubject_SourceTerminatesExceptionally_ShouldNotTryToDispose()
         {

diff --git a/Bonsai.Core.Tests/TypeConversionTests.cs b/Bonsai.Core.Tests/TypeConversionTests.cs
@@ -0,0 +1,58 @@
+using System;
+using System.Reactive.Linq;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Bonsai.Core.Tests
+{
+    public partial class CombinatorBuilderTests
+    {
+        [Combinator]
+        class TypeCombinatorMock<T> : Combinator<T, T>
+        {
+            public override IObservable<T> Process(IObservable<T> source)
+            {
+                return source;
+            }
+        }
+
+        private void BuildConvertCast<TSource, TResult>(TSource value, out TResult result)
+        {
+            var combinator = new TypeCombinatorMock<TResult>();
+            var source = CreateObservableExpression(Observable.Return(value));
+            var resultProvider = TestCombinatorBuilder<TResult>(combinator, source);
+            result = Last(resultProvider).Result;
+        }
+
+        [TestMethod]
+        public void Build_ConvertDoubleToInt_ReturnsTruncatedValue()
+        {
+            var value = 5.5;
+            BuildConvertCast(value, out int result);
+            Assert.AreEqual((int)value, result);
+        }
+
+        [TestMethod]
+        public void Build_ConvertObjectToImplementedInterface_ReturnsValidObject()
+        {
+            BuildConvertCast(5.5, out IComparable result);
+            Assert.IsNotNull(result);
+        }
+
+        [TestMethod]
+        [ExpectedException(typeof(WorkflowBuildException))]
+        public void Build_ConvertObjectToNotImplementedInterface_ThrowsBuildException()
+        {
+            BuildConvertCast(5.5, out IDisposable result);
+            Assert.IsNotNull(result);
+        }
+
+        [TestMethod]
+        [ExpectedException(typeof(WorkflowBuildException))]
+        public void Build_ConvertInterfaceToDifferentInterface_ThrowsBuildException()
+        {
+            BuildConvertCast(5.5, out IComparable result);
+            BuildConvertCast(result, out IDisposable disposable);
+            Assert.AreSame((IDisposable)result, disposable);
+        }
+    }
+}
diff --git a/Bonsai.Core/Expressions/MulticastSubject.cs b/Bonsai.Core/Expressions/MulticastSubject.cs
@@ -60,6 +60,13 @@ public override Expression Build(IEnumerable<Expression> arguments)
             var subjectType = subjectExpression.Type.GetGenericArguments()[0];
             if (observableType != subjectType)
             {
+                if (!HasConversion(observableType, subjectType))
+                {
+                    throw new InvalidOperationException(
+                        $"No coercion operator is defined between types '{observableType}' and '{subjectType}'."
+                    );
+                }
+
                 source = CoerceMethodArgument(typeof(IObservable<>).MakeGenericType(subjectType), source);
                 observableType = subjectType;
             }