Fixes #2944. TreeView unit tests fail intermittently: Ensures Attribute and Color are read only value types

Terminal.Gui/Configuration/AttributeJsonConverter.cs

@@ -1,99 +1,97 @@
 using System;
 using System.Text.Json;
 using System.Text.Json.Serialization;
-using Terminal.Gui;
 
-namespace Terminal.Gui {
-	/// <summary>
-	/// Json converter fro the <see cref="Attribute"/> class.
-	/// </summary>
-	class AttributeJsonConverter : JsonConverter<Attribute> {
-		private static AttributeJsonConverter instance;
+namespace Terminal.Gui; 
 
-		/// <summary>
-		/// 
-		/// </summary>
-		public static AttributeJsonConverter Instance {
-			get {
-				if (instance == null) {
-					instance = new AttributeJsonConverter ();
-				}
+/// <summary>
+/// Json converter fro the <see cref="Attribute"/> class.
+/// </summary>
+class AttributeJsonConverter : JsonConverter<Attribute> {
+	static AttributeJsonConverter _instance;
 
-				return instance;
+	/// <summary>
+	/// 
+	/// </summary>
+	public static AttributeJsonConverter Instance {
+		get {
+			if (_instance == null) {
+				_instance = new AttributeJsonConverter ();
 			}
+
+			return _instance;
 		}
+	}
 
-		public override Attribute Read (ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
-		{
-			if (reader.TokenType != JsonTokenType.StartObject) {
-				throw new JsonException ($"Unexpected StartObject token when parsing Attribute: {reader.TokenType}.");
-			}
+	public override Attribute Read (ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+	{
+		if (reader.TokenType != JsonTokenType.StartObject) {
+			throw new JsonException ($"Unexpected StartObject token when parsing Attribute: {reader.TokenType}.");
+		}
 
-			Attribute attribute = new Attribute ();
-			Color foreground = null;
-			Color background = null;
-			while (reader.Read ()) {
-				if (reader.TokenType == JsonTokenType.EndObject) {
-					if (foreground == null || background == null) {
-						throw new JsonException ($"Both Foreground and Background colors must be provided.");
-					} 
-					return new Attribute (foreground, background);
+		var attribute = new Attribute ();
+		Color? foreground = null;
+		Color? background = null;
+		while (reader.Read ()) {
+			if (reader.TokenType == JsonTokenType.EndObject) {
+				if (foreground == null || background == null) {
+					throw new JsonException ("Both Foreground and Background colors must be provided.");
 				}
+				return new Attribute (foreground.Value, background.Value);
+			}
 
-				if (reader.TokenType != JsonTokenType.PropertyName) {
-					throw new JsonException ($"Unexpected token when parsing Attribute: {reader.TokenType}.");
-				}
+			if (reader.TokenType != JsonTokenType.PropertyName) {
+				throw new JsonException ($"Unexpected token when parsing Attribute: {reader.TokenType}.");
+			}
 
-				string propertyName = reader.GetString ();
-				reader.Read ();
-				string color = $"\"{reader.GetString ()}\"";
+			var propertyName = reader.GetString ();
+			reader.Read ();
+			var color = $"\"{reader.GetString ()}\"";
 
-				switch (propertyName.ToLower ()) {
-				case "foreground":
-					foreground = JsonSerializer.Deserialize<Color> (color, options);
-					break;
-				case "background":
-					background = JsonSerializer.Deserialize<Color> (color, options);
-					break;
-				//case "bright":
-				//case "bold":
-				//	attribute.Bright = reader.GetBoolean ();
-				//	break;
-				//case "dim":
-				//	attribute.Dim = reader.GetBoolean ();
-				//	break;
-				//case "underline":
-				//	attribute.Underline = reader.GetBoolean ();
-				//	break;
-				//case "blink":
-				//	attribute.Blink = reader.GetBoolean ();
-				//	break;
-				//case "reverse":
-				//	attribute.Reverse = reader.GetBoolean ();
-				//	break;
-				//case "hidden":
-				//	attribute.Hidden = reader.GetBoolean ();
-				//	break;
-				//case "strike-through":
-				//	attribute.StrikeThrough = reader.GetBoolean ();
-				//	break;				
-				default:
-					throw new JsonException ($"Unknown Attribute property {propertyName}.");
-				}
+			switch (propertyName?.ToLower ()) {
+			case "foreground":
+				foreground = JsonSerializer.Deserialize<Color> (color, options);
+				break;
+			case "background":
+				background = JsonSerializer.Deserialize<Color> (color, options);
+				break;
+			//case "bright":
+			//case "bold":
+			//	attribute.Bright = reader.GetBoolean ();
+			//	break;
+			//case "dim":
+			//	attribute.Dim = reader.GetBoolean ();
+			//	break;
+			//case "underline":
+			//	attribute.Underline = reader.GetBoolean ();
+			//	break;
+			//case "blink":
+			//	attribute.Blink = reader.GetBoolean ();
+			//	break;
+			//case "reverse":
+			//	attribute.Reverse = reader.GetBoolean ();
+			//	break;
+			//case "hidden":
+			//	attribute.Hidden = reader.GetBoolean ();
+			//	break;
+			//case "strike-through":
+			//	attribute.StrikeThrough = reader.GetBoolean ();
+			//	break;				
+			default:
+				throw new JsonException ($"Unknown Attribute property {propertyName}.");
 			}
-			throw new JsonException ();
-		}
-
-		public override void Write (Utf8JsonWriter writer, Attribute value, JsonSerializerOptions options)
-		{
-			writer.WriteStartObject ();
-			writer.WritePropertyName (nameof(Attribute.Foreground));
-			ColorJsonConverter.Instance.Write (writer, value.Foreground, options);
-			writer.WritePropertyName (nameof (Attribute.Background));
-			ColorJsonConverter.Instance.Write (writer, value.Background, options);
-
-			writer.WriteEndObject ();
 		}
+		throw new JsonException ();
 	}
-}
 
+	public override void Write (Utf8JsonWriter writer, Attribute value, JsonSerializerOptions options)
+	{
+		writer.WriteStartObject ();
+		writer.WritePropertyName (nameof (Attribute.Foreground));
+		ColorJsonConverter.Instance.Write (writer, value.Foreground, options);
+		writer.WritePropertyName (nameof (Attribute.Background));
+		ColorJsonConverter.Instance.Write (writer, value.Background, options);
+
+		writer.WriteEndObject ();
+	}
+}

Terminal.Gui/Configuration/ConfigProperty.cs

@@ -1,31 +1,41 @@
-using System;
+#nullable enable
+
+using System;
 using System.Reflection;
 using System.Text.Json;
 using System.Text.Json.Serialization;
 
-#nullable enable
-
 namespace Terminal.Gui;
 
 /// <summary>
-/// Holds a property's value and the <see cref="PropertyInfo"/> that allows <see cref="ConfigurationManager"/> 
+/// Holds a property's value and the <see cref="PropertyInfo"/> that allows <see cref="ConfigurationManager"/>
 /// to get and set the property's value.
 /// </summary>
 /// <remarks>
-/// Configuration properties must be <see langword="public"/> and <see langword="static"/> 
+/// Configuration properties must be <see langword="public"/> and <see langword="static"/>
 /// and have the <see cref="SerializableConfigurationProperty"/>
-/// attribute. If the type of the property requires specialized JSON serialization, 
-/// a <see cref="JsonConverter"/> must be provided using 
+/// attribute. If the type of the property requires specialized JSON serialization,
+/// a <see cref="JsonConverter"/> must be provided using
 /// the <see cref="JsonConverterAttribute"/> attribute.
 /// </remarks>
 public class ConfigProperty {
-	private object? propertyValue;
 
 	/// <summary>
 	/// Describes the property.
 	/// </summary>
 	public PropertyInfo? PropertyInfo { get; set; }
 
+	/// <summary>
+	/// Holds the property's value as it was either read from the class's implementation or from a config file.
+	/// If the property has not been set (e.g. because no configuration file specified a value),
+	/// this will be <see langword="null"/>.
+	/// </summary>
+	/// <remarks>
+	/// On <see langword="set"/>, performs a sparse-copy of the new value to the existing value (only copies elements of
+	/// the object that are non-null).
+	/// </remarks>
+	public object? PropertyValue { get; set; }
+
 	/// <summary>
 	/// Helper to get either the Json property named (specified by [JsonPropertyName(name)]
 	/// or the actual property name.
@@ -38,34 +48,18 @@ public static string GetJsonPropertyName (PropertyInfo pi)
 		return jpna?.Name ?? pi.Name;
 	}
 
-	/// <summary>
-	/// Holds the property's value as it was either read from the class's implementation or from a config file. 
-	/// If the property has not been set (e.g. because no configuration file specified a value), 
-	/// this will be <see langword="null"/>.
-	/// </summary>
-	/// <remarks>
-	/// On <see langword="set"/>, performs a sparse-copy of the new value to the existing value (only copies elements of 
-	/// the object that are non-null).
-	/// </remarks>
-	public object? PropertyValue {
-		get => propertyValue;
-		set {
-			propertyValue = value;
-		}
-	}
-
 	internal object? UpdateValueFrom (object source)
 	{
 		if (source == null) {
 			return PropertyValue;
 		}
 
 		var ut = Nullable.GetUnderlyingType (PropertyInfo!.PropertyType);
-		if (source.GetType () != PropertyInfo!.PropertyType && (ut != null && source.GetType () != ut)) {
+		if (source.GetType () != PropertyInfo!.PropertyType && ut != null && source.GetType () != ut) {
 			throw new ArgumentException ($"The source object ({PropertyInfo!.DeclaringType}.{PropertyInfo!.Name}) is not of type {PropertyInfo!.PropertyType}.");
 		}
-		if (PropertyValue != null && source != null) {
-			PropertyValue = ConfigurationManager.DeepMemberwiseCopy (source, PropertyValue);
+		if (PropertyValue != null) {
+			PropertyValue = DeepMemberwiseCopy (source, PropertyValue);
 		} else {
 			PropertyValue = source;
 		}
@@ -78,10 +72,7 @@ public object? PropertyValue {
 	/// into <see cref="PropertyValue"/>.
 	/// </summary>
 	/// <returns></returns>
-	public object? RetrieveValue ()
-	{
-		return PropertyValue = PropertyInfo!.GetValue (null);
-	}
+	public object? RetrieveValue () => PropertyValue = PropertyInfo!.GetValue (null);
 
 	/// <summary>
 	/// Applies the <see cref="PropertyValue"/> to the property described by <see cref="PropertyInfo"/>.
@@ -91,22 +82,23 @@ public bool Apply ()
 	{
 		if (PropertyValue != null) {
 			try {
-				PropertyInfo?.SetValue (null, ConfigurationManager.DeepMemberwiseCopy (PropertyValue, PropertyInfo?.GetValue (null)));
+				PropertyInfo?.SetValue (null, DeepMemberwiseCopy (PropertyValue, PropertyInfo?.GetValue (null)));
 			} catch (TargetInvocationException tie) {
 				// Check if there is an inner exception
 				if (tie.InnerException != null) {
 					// Handle the inner exception separately without catching the outer exception
-					Exception innerException = tie.InnerException;
+					var innerException = tie.InnerException;
 
 					// Handle the inner exception here
 					throw new JsonException ($"Error Applying Configuration Change: {innerException.Message}", innerException);
 				}
 
 				// Handle the outer exception or rethrow it if needed
 				throw new JsonException ($"Error Applying Configuration Change: {tie.Message}", tie);
+			} catch (ArgumentException ae) {
+				throw new JsonException ($"Error Applying Configuration Change ({PropertyInfo?.Name}): {ae.Message}", ae);
 			}
 		}
 		return PropertyValue != null;
 	}
-
-}
+}