Initial implementation of new validation approach

I've altered the way that the deserializers work and are defined so that they are simpler to create, but also so that we can record validation failures during the deserialization process. This allows us to provide much better validation messages than we can if we take the approach of validating based on the deserialized objects.

In this initial change I haven't gone ahead and updated all of the deserializers because I wanted the change to be kept small. I've also not actually hooked up the validation messages in any way - instead I've just provided a stub ErrorReporter object that doesn't actually do anything yet.

Part of tomkerkhove#592

src/Promitor.Core.Scraping/Configuration/Serialization/ConfigurationSerializer.cs

@@ -57,7 +57,8 @@ private MetricsDeclaration InterpretYamlStream(YamlStream metricsDeclarationYaml
             switch (specVersion)
             {
                 case SpecVersion.v1:
-                    var v1Config = _v1Deserializer.Deserialize(rootNode);
+                    var errorReporter = new ErrorReporter();
+                    var v1Config = _v1Deserializer.Deserialize(rootNode, errorReporter);
 
                     return _mapper.Map<MetricsDeclaration>(v1Config);
                 default:

src/Promitor.Core.Scraping/Configuration/Serialization/Deserializer.cs

@@ -1,4 +1,8 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Linq.Expressions;
+using System.Reflection;
 using System.Runtime.Serialization;
 using GuardNet;
 using Microsoft.Extensions.Logging;
@@ -7,7 +11,10 @@
 namespace Promitor.Core.Scraping.Configuration.Serialization
 {
     public abstract class Deserializer<TObject> : IDeserializer<TObject>
+        where TObject: new()
     {
+        private readonly Dictionary<string, FieldContext> _fields = new Dictionary<string, FieldContext>();
+
         protected ILogger Logger { get; }
 
         protected Deserializer(ILogger logger)
@@ -17,9 +24,41 @@ protected Deserializer(ILogger logger)
             Logger = logger;
         }
 
-        public abstract TObject Deserialize(YamlMappingNode node);
+        /// <inheritdoc />
+        public virtual TObject Deserialize(YamlMappingNode node, IErrorReporter errorReporter)
+        {
+            var result = new TObject();
+
+            foreach (var child in node.Children)
+            {
+                if (_fields.TryGetValue(child.Key.ToString(), out var fieldContext))
+                {
+                    fieldContext.SetValue(
+                        result, GetFieldValue(child, fieldContext, errorReporter));
+                }
+                else
+                {
+                    errorReporter.ReportWarning(child.Key, $"Unknown field '{child.Key}'.");
+                }
+            }
+
+            foreach (var unsetField in _fields.Where(field => !field.Value.HasBeenSet))
+            {
+                if (unsetField.Value.IsRequired)
+                {
+                    errorReporter.ReportError(node, $"'{unsetField.Key}' is a required field but was not found.");
+                }
+                else
+                {
+                    unsetField.Value.SetDefaultValue(result);
+                }
+            }
+
+            return result;
+        }
 
-        public List<TObject> Deserialize(YamlSequenceNode nodes)
+        /// <inheritdoc />
+        public List<TObject> Deserialize(YamlSequenceNode nodes, IErrorReporter errorReporter)
         {
             Guard.NotNull(nodes, nameof(nodes));
 
@@ -31,11 +70,106 @@ public List<TObject> Deserialize(YamlSequenceNode nodes)
                     throw new SerializationException($"Failed parsing metrics because we couldn't cast an item to {nameof(YamlMappingNode)}");
                 }
 
-                var deserializedObject = Deserialize(metricNode);
+                var deserializedObject = Deserialize(metricNode, errorReporter);
                 deserializedObjects.Add(deserializedObject);
             }
 
             return deserializedObjects;
         }
+
+        protected void MapRequired<TReturn>(Expression<Func<TObject, TReturn>> accessorExpression, Func<string, KeyValuePair<YamlNode, YamlNode>, IErrorReporter, object> customMapperFunc = null)
+        {
+            var memberExpression = (MemberExpression)accessorExpression.Body;
+            _fields[GetName(memberExpression)] = new FieldContext(memberExpression.Member as PropertyInfo, true, default(TReturn), customMapperFunc);
+        }
+
+        protected void MapOptional<TReturn>(Expression<Func<TObject, TReturn>> accessorExpression, TReturn defaultValue = default, Func<string, KeyValuePair<YamlNode, YamlNode>, IErrorReporter, object> customMapperFunc = null)
+        {
+            var memberExpression = (MemberExpression)accessorExpression.Body;
+            _fields[GetName(memberExpression)] = new FieldContext(memberExpression.Member as PropertyInfo, false, defaultValue, customMapperFunc);
+        }
+
+        private static string GetName(MemberExpression memberExpression)
+        {
+            return Char.ToLowerInvariant(memberExpression.Member.Name[0]) + memberExpression.Member.Name.Substring(1);
+        }
+
+        private static object GetFieldValue(
+            KeyValuePair<YamlNode, YamlNode> fieldNodePair, FieldContext fieldContext, IErrorReporter errorReporter)
+        {
+            if (fieldContext.CustomMapperFunc != null)
+            {
+                return fieldContext.CustomMapperFunc(fieldNodePair.Value.ToString(), fieldNodePair, errorReporter);
+            }
+
+            var propertyType = Nullable.GetUnderlyingType(fieldContext.PropertyInfo.PropertyType) ?? fieldContext.PropertyInfo.PropertyType;
+            if (propertyType.IsEnum)
+            {
+                var parseSucceeded = System.Enum.TryParse(
+                    propertyType, fieldNodePair.Value.ToString(), out var enumValue);
+
+                if (!parseSucceeded)
+                {
+                    errorReporter.ReportError(fieldNodePair.Value, $"'{fieldNodePair.Value}' is not a valid value for '{fieldNodePair.Key}'.");
+                }
+
+                return enumValue;
+            }
+
+            if (typeof(IDictionary<string, string>).IsAssignableFrom(propertyType))
+            {
+                return ((YamlMappingNode)fieldNodePair.Value).GetDictionary();
+            }
+
+            if (propertyType == typeof(TimeSpan))
+            {
+                var parseSucceeded = TimeSpan.TryParse(fieldNodePair.Value.ToString(), out var timeSpanValue);
+                if (!parseSucceeded)
+                {
+                    errorReporter.ReportError(fieldNodePair.Value, $"'{fieldNodePair.Value}' is not a valid value for '{fieldNodePair.Key}'. The value must be in the format 'hh:mm:ss'.");
+                }
+
+                return timeSpanValue;
+            }
+
+            try
+            {
+                return Convert.ChangeType(fieldNodePair.Value.ToString(), fieldContext.PropertyInfo.PropertyType);
+            }
+            catch (FormatException)
+            {
+                errorReporter.ReportError(fieldNodePair.Value, $"'{fieldNodePair.Value}' is not a valid value for '{fieldNodePair.Key}'. The value must be of type {fieldContext.PropertyInfo.PropertyType}.");
+            }
+
+            return null;
+        }
+
+        private class FieldContext
+        {
+            public FieldContext(PropertyInfo propertyInfo, bool isRequired, object defaultValue, Func<string, KeyValuePair<YamlNode, YamlNode>, IErrorReporter, object> customMapperFunc)
+            {
+                CustomMapperFunc = customMapperFunc;
+                PropertyInfo = propertyInfo;
+                IsRequired = isRequired;
+                DefaultValue = defaultValue;
+            }
+
+            public bool HasBeenSet { get; private set; }
+            public PropertyInfo PropertyInfo { get; }
+            public bool IsRequired { get; }
+            public object DefaultValue { get; }
+            public Func<string, KeyValuePair<YamlNode, YamlNode>, IErrorReporter, object> CustomMapperFunc { get; }
+
+            public void SetValue(TObject target, object value)
+            {
+                PropertyInfo.SetValue(target, value);
+                HasBeenSet = true;
+            }
+
+            public void SetDefaultValue(TObject target)
+            {
+                PropertyInfo.SetValue(target, DefaultValue);
+            }
+        }
     }
 }

