From ea1ee2b68c99fa696ae7efcc6cd04ad9ded10768 Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Wed, 14 Nov 2018 23:32:50 +0100 Subject: [PATCH] Do not check if key is present before removing item. (#1064) Use Nullable.GetValueOrDefault() instead of Nullable.Value when it is known to have a value. --- .../AuthenticationProperties.cs | 10 +- .../AuthenticationPropertiesTests.cs | 94 +++++++++++++++++++ 2 files changed, 99 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticationProperties.cs b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticationProperties.cs index 271329209a..ebb3995472 100644 --- a/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticationProperties.cs +++ b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticationProperties.cs @@ -122,7 +122,7 @@ namespace Microsoft.AspNetCore.Authentication { Items[key] = value; } - else if (Items.ContainsKey(key)) + else { Items.Remove(key); } @@ -169,9 +169,9 @@ namespace Microsoft.AspNetCore.Authentication { if (value.HasValue) { - Items[key] = value.Value.ToString(); + Items[key] = value.GetValueOrDefault().ToString(); } - else if (Items.ContainsKey(key)) + else { Items.Remove(key); } @@ -201,9 +201,9 @@ namespace Microsoft.AspNetCore.Authentication { if (value.HasValue) { - Items[key] = value.Value.ToString(UtcDateTimeFormat, CultureInfo.InvariantCulture); + Items[key] = value.GetValueOrDefault().ToString(UtcDateTimeFormat, CultureInfo.InvariantCulture); } - else if (Items.ContainsKey(key)) + else { Items.Remove(key); } diff --git a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationPropertiesTests.cs b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationPropertiesTests.cs index 639c9b558e..84db381ce4 100644 --- a/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationPropertiesTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Core.Test/AuthenticationPropertiesTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Linq; using Xunit; @@ -73,6 +74,10 @@ namespace Microsoft.AspNetCore.Authentication.Core.Test props.SetString("foo", null); Assert.Null(props.GetString("foo")); Assert.Equal(1, props.Items.Count); + + props.SetString("doesntexist", null); + Assert.False(props.Items.ContainsKey("doesntexist")); + Assert.Equal(1, props.Items.Count); } [Fact] @@ -203,5 +208,94 @@ namespace Microsoft.AspNetCore.Authentication.Core.Test props.Items.Clear(); Assert.Null(props.AllowRefresh); } + + [Fact] + public void SetDateTimeOffset() + { + var props = new MyAuthenticationProperties(); + + props.SetDateTimeOffset("foo", new DateTimeOffset(new DateTime(2018, 03, 19, 12, 34, 56, DateTimeKind.Utc))); + Assert.Equal("Mon, 19 Mar 2018 12:34:56 GMT", props.Items["foo"]); + + props.SetDateTimeOffset("foo", null); + Assert.False(props.Items.ContainsKey("foo")); + + props.SetDateTimeOffset("doesnotexist", null); + Assert.False(props.Items.ContainsKey("doesnotexist")); + } + + [Fact] + public void GetDateTimeOffset() + { + var props = new MyAuthenticationProperties(); + var dateTimeOffset = new DateTimeOffset(new DateTime(2018, 03, 19, 12, 34, 56, DateTimeKind.Utc)); + + props.Items["foo"] = dateTimeOffset.ToString("r", CultureInfo.InvariantCulture); + Assert.Equal(dateTimeOffset, props.GetDateTimeOffset("foo")); + + props.Items.Remove("foo"); + Assert.Null(props.GetDateTimeOffset("foo")); + + props.Items["foo"] = "BAR"; + Assert.Null(props.GetDateTimeOffset("foo")); + Assert.Equal("BAR", props.Items["foo"]); + } + + [Fact] + public void SetBool() + { + var props = new MyAuthenticationProperties(); + + props.SetBool("foo", true); + Assert.Equal(true.ToString(), props.Items["foo"]); + + props.SetBool("foo", false); + Assert.Equal(false.ToString(), props.Items["foo"]); + + props.SetBool("foo", null); + Assert.False(props.Items.ContainsKey("foo")); + } + + [Fact] + public void GetBool() + { + var props = new MyAuthenticationProperties(); + + props.Items["foo"] = true.ToString(); + Assert.True(props.GetBool("foo")); + + props.Items["foo"] = false.ToString(); + Assert.False(props.GetBool("foo")); + + props.Items["foo"] = null; + Assert.Null(props.GetBool("foo")); + + props.Items["foo"] = "BAR"; + Assert.Null(props.GetBool("foo")); + Assert.Equal("BAR", props.Items["foo"]); + } + + public class MyAuthenticationProperties : AuthenticationProperties + { + public new DateTimeOffset? GetDateTimeOffset(string key) + { + return base.GetDateTimeOffset(key); + } + + public new void SetDateTimeOffset(string key, DateTimeOffset? value) + { + base.SetDateTimeOffset(key, value); + } + + public new void SetBool(string key, bool? value) + { + base.SetBool(key, value); + } + + public new bool? GetBool(string key) + { + return base.GetBool(key); + } + } } }