Move TempDataPropertyProvider into filter

src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs

@@ -81,9 +81,6 @@ namespace Microsoft.Extensions.DependencyInjection
             // Action executors
             services.TryAddSingleton<PageResultExecutor>();
             services.TryAddSingleton<RedirectToPageResultExecutor>();
-
-            // Random infrastructure
-            services.TryAddSingleton<TempDataPropertyProvider>();
         }
     }
 }

src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs

@@ -22,7 +22,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
     {
         private readonly IPageHandlerMethodSelector _selector;
         private readonly PageContext _pageContext;
-        private readonly TempDataPropertyProvider _propertyProvider;
 
         private Page _page;
         private object _model;
@@ -30,7 +29,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
 
         public PageActionInvoker(
             IPageHandlerMethodSelector handlerMethodSelector,
-            TempDataPropertyProvider propertyProvider,
             DiagnosticSource diagnosticSource,
             ILogger logger,
             PageContext pageContext,
@@ -45,7 +43,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
                   valueProviderFactories)
         {
             _selector = handlerMethodSelector;
-            _propertyProvider = propertyProvider;
             _pageContext = pageContext;
             CacheEntry = cacheEntry;
         }
@@ -359,11 +356,10 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
                 }
             }
 
-            var originalValues = _propertyProvider.LoadAndTrackChanges(_page, _pageContext.TempData);
             if (propertyFilter != null)
             {
-                propertyFilter.OriginalValues = originalValues;
                 propertyFilter.Subject = _page;
+                propertyFilter.ApplyTempDataChanges(_pageContext.HttpContext);
             }
 
             IActionResult result = null;

src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs

@@ -39,7 +39,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
         private readonly HtmlHelperOptions _htmlHelperOptions;
         private readonly RazorPagesOptions _razorPagesOptions;
         private readonly IPageHandlerMethodSelector _selector;
-        private readonly TempDataPropertyProvider _propertyProvider;
         private readonly RazorProject _razorProject;
         private readonly DiagnosticSource _diagnosticSource;
         private readonly ILogger<PageActionInvoker> _logger;
@@ -59,7 +58,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
             IOptions<HtmlHelperOptions> htmlHelperOptions,
             IOptions<RazorPagesOptions> razorPagesOptions,
             IPageHandlerMethodSelector selector,
-            TempDataPropertyProvider propertyProvider,
             RazorProject razorProject,
             DiagnosticSource diagnosticSource,
             ILoggerFactory loggerFactory)
@@ -77,7 +75,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
             _htmlHelperOptions = htmlHelperOptions.Value;
             _razorPagesOptions = razorPagesOptions.Value;
             _selector = selector;
-            _propertyProvider = propertyProvider;
             _razorProject = razorProject;
             _diagnosticSource = diagnosticSource;
             _logger = loggerFactory.CreateLogger<PageActionInvoker>();
@@ -159,7 +156,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
 
             return new PageActionInvoker(
                 _selector,
-                _propertyProvider,
                 _diagnosticSource,
                 _logger,
                 pageContext,

src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataPropertyFilter.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Reflection;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.Extensions.Internal;
 
@@ -26,6 +27,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
 
         public IDictionary<PropertyInfo, object> OriginalValues { get; set; }
 
+        /// <summary>
+        /// Puts the modified values of <see cref="Subject"/> into <paramref name="tempData"/>.
+        /// </summary>
+        /// <param name="tempData">The <see cref="ITempDataDictionary"/> to be updated.</param>
         public void OnTempDataSaving(ITempDataDictionary tempData)
         {
             if (Subject != null && OriginalValues != null)
@@ -44,38 +49,66 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             }
         }
 
