Add new test that validate how object level validation interact with …

…property level validation (#455)

* Add tests based on special case for #453

* Add possible fix for regression

* fix CodeQL comment

src/OpenRiaServices.Client/Framework/Entity.cs

@@ -1309,6 +1309,12 @@ protected void ValidateProperty(string propertyName, object value)
                 validationContext.MemberName = propertyName;
                 this.ValidateProperty(validationContext, value);
             }
+            else if (MetaType.RequiresObjectValidation)
+            {
+                // Validation error must have been set by object level validation
+                // Clear it to mimic old behaviour where validate property was always called in these scenarios
+                this.ValidationResultCollection.ReplaceErrors(propertyName, Array.Empty<ValidationResult>());
+            }
         }
 
         /// <summary>

src/OpenRiaServices.Client/Framework/Internal/MetaType.cs

@@ -32,6 +32,7 @@ public sealed class MetaType
         [ThreadStatic]
         private static Dictionary<Type, MetaType> s_metaTypes;
         private readonly bool _requiresValidation;
+        private readonly bool _requiresObjectValidation;
         private readonly Type[] _childTypes;
         private readonly Dictionary<string, MetaMember> _metaMembers = new Dictionary<string, MetaMember>();
         private readonly ReadOnlyCollection<MetaMember> _dataMembers;
@@ -121,7 +122,8 @@ select attribute
             this.Type = type;
 
             _validationAttributes = new ReadOnlyCollection<ValidationAttribute>(this.Type.GetCustomAttributes(typeof(ValidationAttribute), true).OfType<ValidationAttribute>().ToArray());
-            _requiresValidation = _requiresValidation || _validationAttributes.Any() || typeof(IValidatableObject).IsAssignableFrom(type);
+            _requiresObjectValidation = _validationAttributes.Any() || typeof(IValidatableObject).IsAssignableFrom(type);
+            _requiresValidation = _requiresValidation || _requiresObjectValidation;
 
             // for identity purposes, we need to make sure values are always ordered
             KeyMembers = new ReadOnlyCollection<MetaMember>(_metaMembers.Values.Where(m => m.IsKeyMember).OrderBy(m => m.Name).ToArray());
@@ -262,6 +264,12 @@ public IEnumerable<MetaMember> AssociationMembers
         /// </summary>
         public bool RequiresValidation => this._requiresValidation;
 
+        /// <summary>
+        /// Gets a value indicating whether the Type requires any Type level
+        /// validation.
+        /// </summary>
+        internal bool RequiresObjectValidation => this._requiresObjectValidation;
+
         /// <summary>
         /// Gets a value indicating whether the Type has any members marked with
         /// CompositionAttribute.

src/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/EntityValidationTests.cs

