Porting Feedback from MR (#23523)

* address ch1 feedback

* ch2 feedback

* ch4 updates

* remove static variable from Startup

* Update docs/architecture/porting-existing-aspnet-apps/configuration-differences.md

Co-authored-by: David Pine <[email protected]>

* Update docs/architecture/porting-existing-aspnet-apps/middleware-modules-handlers.md

Co-authored-by: David Pine <[email protected]>

* Update docs/architecture/porting-existing-aspnet-apps/understand-update-dependencies.md

Co-authored-by: David Pine <[email protected]>

Co-authored-by: David Pine <[email protected]>

Author: ardalis

docs/architecture/porting-existing-aspnet-apps/configuration-differences.md

@@ -54,7 +54,7 @@ public class TestModel : PageModel
 
 **Figure 2-2.** Accessing configuration values with `IConfiguration`.
 
-Using the options pattern, settings access is similar but is strongly typed and more specific to the setting(s) needed by the consuming class, as Figure 2-3 demonstrates.
+Using the [options pattern](/dotnet/core/extensions/options), settings access is similar but is strongly typed and more specific to the setting(s) needed by the consuming class, as Figure 2-3 demonstrates.
 
 ```csharp
 public class PositionOptions
@@ -92,7 +92,7 @@ services.Configure<PositionOptions>(Configuration.GetSection(PositionOptions.Pos
 
 ## Migrate configuration
 
-When considering how to port an app's configuration settings from .NET Framework to .NET Core, the first step is to identify all of the configuration settings that are being used. Most of these will be in the *web.config* file in the app's root folder, but some apps expect settings to be found in the shared *machine.config* file as well.
+When considering how to port an app's configuration settings from .NET Framework to .NET Core, the first step is to identify all of the configuration settings that are being used. Most of these will be in the *web.config* file in the app's root folder, but some apps expect settings to be found in the shared *machine.config* file as well. These settings will include elements of the `appSettings` element, the `connectionStrings` element, and any custom configuration elements as well. In .NET Core, all of these settings are typically stored in the *appsettings.json* file.
 
 Once all settings in the config files have been cataloged, the next step should be to identify where and how the settings are used in the app itself. If some settings aren't being used, these can probably be omitted from the migration. For each setting, note all of the places it's being used so you can be sure you don't miss any when you migrate the code.
 

docs/architecture/porting-existing-aspnet-apps/deployment-strategies.md

@@ -16,7 +16,7 @@ Because .NET Core runs on Linux, you'll find some hosting options available that
 * Your organization has infrastructure or expertise.
 * Hosting providers offer attractive features or pricing for Linux-based hosting.
 
-.NET Core opens to door to these options.
+.NET Core opens the door to these options.
 
 ## Cloud native development
 

docs/architecture/porting-existing-aspnet-apps/example-migration-eshop.md

@@ -53,7 +53,7 @@ Most of the incompatible types refer to `Controller` and various related attribu
 
 ## Update project files and NuGet reference syntax
 
-Next, migrate from the older *.csproj* file structure to the newer, simpler structure introduced with .NET Core. In doing so, you'll also migrate from using a *packages.config* file for NuGet references to using `<PackageReference>` elements in the project file.
+Next, migrate from the older *.csproj* file structure to the newer, simpler structure introduced with .NET Core. In doing so, you'll also migrate from using a *packages.config* file for NuGet references to using `<PackageReference>` elements in the project file. Old-style project files may also use `<PackageReference>` elements, so it usually makes sense to migrate all NuGet package references to this format first, before upgrading to the new project file format.
 
 The original project's *eShopLegacyMVC.csproj* file is 418 lines long. A sample of the project file is shown in Figure 4-6. To offer a sense of its overall size and complexity, the right side of the image contains a miniature view of the entire file.
 
@@ -69,7 +69,7 @@ In addition to the C# project file, NuGet dependencies are stored in a separate
 
 **Figure 4-7.** The *packages.config* file.
 
-After upgrading to the new *.csproj* file format, you can migrate *packages.config* in class library projects using Visual Studio. This functionality doesn't work with ASP.NET projects, however. [Learn more about migrating *packages.config* to `<PackageReference>` in Visual Studio](/nuget/consume-packages/migrate-packages-config-to-package-reference). If you have a large number of projects to migrate, [this community tool may help](https://github.com/MarkKharitonov/NuGetPCToPRMigrator).
+You can migrate *packages.config* in class library projects using Visual Studio. This functionality doesn't work with ASP.NET projects, however. [Learn more about migrating *packages.config* to `<PackageReference>` in Visual Studio](/nuget/consume-packages/migrate-packages-config-to-package-reference). If you have a large number of projects to migrate, [this community tool may help](https://github.com/MarkKharitonov/NuGetPCToPRMigrator). If you're using a tool to migrate the project file to the new format, you should do that after you've finished migrating all NuGet references to use `<PackageReverence>`.
 
 ## Create new ASP.NET Core project
 
@@ -307,20 +307,13 @@ Building again reveals one more error loading jQuery Validation on the *Create*
 <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery-validate/1.17.0/jquery.validate.min.js" integrity="sha512-O/nUTF5mdFkhEoQHFn9N5wmgYyW323JO6v8kr6ltSRKriZyTr/8417taVWeabVS4iONGk2V444QD0P2cwhuTkg==" crossorigin="anonymous"></script>
 ```
 
-The last thing to fix in the views is the reference to `Session` to display how long the app has been running, and on which machine. We can collect this data in `Startup` as static variables and display the variables on the layout page. Add the following properties to *Startup.cs*:
-
-```csharp
-public static DateTime StartTime { get; } = DateTime.UtcNow;
-public static string MachineName { get; } = Environment.MachineName;
-```
-
-Then replace the content of the footer in the layout with the following code:
+The last thing to fix in the views is the reference to `Session` to display how long the app has been running, and on which machine. We can display this data directly in the site's *_Layout.cshtml* by using `System.Environment.MachineName` and `System.Diagnostics.Process.GetCurrentProcess().StartTime`:
 
 ```razor
 <section class="col-sm-6">
     <img class="esh-app-footer-text hidden-xs" src="~/images/main_footer_text.png" width="335" height="26" alt="footer text image" />
     <br />
-<small>@eShopPorted.Startup.MachineName - @eShopPorted.Startup.StartTime.ToString() UTC</small>
+<small>@Environment.MachineName - @System.Diagnostics.Process.GetCurrentProcess().StartTime.ToString() UTC</small>
 </section>
 ```
 
@@ -442,7 +435,7 @@ public void ConfigureServices(IServiceCollection services)
 }
 ```
 
-The preceding code is the minimal configuration required to get MVC features working. There are many additional features that can be configured from this call, but for now this will suffice to build the app. Running it now routes the default request properly, but since we've not yet configured DI, an error occurs while activating `CatalogController`, because no implementation of type `ICatalogService` has been provided yet. We'll return to configure MVC further in a moment. For now, let's migrate the app's dependency injection.
+The preceding code is the minimal configuration required to get MVC features working. There are many additional features that can be configured from this call (some of which are detailed later in this chapter), but for now this will suffice to build the app. Running it now routes the default request properly, but since we've not yet configured DI, an error occurs while activating `CatalogController`, because no implementation of type `ICatalogService` has been provided yet. We'll return to configure MVC further in a moment. For now, let's migrate the app's dependency injection.
 
 #### Migrate dependency injection configuration
 

docs/architecture/porting-existing-aspnet-apps/hosting-differences.md

@@ -11,11 +11,12 @@ ASP.NET MVC apps are hosted in IIS, and may rely on IIS configuration for their
 
 [ASP.NET Core apps can run on a number of different servers](/aspnet/core/fundamentals/servers/). The default cross platform server, Kestrel, is a good default choice. Apps running in Kestrel can be hosted by IIS, running in a separate process. On Windows, apps can also run in process on IIS or using HTTP.sys. Kestrel is generally recommended for best performance. HTTP.sys can be used in scenarios where the app is exposed to the Internet and required capabilities are supported by HTTP.sys but not Kestrel.
 
-Kestrel does not have an equivalent to IIS modules (though apps hosted in IIS can still take advantage of IIS modules). To achieve equivalent behavior, middleware configured in the ASP.NET Core app itself is typically used.
+Kestrel does not have an equivalent to IIS modules (though apps hosted in IIS can still take advantage of IIS modules). To achieve equivalent behavior, [middleware](/aspnet/core/fundamentals/middleware/) configured in the ASP.NET Core app itself is typically used.
 
 ## References
 
 - [IIS Modules](/iis/get-started/introduction-to-iis/iis-modules-overview)
+- [ASP.NET Core Middleware](/aspnet/core/fundamentals/middleware/)
 - [ASP.NET Core Servers](/aspnet/core/fundamentals/servers/)
 
 >[!div class="step-by-step"]

docs/architecture/porting-existing-aspnet-apps/identify-migration-sequence.md

@@ -56,8 +56,8 @@ Watch an overview of how to employ this approach in this [dotNetConf presentatio
 
 - Migrate third-party NuGet dependencies
 - Migrate apps to use new *.csproj* file format
-- Migrate apps to ASP.NET Core (targeting .NET Framework)
 - Update internal NuGet dependencies to .NET Standard
+- Migrate apps to ASP.NET Core (targeting .NET Framework)
 - Update all apps to target .NET Core 3.1
 
 When automating a large suite of apps, it helps significantly if they follow consistent coding guidelines and project organization. Automation efforts rely on this consistency to be effective. In addition to parsing and migrating project files, common code patterns can be migrated automatically. Some code pattern examples include differences in how controller actions are declared or how they return results.

docs/architecture/porting-existing-aspnet-apps/incremental-migration-strategies.md

@@ -7,12 +7,20 @@ ms.date: 11/13/2020
 
 # Strategies for migrating incrementally
 
-The biggest challenge with migrating any large app is determining how to break the process into smaller tasks. There are several strategies that can be applied for this purpose, including breaking the app into horizontal layers such as UI, data access, business logic, or breaking up the app into separate, smaller apps. Another strategy is to upgrade some or all of the app to different framework versions on the way to a recent .NET Core release. One approach you could use, is to migrate [vertical slices](https://deviq.com/practices/vertical-slices) of the app, rather than attempting to migrate one horizontal layer at a time. Let's consider each of these different approaches.
+The biggest challenge with migrating any large app is determining how to break the process into smaller tasks. There are several strategies that can be applied for this purpose, including breaking the app into horizontal layers such as UI, data access, business logic, or breaking up the app into separate, smaller apps. Another strategy is to upgrade some or all of the app to different framework versions on the way to a recent .NET Core release. One approach you could use is to migrate [vertical slices](https://deviq.com/practices/vertical-slices) of the app, rather than attempting to migrate one horizontal layer at a time. Let's consider each of these different approaches.
 
-Consider the challenge of migrating a large ASP.NET 4.5 app. One approach is to migrate the entire app directly from .NET Framework 4.5 to .NET Core 3.1. However, this approach needs to account for every breaking change between the two frameworks and versions, which are substantial.
+## Migrating slice by slice
+
+One successful approach to migrating is to identify vertical slices of functionality and migrate them to the target platform one by one. The first step is to create a new ASP.NET Core 3.1 or 5 app. Next, identify the individual page or API endpoint that will be migrated first. Build out just the necessary functionality to support this one route in the ASP.NET Core app. Then use HTTP rewriting and/or a reverse proxy to start sending requests for these pages or endpoints to the new app rather than the ASP.NET app. This approach is well-suited to API projects, but can also work for many MVC apps.
+
+When migrating slice by slice, the entire stack of the individual API endpoint or requested route is recreated in the new project or solution. The very first such slice typically requires the most effort, since it will typically need several projects to be created and decisions to be made about data access and solution organization. Once the first slice's functionality mirrors the existing app's, it can be deployed and the existing app can redirect to it or simply be removed. This approach is then repeated until the entire app has been ported to the new structure.
+
+Some specific guidance on how to follow this strategy using IIS is covered in [Chapter 5, Deployment Scenarios](deployment-scenarios.md).
 
 ## Migrating layer by layer
 
+Consider the challenge of migrating a large ASP.NET 4.5 app. One approach is to migrate the entire app directly from .NET Framework 4.5 to .NET Core 3.1. However, this approach needs to account for every breaking change between the two frameworks and versions, which are substantial. Performing this work on one project at a time provides a set of stepping stones so that the entire solution doesn't need to be moved at once.
+
 One recent addition to the .NET ecosystem that helps with interoperability between different .NET frameworks is [.NET Standard](https://dotnet.microsoft.com/platform/dotnet-standard). .NET Standard allows libraries to build against the agreed upon set of common APIs, ensuring they can be used in any .NET app. .NET Standard 2.0 is notable because it covers most base class library functionality used by most .NET Framework and .NET Core apps. Unfortunately, the earliest version of .NET with support for .NET Standard 2.0 is .NET Framework 4.6.1, and there are a number of updates in .NET Framework 4.8 that make it a compelling choice for initial upgrades.
 
 One approach to incrementally upgrade a .NET Framework 4.5 system layer-by-layer is to first update its class library dependencies to .NET Framework 4.8. Then, modify these libraries to be .NET Standard class libraries. Use multi-targeting and conditional compilation, if necessary. This step can be helpful in scenarios where app dependencies require .NET Framework and cannot easily be ported directly to use .NET Standard and .NET Core. Since .NET Framework libraries can be consumed by ASP.NET Core 2.1 apps, the next step is to migrate some or all of the web functionality of the app to ASP.NET Core 2.1 (as described in the [previous chapter](choose-net-core-version.md)). This is a "bottom up" approach, starting with low level class library dependencies and working up to the web app entry point.
@@ -25,12 +33,6 @@ Instead of a "bottom up" approach, another alternative is to start with the web
 
 Instead of a "bottom up" approach, another alternative is to start with the web app (or even the entire solution) and use an automated tool to assist with the upgrade. The [.NET Upgrade Assistant tool](https://aka.ms/dotnet-upgrade-assistant) can be used to help upgrade .NET Framework apps to .NET Core / .NET 5. It automates many of the common tasks related to upgrading apps, such as modifying project file format, setting appropriate target frameworks, updating NuGet dependencies, and more.
 
-## Migrating slice by slice
-
-Another approach to the migration would be to identify vertical slices of functionality, and migrate them to the target platform one by one. The first step would be to create a new ASP.NET Core 3.1 or 5 app. Next, identify the individual page or API endpoint that will be migrated first. Build out just the necessary functionality to support this one route in the ASP.NET Core app. Then use HTTP rewriting and/or a reverse proxy to start sending requests for these pages or endpoints to the new app rather than the ASP.NET app.
-
-Some specific guidance on how to follow this strategy using IIS is covered in [Chapter 5, Deployment Scenarios](deployment-scenarios.md).
-
 ## References
 
 - [What is .NET Standard?](https://dotnet.microsoft.com/platform/dotnet-standard)

docs/architecture/porting-existing-aspnet-apps/middleware-modules-handlers.md

@@ -21,6 +21,63 @@ ASP.NET Core defines a request pipeline in each app's `Configure` method. This r
 
 Behavior in an ASP.NET MVC app that uses HTTP modules is probably best suited to [custom middleware](/aspnet/core/fundamentals/middleware/?preserve-view=true&view=aspnetcore-3.1). Custom HTTP handlers can be replaced with custom routes or endpoints that respond to the same path.
 
+## Accessing HttpContext
+
+Many .NET apps reference the current request's context through `HttpContext.Current`. This static access can be a common source of problems with testing and other code usage outside of individual requests. When building ASP.NET Core apps, access to the current HttpContext should be provided as a method parameter on middleware, as this sample demonstrates:
+
+```csharp
+public class Middleware
+{
+    private readonly RequestDelegate _next;
+
+    public Middleware(RequestDelegate next)
+    {
+        _next = next;
+    }
+
+    public Task Invoke(HttpContext httpContext)
+    {
+        return _next(httpContext);
+    }
+}
+```
+
+Similarly, ASP.NET Core filters pass a context argument to their methods, from which the current HttpContext can be accessed:
+
+```csharp
+public class MyActionFilterAttribute : ActionFilterAttribute
+{
+    public override void OnResultExecuting(ResultExecutingContext context)
+    {
+        var headers = context.HttpContext.Request.Headers;
+        // do something based on a header
+
+        base.OnResultExecuting(context);
+    }
+}
+```
+
+If you have components or services that require access to HttpContext, rather than using a static call like `HttpContext.Current` you should instead use constructor dependency injection and the [IHttpContextAccessor](https://docs.microsoft.com/dotnet/api/microsoft.aspnetcore.http.ihttpcontextaccessor) interface:
+
+```csharp
+public class MyService
+{
+    private readonly IHttpContextAccessor _httpContextAccessor;
+
+    public MyService(IHttpContextAccessor httpContextAccessor)
+    {
+        _httpContextAccessor = httpContextAccessor;
+    }
+
+    public void DoSomething()
+    {
+        var currentContext = _httpContextAccessor.HttpContext;
+    }
+}
+```
+
+This approach eliminates the static coupling of the method to the current context while providing access in a testable fashion.
+
 ## References
 
 - [ASP.NET HTTP modules and HTTP handlers](/troubleshoot/aspnet/http-modules-handlers)

docs/architecture/porting-existing-aspnet-apps/migrate-aspnet-core-2-1.md

@@ -7,7 +7,7 @@ ms.date: 11/13/2020
 
 # Migrate to ASP.NET Core 2.1
 
-ASP.NET Core 2.1 is an interesting release because it's the only currently supported .NET Core release to support both .NET Core and .NET Framework runtimes. As such, it may offer an easier upgrade path for some apps when compared to upgrading all parts of the app to .NET Core at once. As an LTS release, support for .NET Core 2.1 will continue through August 2021. Support for ASP.NET Core 2.1 running on .NET Framework will continue for as long as its underlying .NET Framework is supported.
+ASP.NET Core 2.1 is an interesting release because it's the only currently supported ASP.NET Core release to support both .NET Core and .NET Framework runtimes. As such, it may offer an easier upgrade path for some apps when compared to upgrading all parts of the app to .NET Core at once. As an LTS release, support for .NET Core 2.1 will continue through August 2021. Support for ASP.NET Core 2.1 running on .NET Framework will continue for as long as its underlying .NET Framework is supported.
 
 ## Should apps run on .NET Framework with ASP.NET Core 2.1