Add nullable to DataProtection (#22591)

pranavkm · web-flow · commit f04992fab584 · 2020-06-13T20:39:41.000-07:00
* Add nullable to DataProtection Contributes to #5680
diff --git a/src/DataProtection/Abstractions/ref/Microsoft.AspNetCore.DataProtection.Abstractions.csproj b/src/DataProtection/Abstractions/ref/Microsoft.AspNetCore.DataProtection.Abstractions.csproj
@@ -3,6 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>netstandard2.0;$(DefaultNetCoreTargetFramework)</TargetFrameworks>
     <TargetFrameworks Condition="'$(DotNetBuildFromSource)' == 'true'">$(DefaultNetCoreTargetFramework)</TargetFrameworks>
+    <Nullable>annotations</Nullable>
   </PropertyGroup>
   <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
     <Compile Include="Microsoft.AspNetCore.DataProtection.Abstractions.netstandard2.0.cs" />
diff --git a/src/DataProtection/Abstractions/src/DataProtectionCommonExtensions.cs b/src/DataProtection/Abstractions/src/DataProtectionCommonExtensions.cs
@@ -88,7 +88,7 @@ public static IDataProtector CreateProtector(this IDataProtectionProvider provid
             // because we don't want the code provider.CreateProtector() [parameterless] to inadvertently compile.
             // The actual signature for this method forces at least one purpose to be provided at the call site.
 
-            IDataProtector protector = provider.CreateProtector(purpose);
+            IDataProtector? protector = provider.CreateProtector(purpose);
             if (subPurposes != null && subPurposes.Length > 0)
             {
                 protector = protector?.CreateProtector((IEnumerable<string>)subPurposes);
@@ -111,7 +111,7 @@ public static IDataProtectionProvider GetDataProtectionProvider(this IServicePro
 
             // We have our own implementation of GetRequiredService<T> since we don't want to
             // take a dependency on DependencyInjection.Interfaces.
-            IDataProtectionProvider provider = (IDataProtectionProvider)services.GetService(typeof(IDataProtectionProvider));
+            var provider = (IDataProtectionProvider?)services.GetService(typeof(IDataProtectionProvider));
             if (provider == null)
             {
                 throw new InvalidOperationException(Resources.FormatDataProtectionExtensions_NoService(typeof(IDataProtectionProvider).FullName));
diff --git a/src/DataProtection/Abstractions/src/Error.cs b/src/DataProtection/Abstractions/src/Error.cs
@@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.DataProtection
 {
     internal static class Error
     {
-        public static CryptographicException CryptCommon_GenericError(Exception inner = null)
+        public static CryptographicException CryptCommon_GenericError(Exception? inner = null)
         {
             return new CryptographicException(Resources.CryptCommon_GenericError, inner);
         }
diff --git a/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj b/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj
@@ -1,4 +1,4 @@
-﻿<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <Description>ASP.NET Core data protection abstractions.
@@ -9,6 +9,7 @@ Microsoft.AspNetCore.DataProtection.IDataProtector</Description>
     <IsAspNetCoreApp>true</IsAspNetCoreApp>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <PackageTags>aspnetcore;dataprotection</PackageTags>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/DataProtection/Cryptography.Internal/ref/Microsoft.AspNetCore.Cryptography.Internal.csproj b/src/DataProtection/Cryptography.Internal/ref/Microsoft.AspNetCore.Cryptography.Internal.csproj
@@ -3,6 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>netstandard2.0;$(DefaultNetCoreTargetFramework)</TargetFrameworks>
     <TargetFrameworks Condition="'$(DotNetBuildFromSource)' == 'true'">$(DefaultNetCoreTargetFramework)</TargetFrameworks>
+    <Nullable>annotations</Nullable>
   </PropertyGroup>
   <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
     <Compile Include="Microsoft.AspNetCore.Cryptography.Internal.netstandard2.0.cs" />
diff --git a/src/DataProtection/Cryptography.Internal/src/CryptoUtil.cs b/src/DataProtection/Cryptography.Internal/src/CryptoUtil.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Runtime.CompilerServices;
 using System.Runtime.ConstrainedExecution;
 using System.Runtime.InteropServices;
@@ -16,7 +17,7 @@ internal unsafe static class CryptoUtil
     {
         // This isn't a typical Debug.Assert; the check is always performed, even in retail builds.
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static void Assert(bool condition, string message)
+        public static void Assert([DoesNotReturnIf(false)] bool condition, string message)
         {
             if (!condition)
             {
diff --git a/src/DataProtection/Cryptography.Internal/src/Microsoft.AspNetCore.Cryptography.Internal.csproj b/src/DataProtection/Cryptography.Internal/src/Microsoft.AspNetCore.Cryptography.Internal.csproj
@@ -1,4 +1,4 @@
-﻿<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <Description>Infrastructure for ASP.NET Core cryptographic packages. Applications and libraries should not reference this package directly.</Description>
@@ -8,6 +8,7 @@
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <PackageTags>aspnetcore;dataprotection</PackageTags>
+    <Nullable>annotations</Nullable>
   </PropertyGroup>
 
 </Project>
diff --git a/src/DataProtection/Cryptography.KeyDerivation/ref/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj b/src/DataProtection/Cryptography.KeyDerivation/ref/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj
@@ -3,6 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>netstandard2.0;$(DefaultNetCoreTargetFramework)</TargetFrameworks>
     <TargetFrameworks Condition="'$(DotNetBuildFromSource)' == 'true'">$(DefaultNetCoreTargetFramework)</TargetFrameworks>
+    <Nullable>annotations</Nullable>
   </PropertyGroup>
   <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
     <Compile Include="Microsoft.AspNetCore.Cryptography.KeyDerivation.netstandard2.0.cs" />
diff --git a/src/DataProtection/Cryptography.KeyDerivation/src/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj b/src/DataProtection/Cryptography.KeyDerivation/src/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj
@@ -1,4 +1,4 @@
-﻿<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <Description>ASP.NET Core utilities for key derivation.</Description>
@@ -7,6 +7,8 @@
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <PackageTags>aspnetcore;dataprotection</PackageTags>
+    <Nullable>enable</Nullable>
+    <Nullable Condition="'$(TargetFramework)' == 'netstandard2.0'">annotations</Nullable>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/DataProtection/DataProtection/ref/Microsoft.AspNetCore.DataProtection.csproj b/src/DataProtection/DataProtection/ref/Microsoft.AspNetCore.DataProtection.csproj
@@ -3,6 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>netstandard2.0;$(DefaultNetCoreTargetFramework)</TargetFrameworks>
     <TargetFrameworks Condition="'$(DotNetBuildFromSource)' == 'true'">$(DefaultNetCoreTargetFramework)</TargetFrameworks>
+    <Nullable>annotations</Nullable>
   </PropertyGroup>
   <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
     <Compile Include="Microsoft.AspNetCore.DataProtection.netstandard2.0.cs" />
diff --git a/src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj b/src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj
@@ -9,6 +9,7 @@
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <PackageTags>aspnetcore;dataprotection</PackageTags>
+    <Nullable>annotations</Nullable>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/DataProtection/EntityFrameworkCore/src/DataProtectionKey.cs b/src/DataProtection/EntityFrameworkCore/src/DataProtectionKey.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 namespace Microsoft.AspNetCore.DataProtection.EntityFrameworkCore
@@ -16,11 +16,11 @@ public class DataProtectionKey
         /// <summary>
         /// The friendly name of the <see cref="DataProtectionKey"/>.
         /// </summary>
-        public string FriendlyName { get; set; }
+        public string? FriendlyName { get; set; }
 
         /// <summary>
         /// The XML representation of the <see cref="DataProtectionKey"/>.
         /// </summary>
-        public string Xml { get; set; }
+        public string? Xml { get; set; }
     }
 }
diff --git a/src/DataProtection/EntityFrameworkCore/src/EntityFrameworkCoreXmlRepository.cs b/src/DataProtection/EntityFrameworkCore/src/EntityFrameworkCoreXmlRepository.cs
@@ -40,13 +40,25 @@ public EntityFrameworkCoreXmlRepository(IServiceProvider services, ILoggerFactor
         /// <inheritdoc />
         public virtual IReadOnlyCollection<XElement> GetAllElements()
         {
-            using (var scope = _services.CreateScope())
+            // forces complete enumeration
+            return GetAllElementsCore().ToList().AsReadOnly();
+
+            IEnumerable<XElement> GetAllElementsCore()
             {
-                var context = scope.ServiceProvider.GetRequiredService<TContext>();
+                using (var scope = _services.CreateScope())
+                {
+                    var context = scope.ServiceProvider.GetRequiredService<TContext>();
+
+                    foreach (var key in context.DataProtectionKeys.AsNoTracking())
+                    {
+                        _logger.ReadingXmlFromKey(key.FriendlyName!, key.Xml);
 
-                // Put logger in a local such that `this` isn't captured.
-                var logger = _logger;
-                return context.DataProtectionKeys.AsNoTracking().Select(key => TryParseKeyXml(key.Xml, logger)).ToList().AsReadOnly();
+                        if (!string.IsNullOrEmpty(key.Xml))
+                        {
+                            yield return XElement.Parse(key.Xml);
+                        }
+                    }
+                }
             }
         }
 
@@ -67,18 +79,5 @@ public void StoreElement(XElement element, string friendlyName)
                 context.SaveChanges();
             }
         }
-
-        private static XElement TryParseKeyXml(string xml, ILogger logger)
-        {
-            try
-            {
-                return XElement.Parse(xml);
-            }
-            catch (Exception e)
-            {
-                logger?.LogExceptionWhileParsingKeyXml(xml, e);
-                return null;
-            }
-        }
     }
 }
diff --git a/src/DataProtection/EntityFrameworkCore/src/LoggingExtensions.cs b/src/DataProtection/EntityFrameworkCore/src/LoggingExtensions.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -7,23 +7,23 @@ namespace Microsoft.Extensions.Logging
 {
     internal static class LoggingExtensions
     {
-        private static readonly Action<ILogger, string, Exception> _anExceptionOccurredWhileParsingKeyXml;
-        private static readonly Action<ILogger, string, string, Exception> _savingKeyToDbContext;
+        private static readonly Action<ILogger, string?, string?, Exception?> _readingXmlFromKey;
+        private static readonly Action<ILogger, string, string, Exception?> _savingKeyToDbContext;
 
         static LoggingExtensions()
         {
-            _anExceptionOccurredWhileParsingKeyXml = LoggerMessage.Define<string>(
-                eventId: new EventId(1, "ExceptionOccurredWhileParsingKeyXml"),
-                logLevel: LogLevel.Warning,
-                formatString: "An exception occurred while parsing the key xml '{Xml}'.");
+            _readingXmlFromKey = LoggerMessage.Define<string?, string?>(
+                eventId: new EventId(1, "ReadKeyFromElement"),
+                logLevel: LogLevel.Debug,
+                formatString: "Reading data with key '{FriendlyName}', value '{Value}'.");
             _savingKeyToDbContext = LoggerMessage.Define<string, string>(
                 eventId: new EventId(2, "SavingKeyToDbContext"),
                 logLevel: LogLevel.Debug,
                 formatString: "Saving key '{FriendlyName}' to '{DbContext}'.");
         }
 
-        public static void LogExceptionWhileParsingKeyXml(this ILogger logger, string keyXml, Exception exception)
-            => _anExceptionOccurredWhileParsingKeyXml(logger, keyXml, exception);
+        public static void ReadingXmlFromKey(this ILogger logger, string? friendlyName, string? keyXml)
+            => _readingXmlFromKey(logger, friendlyName, keyXml, null);
 
         public static void LogSavingKeyToDbContext(this ILogger logger, string friendlyName, string contextName)
             => _savingKeyToDbContext(logger, friendlyName, contextName, null);
diff --git a/src/DataProtection/EntityFrameworkCore/src/Microsoft.AspNetCore.DataProtection.EntityFrameworkCore.csproj b/src/DataProtection/EntityFrameworkCore/src/Microsoft.AspNetCore.DataProtection.EntityFrameworkCore.csproj
@@ -1,4 +1,4 @@
-﻿<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <Description>Support for storing keys using Entity Framework Core.</Description>
@@ -7,6 +7,7 @@
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <PackageTags>aspnetcore;dataprotection;entityframeworkcore</PackageTags>
     <IsPackable>true</IsPackable>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/DataProtection/Extensions/ref/Microsoft.AspNetCore.DataProtection.Extensions.csproj b/src/DataProtection/Extensions/ref/Microsoft.AspNetCore.DataProtection.Extensions.csproj
@@ -3,6 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>netstandard2.0;$(DefaultNetCoreTargetFramework)</TargetFrameworks>
     <TargetFrameworks Condition="'$(DotNetBuildFromSource)' == 'true'">$(DefaultNetCoreTargetFramework)</TargetFrameworks>
+    <Nullable>annotations</Nullable>
   </PropertyGroup>
   <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
     <Compile Include="Microsoft.AspNetCore.DataProtection.Extensions.netstandard2.0.cs" />
diff --git a/src/DataProtection/Extensions/src/DataProtectionProvider.cs b/src/DataProtection/Extensions/src/DataProtectionProvider.cs
@@ -151,9 +151,9 @@ public static IDataProtectionProvider Create(
         }
 
         internal static IDataProtectionProvider CreateProvider(
-            DirectoryInfo keyDirectory,
+            DirectoryInfo? keyDirectory,
             Action<IDataProtectionBuilder> setupAction,
-            X509Certificate2 certificate)
+            X509Certificate2? certificate)
         {
             // build the service collection
             var serviceCollection = new ServiceCollection();
diff --git a/src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj b/src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj
@@ -1,4 +1,4 @@
-﻿<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <Description>Additional APIs for ASP.NET Core data protection.</Description>
@@ -7,6 +7,7 @@
     <IsAspNetCoreApp>true</IsAspNetCoreApp>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <PackageTags>aspnetcore;dataprotection</PackageTags>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/DataProtection/Extensions/src/TimeLimitedDataProtector.cs b/src/DataProtection/Extensions/src/TimeLimitedDataProtector.cs
@@ -17,7 +17,7 @@ internal sealed class TimeLimitedDataProtector : ITimeLimitedDataProtector
         private const string MyPurposeString = "Microsoft.AspNetCore.DataProtection.TimeLimitedDataProtector.v1";
 
         private readonly IDataProtector _innerProtector;
-        private IDataProtector _innerProtectorWithTimeLimitedPurpose; // created on-demand
+        private IDataProtector? _innerProtectorWithTimeLimitedPurpose; // created on-demand
 
         public TimeLimitedDataProtector(IDataProtector innerProtector)
         {
diff --git a/src/DataProtection/StackExchangeRedis/src/Microsoft.AspNetCore.DataProtection.StackExchangeRedis.csproj b/src/DataProtection/StackExchangeRedis/src/Microsoft.AspNetCore.DataProtection.StackExchangeRedis.csproj
@@ -1,4 +1,4 @@
-﻿<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <Description>Support for storing data protection keys in Redis.</Description>
@@ -7,6 +7,7 @@
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <PackageTags>aspnetcore;dataprotection;redis</PackageTags>
     <IsPackable>true</IsPackable>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/Shared/WebEncoders/WebEncoders.cs b/src/Shared/WebEncoders/WebEncoders.cs
@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 #nullable enable
-
 using System;
 #if NETCOREAPP
 using System.Buffers;

Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.DataProtection`
`9`	`9`	`{`
`10`	`10`	`internal static class Error`
`11`	`11`	`{`
`12`		`- public static CryptographicException CryptCommon_GenericError(Exception inner = null)`
	`12`	`+ public static CryptographicException CryptCommon_GenericError(Exception? inner = null)`
`13`	`13`	`{`
`14`	`14`	`return new CryptographicException(Resources.CryptCommon_GenericError, inner);`
`15`	`15`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`
`4`	`4`	`using System;`
`5`	`5`	`using System.Diagnostics;`
	`6`	`+using System.Diagnostics.CodeAnalysis;`
`6`	`7`	`using System.Runtime.CompilerServices;`
`7`	`8`	`using System.Runtime.ConstrainedExecution;`
`8`	`9`	`using System.Runtime.InteropServices;`
`@@ -16,7 +17,7 @@ internal unsafe static class CryptoUtil`
`16`	`17`	`{`
`17`	`18`	`// This isn't a typical Debug.Assert; the check is always performed, even in retail builds.`
`18`	`19`	`[MethodImpl(MethodImplOptions.AggressiveInlining)]`
`19`		`- public static void Assert(bool condition, string message)`
	`20`	`+ public static void Assert([DoesNotReturnIf(false)] bool condition, string message)`
`20`	`21`	`{`
`21`	`22`	`if (!condition)`
`22`	`23`	`{`