src/Promitor.Core.Scraping/Configuration/Serialization/ErrorReporter.cs

@@ -0,0 +1,15 @@
+using YamlDotNet.RepresentationModel;
+
+namespace Promitor.Core.Scraping.Configuration.Serialization
+{
+    public class ErrorReporter : IErrorReporter
+    {
+        public void ReportError(YamlNode node, string message)
+        {
+        }
+
+        public void ReportWarning(YamlNode node, string message)
+        {
+        }
+    }
+}

src/Promitor.Core.Scraping/Configuration/Serialization/IDeserializer.cs

@@ -7,20 +7,22 @@ namespace Promitor.Core.Scraping.Configuration.Serialization
     /// An object that can deserialize a yaml node into an object.
     /// </summary>
     /// <typeparam name="TObject">The type of object that can be deserialized.</typeparam>
-    public interface IDeserializer<TObject>
+    public interface IDeserializer<TObject> where TObject: new()
     {
         /// <summary>
         /// Deserializes the specified node.
         /// </summary>
         /// <param name="node">The node to deserialize.</param>
+        /// <param name="errorReporter">Used to report deserialization errors.</param>
         /// <returns>The deserialized object.</returns>
-        TObject Deserialize(YamlMappingNode node);
+        TObject Deserialize(YamlMappingNode node, IErrorReporter errorReporter);
 
         /// <summary>
         /// Deserializes an array of elements.
         /// </summary>
         /// <param name="node">The node to deserialize.</param>
+        /// <param name="errorReporter">Used to report deserialization errors.</param>
         /// <returns>The deserialized objects.</returns>
-        List<TObject> Deserialize(YamlSequenceNode node);
+        List<TObject> Deserialize(YamlSequenceNode node, IErrorReporter errorReporter);
     }
 }