-        public void OnActionExecuting(ActionExecutingContext context)
+        /// <summary>
+        /// Applies values from TempData from <paramref name="httpContext"/> to the <see cref="Subject"/>.
+        /// </summary>
+        /// <param name="httpContext">The <see cref="HttpContext"/> used to find TempData.</param>
+        public void ApplyTempDataChanges(HttpContext httpContext)
         {
-            if (PropertyHelpers == null)
+            if (Subject == null)
             {
-                throw new ArgumentNullException(nameof(PropertyHelpers));
+                throw new ArgumentNullException(nameof(Subject));
             }
 
+            var tempData = _factory.GetTempData(httpContext);
+
+            if (OriginalValues == null)
+            {
+                OriginalValues = new Dictionary<PropertyInfo, object>();
+            }
+
+            SetPropertyVaules(tempData, Subject);
+        }
+
+        /// <inheritdoc />
+        public void OnActionExecuting(ActionExecutingContext context)
+        {
             Subject = context.Controller;
             var tempData = _factory.GetTempData(context.HttpContext);
 
             OriginalValues = new Dictionary<PropertyInfo, object>();
 
+            SetPropertyVaules(tempData, Subject);
+        }
+
+        /// <inheritdoc />
+        public void OnActionExecuted(ActionExecutedContext context)
+        {
+        }
+
+        private void SetPropertyVaules(ITempDataDictionary tempData, object subject)
+        {
+            if (PropertyHelpers == null)
+            {
+                return;
+            }
+
             for (var i = 0; i < PropertyHelpers.Count; i++)
             {
-                var property = PropertyHelpers[i].Property;
+                var property = PropertyHelpers[i];
                 var value = tempData[Prefix + property.Name];
 
-                OriginalValues[property] = value;
+                OriginalValues[property.Property] = value;
 
-                var propertyTypeInfo = property.PropertyType.GetTypeInfo();
+                var propertyTypeInfo = property.Property.PropertyType.GetTypeInfo();
 
                 var isReferenceTypeOrNullable = !propertyTypeInfo.IsValueType || Nullable.GetUnderlyingType(property.GetType()) != null;
                 if (value != null || isReferenceTypeOrNullable)
                 {
-                    property.SetValue(Subject, value);
+                    property.SetValue(subject, value);
                 }
             }
         }
-
-        public void OnActionExecuted(ActionExecutedContext context)
-        {
-        }
     }
 }
 

src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/TempDataPropertyProvider.cs

@@ -1,67 +0,0 @@
-// 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;
-using System.Collections.Concurrent;
-using System.Collections.Generic;
-using System.Linq;
-using System.Reflection;
-
-namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
-{
-    public class TempDataPropertyProvider
-    {
-        public static readonly string Prefix = "TempDataProperty-";
-
-        private ConcurrentDictionary<Type, IEnumerable<PropertyInfo>> _subjectProperties =
-            new ConcurrentDictionary<Type, IEnumerable<PropertyInfo>>();
-
-        /// <summary>
-        /// Loads and tracks any changes to the properties of the <paramref name="subject"/>.
-        /// </summary>
-        /// <param name="subject">The properties of the subject are loaded and tracked. May be a <see cref="Controller"/>.</param>
-        /// <param name="tempData">The <see cref="ITempDataDictionary"/>.</param>
-        /// <returns></returns>
-        public IDictionary<PropertyInfo, object> LoadAndTrackChanges(object subject, ITempDataDictionary tempData)
-        {
-            var properties = GetSubjectProperties(subject);
-            var result = new Dictionary<PropertyInfo, object>();
-
-            foreach (var property in properties)
-            {
-                var value = tempData[Prefix + property.Name];
-
-                result[property] = value;
-
-                // TODO: Clarify what behavior should be for null values here
-                if (value != null && property.PropertyType.IsAssignableFrom(value.GetType()))
-                {
-                    property.SetValue(subject, value);
-                }
-            }
-
-            return result;
-        }
-
-        private IEnumerable<PropertyInfo> GetSubjectProperties(object subject)
-        {
-            return _subjectProperties.GetOrAdd(subject.GetType(), subjectType =>
-            {
-                var properties = subjectType.GetRuntimeProperties()
-                    .Where(pi => pi.GetCustomAttribute<TempDataAttribute>() != null);
-
-                if (properties.Any(pi => !(pi.SetMethod != null && pi.SetMethod.IsPublic && pi.GetMethod != null && pi.GetMethod.IsPublic)))
-                {
-                    throw new InvalidOperationException("TempData properties must have a public getter and setter.");
-                }
-
-                if (properties.Any(pi => !(pi.PropertyType.GetTypeInfo().IsPrimitive || pi.PropertyType == typeof(string))))
-                {
-                    throw new InvalidOperationException("TempData properties must be declared as primitive types or string only.");
-                }
-
-                return properties;
-            });
-        }
-    }
-}

