Ensure ChangeSet is always initialized before use

Changelog.md

@@ -19,6 +19,7 @@ This release gives some ❤️ to AvaloniaUI and other frameworks which use ILis
 
 ### Server
 
+* ChangeSet now throws an `InvalidOperationException` if accessed outside a submit operation, where `ChangeSet` previously returned `null`.
 * Added Nullability annotations for a few core types of public API
 
 ### Other

src/OpenRiaServices.Client/Framework/Polyfills.cs

@@ -1,5 +1,6 @@
 #if !NET
 using System;
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Runtime.CompilerServices;
 
@@ -88,6 +89,22 @@ public CallerArgumentExpressionAttribute(string parameterName)
     }
 }
 
+namespace System.Diagnostics
+{
+    /// <summary>
+    /// Types and Methods attributed with StackTraceHidden will be omitted from the stack trace text shown in StackTrace.ToString()
+    /// and Exception.StackTrace
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Struct, Inherited = false)]
+    public sealed class StackTraceHiddenAttribute : Attribute
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="StackTraceHiddenAttribute"/> class.
+        /// </summary>
+        public StackTraceHiddenAttribute() { }
+    }
+}
+
 namespace System.Diagnostics.CodeAnalysis
 {
     //

src/OpenRiaServices.Server/Framework/Data/DomainService.cs

@@ -3,6 +3,7 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Linq;
 using System.Reflection;
@@ -216,16 +217,28 @@ public void Dispose()
         }
 
         /// <summary>
-        /// Gets the current <see cref="ChangeSet"/>. Returns null if no change operations are being performed.
+        /// Gets the current <see cref="ChangeSet"/>. Throws if no change operations are being performed.
         /// </summary>
-        protected ChangeSet? ChangeSet
+        /// <exception cref="InvalidOperationException">Thrown if no change operations are being performed.</exception>
+
+        protected ChangeSet ChangeSet
         {
             get
             {
+                if (this._changeSet == null)
+                    ThrowChangeSetNotInitialized();
+
                 return this._changeSet;
+
+                [DoesNotReturn]
+                [System.Diagnostics.StackTraceHidden]
+                static void ThrowChangeSetNotInitialized() =>
+                    throw new InvalidOperationException(
+                        Resource.DomainService_ChangeSetNotInitialized);
             }
         }
 
+
         #endregion // Properties
 
         /// <summary>
