[System] Improve ApplicationSettingsBase behavior (#6272)

* [System] Add unit test for Xamarin-2315

* [System] Use temporary directory for saving settings files

This change modifies the ApplicationSettingsBaseTests so that they
store the settings files in a temporary directory. This reduces
the risk of failed tests because of stale files from previous test
run, or from a build that runs in parallel.

* [System] Improve Settings behavior (Xamarin-15818)

Improve behavior of ApplicationSettingsBase class to match Windows .NET
more closely.
* Implement Upgrade to allow Upgrading of settings
* Make SettingsPropertyValue serialization work for complex objects
* Make Save and Update throw in unimplemented configurations
* Changed Reload to clear PropertyValues so they are reloaded
  and fire PropertyChanged event for all Properties
* Modified Reset to do what Reload used to do.
* Added a backup Type loading for custom SettingsProviders
* don't call SettingsSaving on Reset()

* [System] Fire SettingsSaving event (Xamarin-2315)

When calling ApplicationSettingsBase.Save() the SettingsSaving
event has to be fired.

* [System] Add unit tests for Xamarin-15818

5 files changed, 356 insertions, 11 deletions

diff --git a/mcs/class/System/System.Configuration/ApplicationSettingsBase.cs b/mcs/class/System/System.Configuration/ApplicationSettingsBase.cs
index 6b12f95425b..7e7676ab5c1 100644
--- a/mcs/class/System/System.Configuration/ApplicationSettingsBase.cs
+++ b/mcs/class/System/System.Configuration/ApplicationSettingsBase.cs
@@ -78,9 +78,13 @@ namespace System.Configuration {
 		public void Reload ()
 		{
 #if (CONFIGURATION_DEP)
-			foreach (SettingsProvider provider in Providers) {
-//				IApplicationSettingsProvider iasp = provider as IApplicationSettingsProvider;
-				CacheValuesByProvider(provider);
+			/* Clear out the old property values so they will be reloaded on request */
+			if (PropertyValues != null) {
+				PropertyValues.Clear();
+			}
+			foreach(SettingsProperty prop in Properties) {
+				/* emit PropertyChanged for every property */
+				OnPropertyChanged(this, new PropertyChangedEventArgs(prop.Name));
 			}
 #endif
 		}
@@ -88,13 +92,31 @@ namespace System.Configuration {
 		public void Reset()
 		{
 #if (CONFIGURATION_DEP)
+			if (Properties != null) {
+				foreach (SettingsProvider provider in Providers) {
+					IApplicationSettingsProvider iasp = provider as IApplicationSettingsProvider;
+					if (iasp != null)
+						iasp.Reset (Context);
+				}
+				InternalSave ();
+			}
+
 			Reload ();
-			foreach (SettingsPropertyValue pv in PropertyValues)
-				pv.PropertyValue = pv.Reset();
 #endif
 		}
 
-		public override void Save()
+		public override void Save ()
+		{
+			var e = new CancelEventArgs ();
+
+			OnSettingsSaving (this, e);
+			if (e.Cancel)
+				return;
+
+			InternalSave ();
+		}
+
+		void InternalSave ()
 		{
 #if (CONFIGURATION_DEP)
 			Context.CurrentSettings = this;
@@ -111,13 +133,42 @@ namespace System.Configuration {
 					provider.SetPropertyValues (Context, cache);
 			}
 			Context.CurrentSettings = null;
+#else
+			throw new NotImplementedException("No useful Save implemented.");
 #endif
 		}
 
-		public virtual void Upgrade()
+		public virtual void Upgrade ()
 		{
+#if (CONFIGURATION_DEP)
+			// if there is a current property, then for each settings
+			// provider in the providers collection, upgrade(ssp)
+			if (Properties != null) {
+				foreach (SettingsProvider provider in Providers) {
+					var appSettingsProvider = provider as IApplicationSettingsProvider;
+					if(appSettingsProvider != null) {
+						appSettingsProvider.Upgrade (Context, GetPropertiesForProvider (provider));
+					}
+				}
+			}
+			Reload ();
+#else
+			throw new NotImplementedException ("No useful Upgrade implemented");
+#endif
 		}
 
+		private SettingsPropertyCollection GetPropertiesForProvider (SettingsProvider provider)
+		{
+           SettingsPropertyCollection properties = new SettingsPropertyCollection ();
+           foreach (SettingsProperty sp in Properties) {
+               if (sp.Provider == provider) {
+                   properties.Add(sp);
+               }
+           }
+
+           return properties;
+        }
+
 		protected virtual void OnPropertyChanged (object sender, 
 							  PropertyChangedEventArgs e)
 		{
@@ -310,7 +361,17 @@ namespace System.Configuration {
 			foreach (Attribute a in prop.GetCustomAttributes (false)) {
 				/* the attributes we handle natively here */
 				if (a is SettingsProviderAttribute) {
-					Type provider_type = Type.GetType (((SettingsProviderAttribute)a).ProviderTypeName);
+					var providerTypeName = ((SettingsProviderAttribute)a).ProviderTypeName;
+					Type provider_type = Type.GetType (providerTypeName);
+					if(provider_type == null) { // Type failed to find the type by name
+						var typeNameParts = providerTypeName.Split('.');
+						if(typeNameParts.Length > 1) { //Load the assembly that providerTypeName claims
+							var assy = Assembly.Load(typeNameParts[0]);
+							if(assy != null) {
+								provider_type = assy.GetType(providerTypeName); //try to get the type from that Assembly
+							}
+						}
+					}
 					provider = (SettingsProvider) Activator.CreateInstance (provider_type);
 					provider.Initialize (null, null);
 				}
diff --git a/mcs/class/System/System.Configuration/CustomizableFileSettingsProvider.cs b/mcs/class/System/System.Configuration/CustomizableFileSettingsProvider.cs
index d74a226f1e7..bdbf0f816a3 100644
--- a/mcs/class/System/System.Configuration/CustomizableFileSettingsProvider.cs
+++ b/mcs/class/System/System.Configuration/CustomizableFileSettingsProvider.cs
@@ -819,6 +819,11 @@ namespace System.Configuration
 
 		public void Reset (SettingsContext context)
 		{
+			if (values == null) {
+				SettingsPropertyCollection coll = new SettingsPropertyCollection ();
+				GetPropertyValues (context, coll);
+			}
+
 			if (values != null) {
 				foreach (SettingsPropertyValue propertyValue in values) {
 					// Can't use propertyValue.Property.DefaultValue
diff --git a/mcs/class/System/System.Configuration/SettingsPropertyValue.cs b/mcs/class/System/System.Configuration/SettingsPropertyValue.cs
index 9bf62c00c62..a15ac24fa0d 100644
--- a/mcs/class/System/System.Configuration/SettingsPropertyValue.cs
+++ b/mcs/class/System/System.Configuration/SettingsPropertyValue.cs
@@ -45,6 +45,7 @@ namespace System.Configuration
 		{
 			this.property = property;
 			needPropertyValue = true;
+			needSerializedValue = true;
 		}
 
 		public bool Deserialized {
@@ -83,6 +84,8 @@ namespace System.Configuration
 					propertyValue = GetDeserializedValue (serializedValue);
 					if (propertyValue == null) {
 						propertyValue = GetDeserializedDefaultValue ();
+						serializedValue = null;
+						needSerializedValue = true;
 						defaulted = true;
 					}
 					needPropertyValue = false;
@@ -107,9 +110,7 @@ namespace System.Configuration
 
 		public object SerializedValue {
 			get {
-				if (needSerializedValue) {
-					needSerializedValue = false;
-
+				if ((needSerializedValue || IsDirty) && !UsingDefaultValue) {
 					switch (property.SerializeAs)
 					{
 					case SettingsSerializeAs.String:
@@ -143,6 +144,8 @@ namespace System.Configuration
 						break;
 					}
 
+					needSerializedValue = false;
+					dirty = false;
 				}
 
 				return serializedValue;
@@ -150,6 +153,7 @@ namespace System.Configuration
 			set {
 				serializedValue = value;
 				needPropertyValue = true;
+				needSerializedValue = false;
 			}
 		}
 
@@ -165,6 +169,7 @@ namespace System.Configuration
 			dirty = true;
 			defaulted = true;
 			needPropertyValue = true;
+			needSerializedValue = true;
 			return propertyValue;
 		}
 
diff --git a/mcs/class/System/Test/System.Configuration/ApplicationSettingsBaseTest.cs b/mcs/class/System/Test/System.Configuration/ApplicationSettingsBaseTest.cs
index 50b605adae7..f8bc1638830 100644
--- a/mcs/class/System/Test/System.Configuration/ApplicationSettingsBaseTest.cs
+++ b/mcs/class/System/Test/System.Configuration/ApplicationSettingsBaseTest.cs
@@ -164,6 +164,29 @@ namespace MonoTests.System.Configuration {
 	[TestFixture]
 	public class ApplicationSettingsBaseTest
 	{
+		string tempDir;
+
+		[TestFixtureSetUp]
+		public void FixtureSetup ()
+		{
+			// Use random temp directory to store settings files of tests.
+			tempDir = Path.Combine (Path.GetTempPath (), Path.GetRandomFileName ());
+			Directory.CreateDirectory (tempDir);
+			var localAppData = Path.Combine (tempDir, "LocalAppData");
+			Directory.CreateDirectory (localAppData);
+			var appData = Path.Combine (tempDir, "AppData");
+			Directory.CreateDirectory (appData);
+
+			Environment.SetEnvironmentVariable ("XDG_DATA_HOME", localAppData);
+			Environment.SetEnvironmentVariable ("XDG_CONFIG_HOME", appData);
+		}
+
+		[TestFixtureTearDown]
+		public void FixtureTearDown ()
+		{
+			Directory.Delete (tempDir);
+		}
+
 		[Test]
 		public void TestSettings1_Properties ()
 		{
@@ -522,6 +545,212 @@ namespace MonoTests.System.Configuration {
 				Assert.Fail ("Invalid data was saved to config file.");
 			}
 		}
+		#region Bug #2315
+		class Bug2315Settings : ApplicationSettingsBase
+		{
+			public Bug2315Settings () : base ("Bug2315Settings")
+			{
+			}
+
+			[UserScopedSetting]
+			[DefaultSettingValue ("some text")]
+			public string Text {
+				get { return (string)this ["Text"]; }
+				set { this ["Text"] = value; }
+			}
+		}
+
+		[Test]
+		public void SettingSavingEventFired_Bug2315 ()
+		{
+			bool settingsSavingCalled = false;
+			var settings = new Bug2315Settings ();
+			settings.SettingsSaving += (object sender, CancelEventArgs e) => {
+				settingsSavingCalled = true;
+			};
+
+			settings.Text = DateTime.Now.ToString ();
+			settings.Save ();
+
+			Assert.IsTrue (settingsSavingCalled);
+		}
+		#endregion
+
+		#region Bug #15818
+		class Bug15818SettingsProvider: SettingsProvider, IApplicationSettingsProvider
+		{
+			public Bug15818SettingsProvider ()
+			{
+			}
+
+			public static void ResetUpgradeCalled ()
+			{
+				UpgradeCalled = false;
+			}
+
+			public static bool UpgradeCalled { get; private set; }
+
+			public override void Initialize (string name, NameValueCollection config)
+			{
+				if (name != null && config != null) {
+					base.Initialize (name, config);
+				}
+			}
+
+			public override string Name
+			{
+				get { return "Bug15818SettingsProvider"; }
+			}
+
+			public override string Description
+			{
+				get { return "Bug15818SettingsProvider"; }
+			}
+
+			public override string ApplicationName
+			{
+				get { return "Bug15818"; }
+				set { }
+			}
+
+			public override SettingsPropertyValueCollection GetPropertyValues (SettingsContext context, SettingsPropertyCollection collection)
+			{
+				return null;
+			}
+
+			public override void SetPropertyValues (SettingsContext context, SettingsPropertyValueCollection collection)
+			{
+			}
+
+			#region IApplicationSettingsProvider implementation
+
+			public SettingsPropertyValue GetPreviousVersion (SettingsContext context, SettingsProperty property)
+			{
+				return null;
+			}
+
+			public void Reset (SettingsContext context)
+			{
+			}
+
+			public void Upgrade (SettingsContext context, SettingsPropertyCollection properties)
+			{
+				UpgradeCalled = true;
+			}
+
+			#endregion
+		}
+
+		class Bug15818Settings : ApplicationSettingsBase
+		{
+			public Bug15818Settings () : base ("Bug15818Settings")
+			{
+			}
+
+			[UserScopedSetting]
+			[SettingsProvider (typeof (Bug15818SettingsProvider))]
+			[DefaultSettingValue ("some text")]
+			public string Text {
+				get { return (string)this ["Text"]; }
+				set { this ["Text"] = value; }
+			}
+		}
+
+		public class Bug15818Class
+		{
+			public string Name { get; set; }
+			public int Value { get; set; }
+		}
+
+		class Bug15818Settings2 : ApplicationSettingsBase
+		{
+			public Bug15818Settings2 () : base ("Bug15818Settings2")
+			{
+			}
+
+			[UserScopedSetting]
+			[DefaultSettingValue ("default text")]
+			public string Text {
+				get { return (string)this ["Text"]; }
+				set { this ["Text"] = value; }
+			}
+
+			[UserScopedSetting]
+			public Bug15818Class MyObject {
+				get { return (Bug15818Class)this ["MyObject"]; }
+				set { this ["MyObject"] = value; }
+			}
+		}
+
+		[Test]
+		public void UpgradeGetsCalled_Bug15818 ()
+		{
+			Bug15818SettingsProvider.ResetUpgradeCalled ();
+
+			var settings = new Bug15818Settings ();
+			settings.Upgrade ();
+			Assert.IsTrue (Bug15818SettingsProvider.UpgradeCalled);
+		}
+
+		[Test]
+		public void CustomClass_Roundtrip ()
+		{
+			var settings = new Bug15818Settings2
+			{
+				Text = "foo",
+				MyObject = new Bug15818Class { Name = "Some Name", Value = 15818 }
+			};
+			settings.Save ();
+
+			var settings2 = new Bug15818Settings2 ();
+			Assert.AreEqual ("foo", settings2.Text);
+			Assert.IsNotNull (settings2.MyObject);
+			Assert.AreEqual ("Some Name", settings2.MyObject.Name);
+			Assert.AreEqual (15818, settings2.MyObject.Value);
+		}
+
+		[Test]
+		public void ModifiedObjectsAreSerialized_Bug15818 ()
+		{
+			var settings = new Bug15818Settings2
+			{
+				Text = "foo",
+				MyObject = new Bug15818Class { Name = "Some Name", Value = 15818 }
+			};
+			settings.Save ();
+
+			// Modify the value of the object - bug #15818
+			settings.Text = "bla";
+			settings.MyObject.Name = "xyz";
+			settings.MyObject.Value = -1;
+			settings.Save ();
+
+			// Verify that the new values got saved
+			var settings2 = new Bug15818Settings2 ();
+			Assert.AreEqual ("bla", settings2.Text);
+			Assert.IsNotNull (settings2.MyObject);
+			Assert.AreEqual ("xyz", settings2.MyObject.Name);
+			Assert.AreEqual (-1, settings2.MyObject.Value);
+		}
+
+		[Test]
+		public void Reset_FiresPropChangedOnly_Bug15818 ()
+		{
+			bool propChangedCalled = false;
+			bool settingsLoadedCalled = false;
+			bool settingsSavingCalled = false;
+			var settings = new Bug15818Settings2 ();
+			settings.PropertyChanged += (sender, e) => { propChangedCalled = true; };
+			settings.SettingsLoaded += (sender, e) => { settingsLoadedCalled = true; };
+			settings.SettingsSaving += (sender, e) => { settingsSavingCalled = true; };
+
+			settings.Reset ();
+
+			Assert.IsTrue (propChangedCalled, "#1");
+			Assert.IsFalse (settingsLoadedCalled, "#2");
+			Assert.IsFalse (settingsSavingCalled, "#3");
+		}
+		#endregion
 	}
 }
 
diff --git a/mcs/class/System/Test/System.Configuration/SettingsPropertyValueTest.cs b/mcs/class/System/Test/System.Configuration/SettingsPropertyValueTest.cs
index e954bf725a9..78308baf07f 100644
--- a/mcs/class/System/Test/System.Configuration/SettingsPropertyValueTest.cs
+++ b/mcs/class/System/Test/System.Configuration/SettingsPropertyValueTest.cs
@@ -82,6 +82,28 @@ namespace MonoTests.System.Configuration {
 		}
 
 		[Test]
+		public void Properties_ChangeSerialzeAs ()
+		{
+			SettingsProperty p = new SettingsProperty ("property",
+				typeof (int),
+				null,
+				true,
+				10,
+				SettingsSerializeAs.String,
+				null,
+				true,
+				false);
+
+			SettingsPropertyValue v = new SettingsPropertyValue (p);
+
+			// test that setting SerializeAs after changing v.PropertyValue causes
+			// SerializedValue to be in the new format
+			v.PropertyValue = (object)5;
+			p.SerializeAs = SettingsSerializeAs.Xml;
+			Assert.AreEqual ("<?xml version=\"1.0\" encoding=\"utf-16\"?>\n<int>5</int>", ((string)v.SerializedValue).Replace("\r\n", "\n"), "A99");
+		}
+
+		[Test]
 		public void Dirty ()
 		{
 			SettingsProperty p = new SettingsProperty ("property",
@@ -221,6 +243,29 @@ namespace MonoTests.System.Configuration {
 
 		}
 
+		/// <summary>
+		/// This tests the case where we have a SerializedValue but not a PropertyValue.
+		/// </summary>
+		[Test]
+		public void Xml_SerializeNoPropValue ()
+		{
+			SettingsProperty p = new SettingsProperty ("property",
+				typeof (MyData),
+				null,
+				true,
+				10,
+				SettingsSerializeAs.Xml,
+				null,
+				true,
+				false);
+
+			SettingsPropertyValue v = new SettingsPropertyValue (p);
+			v.SerializedValue = "<?xml version=\"1.0\" encoding=\"utf-16\"?>\n<int>10</int>";
+
+			Assert.AreEqual ("<?xml version=\"1.0\" encoding=\"utf-16\"?>\n<int>10</int>", v.SerializedValue);
+
+		}
+
 		[Test]
 		public void Binary_Serialize ()
 		{