From a3ec19e24e79fc717a92660bfd46d097dc11a70f Mon Sep 17 00:00:00 2001 From: Philip Lin Date: Tue, 16 Oct 2018 22:33:41 -0700 Subject: [PATCH] Option.Some method should not allow null value (#445) * add validation to ensure null should not be allowed when calling Option.Some method. --- .../reporters/IoTHubReporterTest.cs | 8 ++++++++ .../endpoints/AsyncEndpointExecutorTest.cs | 2 +- .../Microsoft.Azure.Devices.Edge.Util/Option.cs | 14 +++++++++----- .../OptionTest.cs | 13 +++++++------ 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/edge-agent/test/Microsoft.Azure.Devices.Edge.Agent.IoTHub.Test/reporters/IoTHubReporterTest.cs b/edge-agent/test/Microsoft.Azure.Devices.Edge.Agent.IoTHub.Test/reporters/IoTHubReporterTest.cs index 7525ee4dac2..4031fa352a4 100644 --- a/edge-agent/test/Microsoft.Azure.Devices.Edge.Agent.IoTHub.Test/reporters/IoTHubReporterTest.cs +++ b/edge-agent/test/Microsoft.Azure.Devices.Edge.Agent.IoTHub.Test/reporters/IoTHubReporterTest.cs @@ -1163,6 +1163,7 @@ public async void ReportShutdown() // build current module set ModuleSet currentModuleSet = ModuleSet.Create( edgeAgent, + edgeHub, new TestRuntimeModule( "mod1", "1.0", RestartPolicy.OnUnhealthy, "test", ModuleStatus.Running, new TestConfig("image1"), 0, string.Empty, DateTime.MinValue, DateTime.MinValue, @@ -1202,6 +1203,13 @@ public async void ReportShutdown() { runtimeStatus = "unknown" } + }, + { + edgeHub.Name, + new + { + runtimeStatus = "unknown" + } } }, modules = new Dictionary diff --git a/edge-hub/test/Microsoft.Azure.Devices.Routing.Core.Test/endpoints/AsyncEndpointExecutorTest.cs b/edge-hub/test/Microsoft.Azure.Devices.Routing.Core.Test/endpoints/AsyncEndpointExecutorTest.cs index 93c9e957db6..1189551e8fa 100644 --- a/edge-hub/test/Microsoft.Azure.Devices.Routing.Core.Test/endpoints/AsyncEndpointExecutorTest.cs +++ b/edge-hub/test/Microsoft.Azure.Devices.Routing.Core.Test/endpoints/AsyncEndpointExecutorTest.cs @@ -53,7 +53,7 @@ public async Task SmokeTest() { await executor.Invoke(msg); } - await Task.Delay(20); + await Task.Delay(30); await executor.CloseAsync(); Assert.Equal(3, endpoint.N); Assert.Equal(expected, endpoint.Processed); diff --git a/edge-util/src/Microsoft.Azure.Devices.Edge.Util/Option.cs b/edge-util/src/Microsoft.Azure.Devices.Edge.Util/Option.cs index ce35d840b72..ecc2db3c217 100644 --- a/edge-util/src/Microsoft.Azure.Devices.Edge.Util/Option.cs +++ b/edge-util/src/Microsoft.Azure.Devices.Edge.Util/Option.cs @@ -142,8 +142,8 @@ public void ForEach(Action action) /// /// If this option has a value then it transforms it into a new option instance by - /// calling the callback. Returns - /// if there is no value. + /// calling the callback. It will follow exception if callback returns null. + /// Returns if there is no value. /// [Pure] public Option Map(Func mapping) @@ -203,7 +203,12 @@ public static class Option /// Creates an Option <T> with and marks /// the option object as having a value, i.e., Option<T>.HasValue == true. /// - public static Option Some(T value) => new Option(value, true); + public static Option Some(T value) + { + Preconditions.CheckNotNull(value, nameof(value)); + + return new Option(value, true); + } /// /// Creates an Option <T> with a default value (default(T)) and marks @@ -211,7 +216,6 @@ public static class Option /// public static Option None() => new Option(default(T), false); - public static Option Maybe(T value) where T : class => - value == null ? None() : Some(value); + public static Option Maybe(T value) where T : class => value == null ? None() : Some(value); } } diff --git a/edge-util/test/Microsoft.Azure.Devices.Edge.Util.Test/OptionTest.cs b/edge-util/test/Microsoft.Azure.Devices.Edge.Util.Test/OptionTest.cs index 53b0029d93c..2fc6575e6cb 100644 --- a/edge-util/test/Microsoft.Azure.Devices.Edge.Util.Test/OptionTest.cs +++ b/edge-util/test/Microsoft.Azure.Devices.Edge.Util.Test/OptionTest.cs @@ -58,11 +58,11 @@ public void TestHashCode() { Option some1 = Option.Some(1); Option some2 = Option.Some(2); - Option some3 = Option.Some(null); + Option some3 = Option.None(); Option none = Option.None(); Assert.Equal(0, none.GetHashCode()); - Assert.Equal(1, some3.GetHashCode()); + Assert.Equal(0, some3.GetHashCode()); Assert.NotEqual(some1.GetHashCode(), some2.GetHashCode()); } @@ -71,11 +71,11 @@ public void TestHashCode() public void TestToString() { Option some1 = Option.Some(1); - Option some2 = Option.Some(null); + Option some2 = Option.Some(new object()); Option none = Option.None(); Assert.Equal("Some(1)", some1.ToString()); - Assert.Equal("Some(null)", some2.ToString()); + Assert.Equal("Some(System.Object)", some2.ToString()); Assert.Equal("None", none.ToString()); } @@ -111,12 +111,13 @@ public void TestContains() { Option some = Option.Some(3); Option none = Option.None(); - Option some2 = Option.Some(null); + var value = new object(); + Option some2 = Option.Some(value); Assert.True(some.Contains(3)); Assert.False(some.Contains(2)); Assert.False(none.Contains(3)); - Assert.True(some2.Contains(null)); + Assert.True(some2.Contains(value)); } [Fact]