@@ -718,7 +731,7 @@ protected virtual void Dispose(bool disposing)
         /// <returns>True if the <see cref="ChangeSet"/> is authorized, false otherwise.</returns>
         protected virtual bool AuthorizeChangeSet()
         {
-            foreach (ChangeSetEntry op in this.ChangeSet!.ChangeSetEntries)
+            foreach (ChangeSetEntry op in this.ChangeSet.ChangeSetEntries)
             {
                 object entity = op.Entity;
 
@@ -751,7 +764,7 @@ protected virtual bool AuthorizeChangeSet()
         protected virtual ValueTask<bool> ValidateChangeSetAsync(CancellationToken cancellationToken)
         {
             // Perform validation on the each of the operations.
-            var result = ValidateOperations(this.ChangeSet!.ChangeSetEntries, this.ServiceDescription, this.ValidationContext);
+            var result = ValidateOperations(this.ChangeSet.ChangeSetEntries, this.ServiceDescription, this.ValidationContext);
             return new ValueTask<bool>(result);
         }
 
@@ -766,7 +779,7 @@ await this.InvokeCudOperationsAsync()
             await this.InvokeCustomOperations()
                 .ConfigureAwait(false);
 
-            return !this.ChangeSet!.HasError;
+            return !this.ChangeSet.HasError;
         }
 
         /// <summary>
@@ -794,7 +807,7 @@ protected virtual void OnError(DomainServiceErrorInfo errorInfo) { }
         private void ResolveOperations()
         {
             // Resolve and set the DomainOperationEntry for each operation in the changeset
-            foreach (ChangeSetEntry changeSetEntry in this.ChangeSet!.ChangeSetEntries)
+            foreach (ChangeSetEntry changeSetEntry in this.ChangeSet.ChangeSetEntries)
             {
                 // resolve the DomainOperationEntry
                 Type entityType = changeSetEntry.Entity.GetType();
@@ -984,7 +997,7 @@ private async Task<bool> PersistChangeSetAsyncInternal(CancellationToken cancell
                 if (e.Value != null && e.ValidationResult != null)
                 {
                     IEnumerable<ChangeSetEntry> updateOperations =
-                        this.ChangeSet!.ChangeSetEntries.Where(
+                        this.ChangeSet.ChangeSetEntries.Where(
                             p => p.Operation == DomainOperation.Insert ||
                                  p.Operation == DomainOperation.Update ||
                                  p.Operation == DomainOperation.Delete);
@@ -1003,7 +1016,7 @@ private async Task<bool> PersistChangeSetAsyncInternal(CancellationToken cancell
                 }
             }
 
-            return !this.ChangeSet!.HasError;
+            return !this.ChangeSet.HasError;
         }
 
         /// <summary>
@@ -1040,7 +1053,7 @@ private void ValidateMethodPermissions(DomainOperationEntry domainOperationEntry
         private async Task InvokeCudOperationsAsync()
         {
             object[] parameters = new object[1];
-            foreach (ChangeSetEntry operation in this.ChangeSet!.ChangeSetEntries
+            foreach (ChangeSetEntry operation in this.ChangeSet.ChangeSetEntries
                 .Where(op => op.Operation == DomainOperation.Insert ||
                              op.Operation == DomainOperation.Update ||
                              op.Operation == DomainOperation.Delete))
@@ -1082,7 +1095,7 @@ await this.InvokeDomainOperationEntryAsync(operation.DomainOperationEntry, param
         /// </summary>
         private async Task InvokeCustomOperations()
         {
-            foreach (ChangeSetEntry operation in this.ChangeSet!.ChangeSetEntries.Where(op => op.EntityActions != null && op.EntityActions.Any()))
+            foreach (ChangeSetEntry operation in this.ChangeSet.ChangeSetEntries.Where(op => op.EntityActions != null && op.EntityActions.Any()))
             {
                 foreach (var entityAction in operation.EntityActions)
                 {

src/OpenRiaServices.Server/Framework/Data/Resource.resx

@@ -213,6 +213,9 @@
   <data name="DomainService_AlreadyInitialized" xml:space="preserve">
     <value>This DomainService has already been initialized.</value>
   </data>
+  <data name="DomainService_ChangeSetNotInitialized" xml:space="preserve">
+    <value>The ChangeSet of this DomainService is only available while processing a 'Submit' operation.</value>
+  </data>
   <data name="DomainService_DuplicateCUDMethod" xml:space="preserve">
     <value>The domain operation entry named '{0}' provides redundant functionality. Another method named '{1}' already exists.</value>
   </data>

src/OpenRiaServices.Server/Test/DomainServiceTests.cs

@@ -7,18 +7,18 @@
 using System.Globalization;
 using System.Linq;
 using System.Reflection;
+using System.Threading;
+using System.Threading.Tasks;
+using System.Xml.Linq;
+using Cities;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
 using OpenRiaServices.Client.Test;
 using OpenRiaServices.EntityFramework;
 using OpenRiaServices.Hosting.Wcf;
-using System.Xml.Linq;
-using Cities;
 using OpenRiaServices.LinqToSql;
-using Microsoft.VisualStudio.TestTools.UnitTesting;
-using TestDomainServices;
-using System.Threading.Tasks;
-using System.Threading;
-using OpenRiaServices.Server.UnitTesting;
 using OpenRiaServices.Server.EntityFrameworkCore;
+using OpenRiaServices.Server.UnitTesting;
+using TestDomainServices;
 
 namespace OpenRiaServices.Server.Test
 {
@@ -48,6 +48,60 @@
             System.Threading.Thread.CurrentThread.CurrentUICulture = CultureInfo.InvariantCulture;
         }
 
+        /// <summary>
+        /// Verify that accessing ChangeSet before SubmitAsync throws InvalidOperationException.
+        /// </summary>
+        [TestMethod]
+        public void ChangeSet_ThrowsBeforeSubmitAsync()
+        {
+            var ds = new ChangeSetAccessorDomainService();
+            DomainServiceContext dsc = new DomainServiceContext(new MockDataService(), new MockUser("mathew"), DomainOperationType.Invoke);
+            ds.Initialize(dsc);
+
+            ExceptionHelper.ExpectInvalidOperationException(
+                () => { var _ = ds.GetChangeSet(); },
+                Resource.DomainService_ChangeSetNotInitialized);
+        }
+
+        /// <summary>
+        /// Verify that accessing ChangeSet before Initialize and SubmitAsync throws InvalidOperationException.
+        /// </summary>
+        [TestMethod]
+        public void ChangeSet_ThrowsBeforeInitializationAndSubmit()
+        {
+            var ds = new ChangeSetAccessorDomainService();
+
+            ExceptionHelper.ExpectInvalidOperationException(
+                () => { var _ = ds.GetChangeSet(); },
+                Resource.DomainService_ChangeSetNotInitialized);
+        }
+
+        /// <summary>
+        /// Verify that ChangeSet is accessible during SubmitAsync after initialization.
+        /// </summary>
+        [TestMethod]
+        public async Task ChangeSet_AccessibleAfterSubmitAsync()
+        {
+            var ds = new ChangeSetAccessorDomainService();
+            DomainServiceContext dsc = new DomainServiceContext(new MockDataService(), new MockUser("mathew"), DomainOperationType.Submit);
+            ds.Initialize(dsc);
+
+            City city = new City
+            {
+                Name = "TestCity",
+                CountyName = "TestCounty",
+                StateName = "WA"
+            };
+
+            ChangeSetEntry insertEntry = new ChangeSetEntry(1, city, null, DomainOperation.Insert);
+            ChangeSet cs = new ChangeSet(new[] { insertEntry });
+
+            bool result = await ds.SubmitAsync(cs, CancellationToken.None);
+
+            Assert.IsTrue(result);
+            Assert.IsTrue(ds.ChangeSetWasAccessibleDuringSubmit, "ChangeSet should be accessible during SubmitAsync.");
+        }
+
         /// <summary>
         /// Verify that both DAL providers support accessing their respective
         /// contexts in the constructor.
@@ -168,7 +222,7 @@
             DomainOperationEntry queryOperation = desc.GetQueryMethod("GetCities");
             QueryDescription query = new QueryDescription(queryOperation, Array.Empty<object>(), /* includeTotalCount */ true, Array.Empty<Zip>().AsQueryable().Where(z => z.Code == 98052));
 
-            ExceptionHelper.ExpectArgumentException(() =>  
+            ExceptionHelper.ExpectArgumentException(() =>
                 ds.QueryAsync<City>(query, CancellationToken.None).GetAwaiter().GetResult()
             , "Expression of type 'System.Linq.EnumerableQuery`1[Cities.City]' cannot be used for parameter of type 'System.Linq.IQueryable`1[Cities.Zip]' of method 'System.Linq.IQueryable`1[Cities.Zip] Where[Zip](System.Linq.IQueryable`1[Cities.Zip], System.Linq.Expressions.Expression`1[System.Func`2[Cities.Zip,System.Boolean]])'");
         }
@@ -350,7 +404,7 @@
             ds.Initialize(dsc);
             query = new QueryDescription(queryOperation, new object[] { 50 });
             var queryResult = await ds.QueryAsync<City>(query, CancellationToken.None);
-            
+
             Assert.IsTrue(queryResult.HasValidationErrors, "Should have validation errors");
             Assert.AreEqual(1, queryResult.ValidationErrors.Count());
             Assert.IsNull(ds.LastError);
@@ -564,7 +618,7 @@
             // Ensure metadata (TypeDescriptors) are initialised
             DomainServiceDescription.GetDescription(typeof(TestDomainServices.EF.Northwind));
 
-            var expectedChangedProperties = new []
+            var expectedChangedProperties = new[]
             {
                 "CategoryName",
                 "Description",
@@ -591,7 +645,7 @@
 
             var currentEntry = ctx.ObjectStateManager.GetObjectStateEntry(current);
             string[] actualChangedProperties = currentEntry.GetModifiedProperties().ToArray();
-            
+
             CollectionAssert.AreEquivalent(expectedChangedProperties, actualChangedProperties);
         }
 
@@ -661,7 +715,7 @@
             DbContextExtensions.AttachAsModified(catalog.DbContext.Products, current, original, catalog.DbContext);
 
             var currentEntry = catalog.DbContext.Entry(current);
-            
+
             string[] actualNotChangedProperties = currentEntry.CurrentValues.PropertyNames.Where(p => !currentEntry.Property(p).IsModified).ToArray();
 
             CollectionAssert.AreEquivalent(expectedNotChangedProperties, actualNotChangedProperties);
@@ -2127,9 +2181,10 @@
         public TestDomainService_OverloadTests()
         {
             // initialize with a test user
-            Initialize(new DomainServiceContext(new MockDataService(), new MockUser("mathew") {
+            Initialize(new DomainServiceContext(new MockDataService(), new MockUser("mathew")
+            {
                 IsAuthenticated = true
             }, DomainOperationType.Submit));
         }
 
         // verify that we can override base initialization
@@ -2461,7 +2516,8 @@
         public void UpdateEntity(DomainServiceInsertCustom_Entity entity) { }
         [EntityAction]
         public void UpdateEntityWithInt(DomainServiceInsertCustom_Entity entity,
-            [CustomValidation(typeof(AlwaysFailValidator), "Validate")] int i) { }
+            [CustomValidation(typeof(AlwaysFailValidator), "Validate")] int i)
+        { }
         [EntityAction]
         public void UpdateEntityWithObject(DomainServiceInsertCustom_Entity entity, DomainServiceInsertCustom_Validated_Object obj) { }
         [EntityAction]
@@ -2479,5 +2535,26 @@
         [CustomValidation(typeof(AlwaysFailValidator), "Validate")]
         public int IntProp { get; set; }
     }
+    public class ChangeSetAccessorDomainService : DomainService
+    {
+        public bool ChangeSetWasAccessibleDuringSubmit { get; private set; }
+
+        public ChangeSet GetChangeSet() => ChangeSet;
+
+        [Query]
+        public IQueryable<City> GetCities() => Array.Empty<City>().AsQueryable();
+
+        public void InsertCity(City city)
+        {
+        }
+
+        protected override ValueTask<bool> PersistChangeSetAsync(CancellationToken cancellationToken)
+        {
+            // Verify ChangeSet is accessible during the submit pipeline
+            _ = ChangeSet;
+            ChangeSetWasAccessibleDuringSubmit = true;
+            return new ValueTask<bool>(true);
+        }
+    }
     #endregion // Mock DomainService Types
 }

src/VisualStudio/ItemTemplates/CSharp/AuthenticationDomainService/AuthenticationDomainService.csproj

@@ -31,7 +31,7 @@
     <AppDesignerFolder>Properties</AppDesignerFolder>
     <RootNamespace>AuthenticationDomainService</RootNamespace>
     <AssemblyName>AuthenticationDomainService</AssemblyName>
-    <TargetFrameworkVersion>v4.6</TargetFrameworkVersion>
+    <TargetFrameworkVersion>v4.8</TargetFrameworkVersion>
     <FileAlignment>512</FileAlignment>
     <GeneratePkgDefFile>false</GeneratePkgDefFile>
     <IncludeAssemblyInVSIXContainer>false</IncludeAssemblyInVSIXContainer>
@@ -104,4 +104,4 @@
   <Target Name="AfterBuild">
   </Target>
   -->
-</Project>
+</Project>

src/VisualStudio/ItemTemplates/CSharp/DomainServiceClass/DomainServiceClass.csproj

@@ -31,7 +31,7 @@
     <AppDesignerFolder>Properties</AppDesignerFolder>
     <RootNamespace>DomainServiceClass</RootNamespace>
     <AssemblyName>DomainServiceClass</AssemblyName>
-    <TargetFrameworkVersion>v4.6</TargetFrameworkVersion>
+    <TargetFrameworkVersion>v4.8</TargetFrameworkVersion>
     <FileAlignment>512</FileAlignment>
     <GeneratePkgDefFile>false</GeneratePkgDefFile>
     <IncludeAssemblyInVSIXContainer>false</IncludeAssemblyInVSIXContainer>
@@ -103,4 +103,4 @@
   <Target Name="AfterBuild">
   </Target>
   -->
-</Project>
+</Project>