diff --git a/CHANGELOG.md b/CHANGELOG.md index baae80c7..0d6b733d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### Fixed - Fixed issue surfacing from floating package references. [#371](https://github.com/dotnet/upgrade-assistant/pull/371) +- Microsoft.AspNetCore.Mvc.NewtonsoftJson package should no longer be added to .NET Framework projects [#290](https://github.com/dotnet/upgrade-assistant/pull/390) - Fixed issue to ignore disabled NuGet sources. [#396](https://github.com/dotnet/upgrade-assistant/issues/396) - ## Version 0.2.217201 - 2021-03-23 ([Link](https://www.nuget.org/packages/upgrade-assistant/0.2.217201)) ### Added - Include try-convert version in upgrade-assistant package #358 diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/NewtonsoftReferenceAnalyzer.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/NewtonsoftReferenceAnalyzer.cs index e5fb304e..efc1ee2d 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/NewtonsoftReferenceAnalyzer.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/NewtonsoftReferenceAnalyzer.cs @@ -9,19 +9,25 @@ using Microsoft.Extensions.Logging; namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers { + /// + /// Increases backward compatibility by using the Newtonsoft Serializer for ASP.NET Core. + /// public class NewtonsoftReferenceAnalyzer : IPackageReferencesAnalyzer { private const string NewtonsoftPackageName = "Microsoft.AspNetCore.Mvc.NewtonsoftJson"; + private readonly TargetFrameworkMoniker _netCore3Moniker = new TargetFrameworkMoniker("netcoreapp3.0"); private readonly IPackageLoader _packageLoader; private readonly ILogger _logger; + private readonly ITargetFrameworkMonikerComparer _tfmComparer; public string Name => "Newtonsoft.Json reference analyzer"; - public NewtonsoftReferenceAnalyzer(IPackageLoader packageLoader, ILogger logger) + public NewtonsoftReferenceAnalyzer(IPackageLoader packageLoader, ILogger logger, ITargetFrameworkMonikerComparer tfmComparer) { _packageLoader = packageLoader ?? throw new ArgumentNullException(nameof(packageLoader)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _tfmComparer = tfmComparer ?? throw new ArgumentNullException(nameof(tfmComparer)); } public async Task AnalyzeAsync(IProject project, PackageAnalysisState state, CancellationToken token) @@ -37,7 +43,9 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers } // This reference only needs added to ASP.NET Core exes - if (!(project.Components.HasFlag(ProjectComponents.AspNetCore) && project.OutputType == ProjectOutputType.Exe)) + if (!(project.Components.HasFlag(ProjectComponents.AspNetCore) + && project.OutputType == ProjectOutputType.Exe + && !project.TargetFrameworks.Any(tfm => _tfmComparer.Compare(tfm, _netCore3Moniker) < 0))) { return state; } diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/IPackageReferencesAnalyzer.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/IPackageReferencesAnalyzer.cs index 54097810..8ea823c7 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/IPackageReferencesAnalyzer.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/IPackageReferencesAnalyzer.cs @@ -17,6 +17,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages /// /// The project whose NuGet package references should be analyzed. /// The current analysis state which will be updated and returned. + /// The token used to gracefully cancel this request. /// The analysis state object provided updated based on this analyzer's analysis. Task AnalyzeAsync(IProject project, PackageAnalysisState state, CancellationToken token); } diff --git a/tests/components/Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests/MSBuildStepTestCollection.cs b/tests/components/Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests/MSBuildStepTestCollection.cs index dec84c0d..426d08b2 100644 --- a/tests/components/Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests/MSBuildStepTestCollection.cs +++ b/tests/components/Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests/MSBuildStepTestCollection.cs @@ -13,6 +13,8 @@ namespace Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests [CollectionDefinition(Name)] public class MSBuildStepTestCollection : ICollectionFixture { + // by design this class must be implemented in every test project that uses the MSBuildRegistrationFixture + // https://github.com/xunit/xunit/issues/409 public const string Name = "Package Step Tests"; } } diff --git a/tests/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Configuration.Tests/ConfigUpdaterSubStepTests.cs b/tests/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Configuration.Tests/ConfigUpdaterSubStepTests.cs index 3b52f467..3022e02f 100644 --- a/tests/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Configuration.Tests/ConfigUpdaterSubStepTests.cs +++ b/tests/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Configuration.Tests/ConfigUpdaterSubStepTests.cs @@ -3,10 +3,8 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.IO; using System.Linq; -using System.Text; using System.Threading; using System.Threading.Tasks; using Autofac; diff --git a/tests/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Tests/Analyzers/NewtonsoftReferenceAnalyzerTests.cs b/tests/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Tests/Analyzers/NewtonsoftReferenceAnalyzerTests.cs new file mode 100644 index 00000000..5d307f99 --- /dev/null +++ b/tests/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Tests/Analyzers/NewtonsoftReferenceAnalyzerTests.cs @@ -0,0 +1,163 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Threading; +using System.Threading.Tasks; +using Autofac.Extras.Moq; +using Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers; +using Moq; +using Xunit; + +namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Tests.Analyzers +{ + /// + /// Unit tests for the NewtonsoftReferenceAnalyzer. + /// + public class NewtonsoftReferenceAnalyzerTests + { + private const string NewtonsoftPackageName = "Microsoft.AspNetCore.Mvc.NewtonsoftJson"; + + /// + /// Validates that the analyzer will only be applied when TFM is not net48. + /// + /// a task. + [Fact] + public async Task AnalyzerIsApplicableToAspNetCoreWebProject() + { + // Arrange + using var mock = AutoMock.GetLoose(); + var analyzer = mock.Create(); + var project = CreateProjectForWhichAnalyzerIsApplicable(mock); + var packageState = await CreatePackageAnalysisState(mock, project).ConfigureAwait(false); + var packageLoader = CreatePackageLoader(mock); + + // Act + var actual = await analyzer.AnalyzeAsync(project.Object, packageState, default).ConfigureAwait(false); + + // Assert + Assert.Contains(actual.PackagesToAdd, (package) => package.Name.Equals(NewtonsoftPackageName, System.StringComparison.Ordinal)); + packageLoader.Verify(pl => pl.GetLatestVersionAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once()); + } + + /// + /// Validates that the analyzer will not be applied to net48 TFM. + /// + /// a task. + [Fact] + public async Task AnalyzerIsNotApplicableToNetFrameworkProjects() + { + // Arrange + using var mock = AutoMock.GetLoose(); + + var analyzer = mock.Create(); + var project = CreateProjectForWhichAnalyzerIsApplicable(mock); + var packageState = await CreatePackageAnalysisState(mock, project).ConfigureAwait(false); + var packageLoader = CreatePackageLoader(mock); + + // shift project attributes so that it is not applicable + project.Setup(p => p.TargetFrameworks).Returns(new[] { new TargetFrameworkMoniker("net472") }); + var comparer = mock.Mock(); + comparer.Setup(comparer => comparer.Compare(It.IsAny(), It.IsAny())) + .Returns(-1); + + // Act + var actual = await analyzer.AnalyzeAsync(project.Object, packageState, default).ConfigureAwait(false); + + // Assert + Assert.DoesNotContain(actual.PackagesToAdd, (package) => package.Name.Equals(NewtonsoftPackageName, System.StringComparison.Ordinal)); + packageLoader.Verify(pl => pl.GetLatestVersionAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never()); + } + + /// + /// Validates that the analyzer will not be applied to library projects. + /// + /// a task. + [Fact] + public async Task AnalyzerIsNotApplicableToLibraryProjects() + { + // Arrange + using var mock = AutoMock.GetLoose(); + + var analyzer = mock.Create(); + var project = CreateProjectForWhichAnalyzerIsApplicable(mock); + var packageState = await CreatePackageAnalysisState(mock, project).ConfigureAwait(false); + var packageLoader = CreatePackageLoader(mock); + + // shift project attributes so that it is not applicable + project.Setup(p => p.OutputType).Returns(ProjectOutputType.Library); + + // Act + var actual = await analyzer.AnalyzeAsync(project.Object, packageState, default).ConfigureAwait(false); + + // Assert + Assert.DoesNotContain(actual.PackagesToAdd, (package) => package.Name.Equals(NewtonsoftPackageName, System.StringComparison.Ordinal)); + packageLoader.Verify(pl => pl.GetLatestVersionAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never()); + } + + /// + /// Validates that the analyzer will not be applied to net48 TFM. + /// + /// a task. + [Fact] + public async Task AnalyzerIsOnlyApplicableToAspNetProjects() + { + // Arrange + using var mock = AutoMock.GetLoose(); + + var analyzer = mock.Create(); + var project = CreateProjectForWhichAnalyzerIsApplicable(mock); + var packageState = await CreatePackageAnalysisState(mock, project).ConfigureAwait(false); + var packageLoader = CreatePackageLoader(mock); + + // shift project attributes so that it is not applicable + project.Setup(p => p.Components).Returns(ProjectComponents.Wpf); + + // Act + var actual = await analyzer.AnalyzeAsync(project.Object, packageState, default).ConfigureAwait(false); + + // Assert + Assert.DoesNotContain(actual.PackagesToAdd, (package) => package.Name.Equals(NewtonsoftPackageName, System.StringComparison.Ordinal)); + packageLoader.Verify(pl => pl.GetLatestVersionAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never()); + } + + private static async Task CreatePackageAnalysisState(AutoMock mock, Mock project) + { + var context = mock.Mock(); + context.Setup(c => c.CurrentProject).Returns(project.Object); + var restorer = mock.Mock(); + restorer.Setup(r => r.RestorePackagesAsync( + It.IsAny(), + It.IsAny(), + It.IsAny())).Returns(Task.FromResult(true)); + + return await PackageAnalysisState.CreateAsync(context.Object, restorer.Object, CancellationToken.None).ConfigureAwait(true); + } + + private static Mock CreateProjectForWhichAnalyzerIsApplicable(AutoMock mock) + { + var project = mock.Mock(); + var nugetReferences = mock.Mock(); + nugetReferences.Setup(n => n.IsTransitivelyAvailable(It.IsAny())) + .Returns(false); + + project.Setup(p => p.TargetFrameworks).Returns(new[] { new TargetFrameworkMoniker("net5.0") }); + project.Setup(p => p.Components).Returns(ProjectComponents.AspNetCore); + project.Setup(p => p.OutputType).Returns(ProjectOutputType.Exe); + project.Setup(p => p.NuGetReferences).Returns(nugetReferences.Object); + + return project; + } + + private static Mock CreatePackageLoader(AutoMock mock) + { + var packageLoader = mock.Mock(); + packageLoader.Setup(pl => pl.GetLatestVersionAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.FromResult((NuGetReference?)new NuGetReference(NewtonsoftPackageName, "122.0.0"))); + return packageLoader; + } + } +} diff --git a/tests/tool/Integration.Tests/IntegrationScenarios/AspNetSample/csharp/Upgraded/TemplateMvc.csproj b/tests/tool/Integration.Tests/IntegrationScenarios/AspNetSample/csharp/Upgraded/TemplateMvc.csproj index 429ef287..649180a2 100644 --- a/tests/tool/Integration.Tests/IntegrationScenarios/AspNetSample/csharp/Upgraded/TemplateMvc.csproj +++ b/tests/tool/Integration.Tests/IntegrationScenarios/AspNetSample/csharp/Upgraded/TemplateMvc.csproj @@ -31,8 +31,8 @@ all - +