@@ -25,7 +25,7 @@ public class EntityValidationTests : UnitTestBase
         public void Entity_Validation_Property_Fail_ReadOnly_Throws()
         {
             MockEntity entity = new MockEntity();
-            ExceptionHelper.ExpectInvalidOperationException(delegate()
+            ExceptionHelper.ExpectInvalidOperationException(delegate ()
             {
                 entity.ReadOnlyProperty = "x";
             }, "The 'ReadOnlyProperty' property is read only.");
@@ -54,7 +54,7 @@ public void Entity_Validation_Property_ReadOnly_Missing()
         public void Entity_Validation_Property_Fail_NonPublic_Throws()
         {
             MockEntity entity = new MockEntity();
-            ExceptionHelper.ExpectArgumentException(delegate()
+            ExceptionHelper.ExpectArgumentException(delegate ()
             {
                 entity.NonPublicProperty = "x";
             }, "Type 'MockEntity' does not contain a public property named 'NonPublicProperty'.\r\nParameter name: propertyName");
@@ -65,7 +65,7 @@ public void Entity_Validation_Property_Fail_NonPublic_Throws()
         public void Entity_Validation_Property_Fail_Bogus_Throws()
         {
             MockEntity entity = new MockEntity();
-            ExceptionHelper.ExpectArgumentException(delegate()
+            ExceptionHelper.ExpectArgumentException(delegate ()
             {
                 entity.BogusProperty = "x";
             }, "Type 'MockEntity' does not contain a public property named 'XXX'.\r\nParameter name: propertyName");
@@ -144,7 +144,7 @@ public void Entity_Validation_Object_Fail_Null_Required_Throws()
         {
             MockEntity entity = new MockEntity();
             ValidationContext context = new ValidationContext(entity, null, null);
-            ExceptionHelper.ExpectValidationException(delegate()
+            ExceptionHelper.ExpectValidationException(delegate ()
             {
                 System.ComponentModel.DataAnnotations.Validator.ValidateObject(entity, context);
             }, "The ReadWriteProperty field is required.", typeof(RequiredAttribute), null);
@@ -228,7 +228,7 @@ public void ValidationResult_SameErrorMessageDifferentMembers()
         [Description("When a ValidationContext is provided to the DomainContext, it should be used for Property Validation")]
         public void ValidationContextUsedForPropertyValidation()
         {
-            Dictionary<object, object> items = new Dictionary<object,object>();
+            Dictionary<object, object> items = new Dictionary<object, object>();
             items.Add("TestMethod", "ValidationContextUsedForPropertyValidation");
             ValidationContext providedValidationContext = new ValidationContext(this, null, items);
 
@@ -469,6 +469,43 @@ public void ValidationErrors_PropertyErrorsAreReplacedFromValidateProperty()
             actualChanges.Clear();
         }
 
+
+        [TestMethod]
+        [Description("Test how object level validation interacts with property level validation")]
+        public void ValidationErrors_ObjectLevelValidation()
+        {
+            MockEntity_ObjectValidation entity = new MockEntity_ObjectValidation();
+            INotifyDataErrorInfo notifier = entity;
+
+            List<string> actualChanges = new List<string>();
+            notifier.ErrorsChanged += (s, e) => actualChanges.Add(e.PropertyName);
+
+            // Setting property directly will not catch any error
+            entity.OnlyObjectValidation = string.Empty;
+
+            Assert.IsFalse(entity.HasValidationErrors, "No errors should be detected");
+            Assert.AreEqual(0, actualChanges.Count, "No errors should be detected");
+
+            // Force object level validation
+            IEditableObject editableObject = entity;
+            editableObject.BeginEdit();
+            editableObject.EndEdit();
+
+            Assert.IsTrue(entity.HasValidationErrors, "Erros should be detected");
+            Assert.AreEqual(1, entity.ValidationErrors.Count);
+
+            var actualError = entity.ValidationErrors.First();
+            Assert.AreEqual("Required", actualError.ErrorMessage);
+            CollectionAssert.AreEqual(new[] { nameof(MockEntity_ObjectValidation.OnlyObjectValidation) }, actualError.MemberNames.ToList(), "membernames");
+
+            Assert.AreEqual(1, actualChanges.Count, "1 errors should be detected");
+
+            // Setting property will remove object level validation
+            entity.OnlyObjectValidation = null;
+            Assert.IsFalse(entity.HasValidationErrors, "Errors should have been removed");
+            Assert.AreEqual(2, actualChanges.Count, "change notification should have happened when clearing error");
+        }
+
         [TestMethod]
         [WorkItem(854187)]
         [Description("Verifies that when an invalid entity's changes are rejected, the errors are cleared when RejectChanges is called")]
@@ -943,6 +980,27 @@ public void CallValidateProperty(ValidationContext validationContext, object val
         }
     }
 
+    public class MockEntity_ObjectValidation : Entity, IValidatableObject
+    {
+        private string _onlyObjectValidation;
+
+        public string OnlyObjectValidation
+        {
+            get => _onlyObjectValidation;
+            set
+            {
+                this.ValidateProperty(nameof(OnlyObjectValidation), value);
+                _onlyObjectValidation = value;
+            }
+        }
+
+        IEnumerable<ValidationResult> IValidatableObject.Validate(ValidationContext validationContext)
+        {
+            if (string.IsNullOrEmpty(_onlyObjectValidation))
+                yield return new ValidationResult("Required", new[] { nameof(OnlyObjectValidation) });
+        }
+    }
+
     public class MockEntity_Validation : MockEntity
     {
         public void Overload(int x)