test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs

@@ -820,7 +820,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
                 new TestOptionsManager<HtmlHelperOptions>(),
                 new TestOptionsManager<RazorPagesOptions>(razorPagesOptions ?? new RazorPagesOptions()),
                 Mock.Of<IPageHandlerMethodSelector>(),
-                new TempDataPropertyProvider(),
                 razorProject,
                 new DiagnosticListener("Microsoft.AspNetCore"),
                 NullLoggerFactory.Instance);

test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs

@@ -611,7 +611,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
 
             var invoker = new PageActionInvoker(
                 selector,
-                new TempDataPropertyProvider(),
                 diagnosticSource,
                 logger,
                 pageContext,

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/SaveTempDataPropertyFilterTest.cs

@@ -25,24 +25,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 ["TempDataProperty-Test"] = "FirstValue"
             };
 
-            var factory = new Mock<ITempDataDictionaryFactory>();
+            var filter = CreateSaveTempDataPropertyFilter(httpContext, tempData);
-            factory.Setup(f => f.GetTempData(httpContext))
-                .Returns(tempData);
-
-            var filter = new SaveTempDataPropertyFilter(factory.Object);
 
             var controller = new TestController();
-            var controllerType = controller.GetType().GetTypeInfo();
 
-            var propertyHelper1 = new PropertyHelper(controllerType.GetProperty(nameof(TestController.Test)));
+            filter.PropertyHelpers = BuildPropertyHelpers<TestController>();
-            var propertyHelper2 = new PropertyHelper(controllerType.GetProperty(nameof(TestController.Test2)));
-            var propertyHelpers = new List<PropertyHelper>
-            {
-                propertyHelper1,
-                propertyHelper2,
-            };
-
-            filter.PropertyHelpers = propertyHelpers;
             var context = new ActionExecutingContext(
                 new ActionContext
                 {
@@ -75,23 +62,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 ["TempDataProperty-Test"] = "FirstValue"
             };
 
-            var factory = new Mock<ITempDataDictionaryFactory>();
+            var filter = CreateSaveTempDataPropertyFilter(httpContext, tempData: tempData);
-            factory.Setup(f => f.GetTempData(httpContext))
-                .Returns(tempData);
-
-            var filter = new SaveTempDataPropertyFilter(factory.Object);
             var controller = new TestController();
-            var controllerType = controller.GetType().GetTypeInfo();
 
-            var propertyHelper1 = new PropertyHelper(controllerType.GetProperty(nameof(TestController.Test)));
+            filter.PropertyHelpers = BuildPropertyHelpers<TestController>();
-            var propertyHelper2 = new PropertyHelper(controllerType.GetProperty(nameof(TestController.Test2)));
-            var propertyHelpers = new List<PropertyHelper>
-            {
-                propertyHelper1,
-                propertyHelper2,
-            };
-
-            filter.PropertyHelpers = propertyHelpers;
 
             var context = new ActionExecutingContext(
                 new ActionContext
@@ -113,6 +87,72 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             Assert.Equal(0, controller.Test2);
         }
 
+        [Fact]
+        public void ApplyTempDataChanges_SetsPropertyValue()
+        {
+            // Arrange
+            var httpContext = new DefaultHttpContext();
+
+            var tempData = new TempDataDictionary(httpContext, Mock.Of<ITempDataProvider>())
+            {
+                { "TempDataProperty-Test", "Value" }
+            };
+            tempData.Save();
+
+            var controller = new TestControllerStrings()
+            {
+                TempData = tempData,
+            };
+
+            var provider = CreateSaveTempDataPropertyFilter(httpContext, tempData: tempData);
+            provider.Subject = controller;
+            provider.PropertyHelpers = BuildPropertyHelpers<TestControllerStrings>();
+
+            // Act
+            provider.ApplyTempDataChanges(httpContext);
+
+            // Assert
+            Assert.Equal("Value", controller.Test);
+            Assert.Null(controller.Test2);
+        }
+
+        private IList<PropertyHelper> BuildPropertyHelpers<TSubject>()
+        {
+            var subjectType = typeof(TSubject);
+
+            var properties = subjectType.GetProperties(
+                BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly);
+
+            var result = new List<PropertyHelper>();
+
+            foreach (var property in properties)
+            {
+                result.Add(new PropertyHelper(property));
+            }
+
+            return result;
+        }
+
+        private SaveTempDataPropertyFilter CreateSaveTempDataPropertyFilter(
+            HttpContext httpContext,
+            TempDataDictionary tempData)
+        {
+            var factory = new Mock<ITempDataDictionaryFactory>();
+            factory.Setup(f => f.GetTempData(httpContext))
+                .Returns(tempData);
+
+            return new SaveTempDataPropertyFilter(factory.Object);
+        }
+
+        public class TestControllerStrings : Controller
+        {
+            [TempData]
+            public string Test { get; set; }
+
+            [TempData]
+            public string Test2 { get; set; }
+        }
+
         public class TestController : Controller
         {
             [TempData]

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataPropertyProviderTest.cs

@@ -1,114 +0,0 @@
-// 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;
-using System.Collections.Generic;
-using Microsoft.AspNetCore.Http;
-using Moq;
-using Xunit;
-
-namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
-{
-    public class TempDataPropertyProviderTest
-    {
-        [Fact]
-        public void LoadAndTrackChanges_SetsPropertyValue()
-        {
-            // Arrange
-            var provider = new TempDataPropertyProvider();
-
-            var tempData = new TempDataDictionary(new DefaultHttpContext(), new NullTempDataProvider());
-            tempData["TempDataProperty-TestString"] = "Value";
-            tempData.Save();
-
-            var controller = new TestController()
-            {
-                TempData = tempData,
-            };
-
-            // Act
-            provider.LoadAndTrackChanges(controller, controller.TempData);
-
-            // Assert
-            Assert.Equal("Value", controller.TestString);
-            Assert.Null(controller.TestString2);
-        }
-
-        [Fact]
-        public void LoadAndTrackChanges_ThrowsInvalidOperationException_PrivateSetter()
-        {
-            // Arrange
-            var provider = new TempDataPropertyProvider();
-
-            var tempData = new TempDataDictionary(new DefaultHttpContext(), new NullTempDataProvider());
-            tempData["TempDataProperty-Test"] = "Value";
-            tempData.Save();
-
-            var controller = new TestController_PrivateSet()
-            {
-                TempData = tempData,
-            };
-
-            // Act & Assert
-            var exception = Assert.Throws<InvalidOperationException>(() =>
-                provider.LoadAndTrackChanges(controller, controller.TempData));
-
-            Assert.Equal("TempData properties must have a public getter and setter.", exception.Message);
-        }
-
-        [Fact]
-        public void LoadAndTrackChanges_ThrowsInvalidOperationException_NonPrimitiveType()
-        {
-            // Arrange
-            var provider = new TempDataPropertyProvider();
-
-            var tempData = new TempDataDictionary(new DefaultHttpContext(), new NullTempDataProvider());
-            tempData["TempDataProperty-Test"] = new object();
-            tempData.Save();
-
-            var controller = new TestController_NonPrimitiveType()
-            {
-                TempData = tempData,
-            };
-
-            // Act & Assert
-            var exception = Assert.Throws<InvalidOperationException>(() =>
-                provider.LoadAndTrackChanges(controller, controller.TempData));
-
-            Assert.Equal("TempData properties must be declared as primitive types or string only.", exception.Message);
-        }
-
-        public class TestController : Controller
-        {
-            [TempData]
-            public string TestString { get; set; }
-
-            [TempData]
-            public string TestString2 { get; set; }
-        }
-
-        public class TestController_PrivateSet : Controller
-        {
-            [TempData]
-            public string Test { get; private set; }
-        }
-
-        public class TestController_NonPrimitiveType : Controller
-        {
-            [TempData]
-            public object Test { get; set; }
-        }
-
-        private class NullTempDataProvider : ITempDataProvider
-        {
-            public IDictionary<string, object> LoadTempData(HttpContext context)
-            {
-                return null;
-            }
-
-            public void SaveTempData(HttpContext context, IDictionary<string, object> values)
-            {
-            }
-        }
-    }
-}