src/Promitor.Core.Scraping/Configuration/Serialization/IErrorReporter.cs

@@ -0,0 +1,24 @@
+using YamlDotNet.RepresentationModel;
+
+namespace Promitor.Core.Scraping.Configuration.Serialization
+{
+    /// <summary>
+    /// Used to report errors discovered during the deserialization process.
+    /// </summary>
+    public interface IErrorReporter
+    {
+        /// <summary>
+        /// Reports an error.
+        /// </summary>
+        /// <param name="node">The node containing the error / that should be highlighted to the user.</param>
+        /// <param name="message">The error message.</param>
+        void ReportError(YamlNode node, string message);
+
+        /// <summary>
+        /// Reports a warning (i.e. something that doesn't prevent Promitor from functioning).
+        /// </summary>
+        /// <param name="node">The node containing the error / that should be highlighted to the user.</param>
+        /// <param name="message">The error message.</param>
+        void ReportWarning(YamlNode node, string message);
+    }
+}

src/Promitor.Core.Scraping/Configuration/Serialization/YamlMappingNodeExtensions.cs

@@ -40,6 +40,24 @@ public static string GetString(this YamlMappingNode node, string propertyName)
             return null;
         }
 
+        /// <summary>
+        /// Gets the contents of the node.
+        /// </summary>
+        /// <param name="node">The node containing the property.</param>
+        /// <param name="propertyName">The name of the property.</param>
+        /// <returns>The child items of the property as a dictionary.</returns>
+        public static Dictionary<string, string> GetDictionary(this YamlMappingNode node)
+        {
+            var result = new Dictionary<string, string>();
+
+            foreach (var (key, value) in node.Children)
+            {
+                result[key.ToString()] = value.ToString();
+            }
+
+            return result;
+        }
+
         /// <summary>
         /// Gets the contents of the specified property as a dictionary.
         /// </summary>
@@ -50,14 +68,7 @@ public static Dictionary<string, string> GetDictionary(this YamlMappingNode node
         {
             if (node.Children.TryGetValue(propertyName, out var propertyNode))
             {
-                var result = new Dictionary<string, string>();
-
-                foreach (var (key, value) in ((YamlMappingNode) propertyNode).Children)
-                {
-                    result[key.ToString()] = value.ToString();
-                }
-
-                return result;
+                return GetDictionary(((YamlMappingNode)propertyNode));
             }
 
             return null;
@@ -86,14 +97,15 @@ public static Dictionary<string, string> GetDictionary(this YamlMappingNode node
         /// <param name="node">The yaml node.</param>
         /// <param name="propertyName">The name of the property to deserialize.</param>
         /// <param name="deserializer">The deserializer to use.</param>
+        /// <param name="errorReporter">Used to report information about the deserialization process.</param>
         /// <returns>The deserialized property, or null if the property does not exist.</returns>
         public static TObject DeserializeChild<TObject>(
-            this YamlMappingNode node, string propertyName, IDeserializer<TObject> deserializer)
-            where TObject: class
+            this YamlMappingNode node, string propertyName, IDeserializer<TObject> deserializer, IErrorReporter errorReporter)
+            where TObject: class, new()
         {
             if (node.Children.TryGetValue(propertyName, out var propertyNode))
             {
-                return deserializer.Deserialize((YamlMappingNode)propertyNode);
+                return deserializer.Deserialize((YamlMappingNode)propertyNode, errorReporter);
             }
 
             return null;

src/Promitor.Core.Scraping/Configuration/Serialization/v1/Core/AggregationDeserializer.cs

@@ -1,35 +1,20 @@
 using System;
 using Microsoft.Extensions.Logging;
 using Promitor.Core.Scraping.Configuration.Serialization.v1.Model;
-using YamlDotNet.RepresentationModel;
 
 namespace Promitor.Core.Scraping.Configuration.Serialization.v1.Core
 {
     public class AggregationDeserializer : Deserializer<AggregationV1>
     {
         private const string IntervalTag = "interval";
 
-        private readonly TimeSpan _defaultAggregationInterval = TimeSpan.FromMinutes(5);
+        private static readonly TimeSpan DefaultAggregationInterval = TimeSpan.FromMinutes(5);
 
         public AggregationDeserializer(ILogger<AggregationDeserializer> logger) : base(logger)
         {
+            MapOptional(aggregation => aggregation.Interval, DefaultAggregationInterval);
         }
 
-        public override AggregationV1 Deserialize(YamlMappingNode node)
-        {
-            var interval = node.GetTimeSpan(IntervalTag);
-
-            var aggregation = new AggregationV1 {Interval = interval};
-
-            if (aggregation.Interval == null)
-            {
-                aggregation.Interval = _defaultAggregationInterval;
-                Logger.LogWarning(
-                    "No default aggregation was configured, falling back to {AggregationInterval}",
-                    aggregation.Interval?.ToString("g"));
-            }
-
-            return aggregation;
-        }
+        // TODO: Figure out if we want to make Interval required depending on the context
     }
 }