Skip to content

Commit

Permalink
User/ryanbod/mock settings disk access (#6188)
Browse files Browse the repository at this point in the history
* 1) Making Directory Methods private.
2) Removing the CreateDirectory / DeleteDirectory functionality from all Settings Unit Tests.

* Abstracting disk access via IIOProvider to be able to provide mocks for unit tests instead of writing to disk.   This also prevents developers who are running unit tests from interfering with the PowerToys settings on their local dev box.

* Dependency Injecting stub SettingsUtils for all tests

* Removing ISettingsUtils from constructors of objects that need to be deserialized (ColorPickerSettings/PowerLauncherSettings) as this breaks System.Text.Json

* Removing unused namespace reference

* Removing redifined mock

* As per PR feedback.  Stub Settings utils should work with any settings type if the intent is to compile / avoid null ref exceptions.

Strangely when implementing this fix it became apparent that a stub settings isn't enough, and disk access needed to be mocked.  I can't explain why the tests were passing previously.

* Leveraging GetMockIOProviderForSaveLoadExists
  • Loading branch information
ryanbodrug-microsoft committed Sep 21, 2020
1 parent 6e89ef6 commit 0f6428e
Show file tree
Hide file tree
Showing 40 changed files with 468 additions and 435 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.PowerToys.Settings.UI.Lib.Utilities;

namespace Microsoft.PowerToys.Settings.UI.Lib
{
Expand All @@ -21,15 +23,15 @@ public ColorPickerSettings()
Name = ModuleName;
}

public virtual void Save()
public virtual void Save(ISettingsUtils settingsUtils)
{
// Save settings to file
var options = new JsonSerializerOptions
{
WriteIndented = true,
};

SettingsUtils.SaveSettings(JsonSerializer.Serialize(this, options), ModuleName);
settingsUtils.SaveSettings(JsonSerializer.Serialize(this, options), ModuleName);
}
}
}
17 changes: 17 additions & 0 deletions src/core/Microsoft.PowerToys.Settings.UI.Lib/ISettingsUtils.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.PowerToys.Settings.UI.Lib
{
public interface ISettingsUtils
{
T GetSettings<T>(string powertoy = "", string fileName = "settings.json");

void SaveSettings(string jsonSettings, string powertoy = "", string fileName = "settings.json");

bool SettingsExists(string powertoy = "", string fileName = "settings.json");

void DeleteSettings(string powertoy = "");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.PowerToys.Settings.UI.Lib.Utilities;

namespace Microsoft.PowerToys.Settings.UI.Lib
{
Expand All @@ -21,15 +23,15 @@ public PowerLauncherSettings()
Name = ModuleName;
}

public virtual void Save()
public virtual void Save(ISettingsUtils settingsUtils)
{
// Save settings to file
var options = new JsonSerializerOptions
{
WriteIndented = true,
};

SettingsUtils.SaveSettings(JsonSerializer.Serialize(this, options), ModuleName);
settingsUtils.SaveSettings(JsonSerializer.Serialize(this, options), ModuleName);
}
}
}
41 changes: 23 additions & 18 deletions src/core/Microsoft.PowerToys.Settings.UI.Lib/SettingsUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,35 @@
// See the LICENSE file in the project root for more information.

using System;
using System.IO;
using System.Runtime.Serialization;
using System.Text.Json;
using Microsoft.PowerToys.Settings.UI.Lib.Utilities;

namespace Microsoft.PowerToys.Settings.UI.Lib
{
public static class SettingsUtils
public class SettingsUtils : ISettingsUtils
{
private const string DefaultFileName = "settings.json";
private const string DefaultModuleName = "";
private IIOProvider _ioProvider;

public static void DeleteSettings(string powertoy, string fileName = DefaultFileName)
public SettingsUtils(IIOProvider ioProvider)
{
File.Delete(GetSettingsPath(powertoy, fileName));
_ioProvider = ioProvider ?? throw new ArgumentNullException(nameof(ioProvider));
}

public static bool SettingsFolderExists(string powertoy)
private bool SettingsFolderExists(string powertoy)
{
return Directory.Exists(Path.Combine(LocalApplicationDataFolder(), $"Microsoft\\PowerToys\\{powertoy}"));
return _ioProvider.DirectoryExists(System.IO.Path.Combine(LocalApplicationDataFolder(), $"Microsoft\\PowerToys\\{powertoy}"));
}

public static void CreateSettingsFolder(string powertoy)
private void CreateSettingsFolder(string powertoy)
{
Directory.CreateDirectory(Path.Combine(LocalApplicationDataFolder(), $"Microsoft\\PowerToys\\{powertoy}"));
_ioProvider.CreateDirectory(System.IO.Path.Combine(LocalApplicationDataFolder(), $"Microsoft\\PowerToys\\{powertoy}"));
}

public void DeleteSettings(string powertoy = "")
{
_ioProvider.DeleteDirectory(System.IO.Path.Combine(LocalApplicationDataFolder(), $"Microsoft\\PowerToys\\{powertoy}"));
}

/// <summary>
Expand All @@ -37,38 +42,38 @@ public static string GetSettingsPath(string powertoy, string fileName = DefaultF
{
if (string.IsNullOrWhiteSpace(powertoy))
{
return Path.Combine(
return System.IO.Path.Combine(
LocalApplicationDataFolder(),
$"Microsoft\\PowerToys\\{fileName}");
}

return Path.Combine(
return System.IO.Path.Combine(
LocalApplicationDataFolder(),
$"Microsoft\\PowerToys\\{powertoy}\\{fileName}");
}

public static bool SettingsExists(string powertoy = DefaultModuleName, string fileName = DefaultFileName)
public bool SettingsExists(string powertoy = DefaultModuleName, string fileName = DefaultFileName)
{
return File.Exists(GetSettingsPath(powertoy, fileName));
return _ioProvider.FileExists(GetSettingsPath(powertoy, fileName));
}

/// <summary>
/// Get a Deserialized object of the json settings string.
/// </summary>
/// <returns>Deserialized json settings object.</returns>
public static T GetSettings<T>(string powertoy = DefaultModuleName, string fileName = DefaultFileName)
public T GetSettings<T>(string powertoy = DefaultModuleName, string fileName = DefaultFileName)
{
// Adding Trim('\0') to overcome possible NTFS file corruption.
// Look at issue https://github.com/microsoft/PowerToys/issues/6413 you'll see the file has a large sum of \0 to fill up a 4096 byte buffer for writing to disk
// This, while not totally ideal, does work around the problem by trimming the end.
// The file itself did write the content correctly but something is off with the actual end of the file, hence the 0x00 bug
var jsonSettingsString = File.ReadAllText(GetSettingsPath(powertoy, fileName)).Trim('\0');
var jsonSettingsString = _ioProvider.ReadAllText(GetSettingsPath(powertoy, fileName)).Trim('\0');

return JsonSerializer.Deserialize<T>(jsonSettingsString);
}

// Save settings to a json file.
public static void SaveSettings(string jsonSettings, string powertoy = DefaultModuleName, string fileName = DefaultFileName)
public void SaveSettings(string jsonSettings, string powertoy = DefaultModuleName, string fileName = DefaultFileName)
{
try
{
Expand All @@ -79,15 +84,15 @@ public static void SaveSettings(string jsonSettings, string powertoy = DefaultMo
CreateSettingsFolder(powertoy);
}

File.WriteAllText(GetSettingsPath(powertoy, fileName), jsonSettings);
_ioProvider.WriteAllText(GetSettingsPath(powertoy, fileName), jsonSettings);
}
}
catch
{
}
}

public static string LocalApplicationDataFolder()
private static string LocalApplicationDataFolder()
{
return Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.PowerToys.Settings.UI.Lib.Utilities
{
public interface IIOProvider
{
bool FileExists(string path);

bool DirectoryExists(string path);

bool CreateDirectory(string path);

void DeleteDirectory(string path);

void WriteAllText(string path, string content);

string ReadAllText(string path);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.IO;

namespace Microsoft.PowerToys.Settings.UI.Lib.Utilities
{
public class SystemIOProvider : IIOProvider
{
public bool CreateDirectory(string path)
{
var directioryInfo = Directory.CreateDirectory(path);
return directioryInfo != null;
}

public void DeleteDirectory(string path)
{
Directory.Delete(path);
}

public bool DirectoryExists(string path)
{
return Directory.Exists(path);
}

public bool FileExists(string path)
{
return File.Exists(path);
}

public string ReadAllText(string path)
{
return File.ReadAllText(path);
}

public void WriteAllText(string path, string content)
{
File.WriteAllText(path, content);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,33 @@
using System;
using System.Text.Json;
using Microsoft.PowerToys.Settings.UI.Lib.Helpers;
using Microsoft.PowerToys.Settings.UI.Lib.Utilities;

namespace Microsoft.PowerToys.Settings.UI.Lib.ViewModels
{
public class ColorPickerViewModel : Observable
{
private readonly ISettingsUtils _settingsUtils;
private ColorPickerSettings _colorPickerSettings;
private bool _isEnabled;

private Func<string, int> SendConfigMSG { get; }

public ColorPickerViewModel(Func<string, int> ipcMSGCallBackFunc)
public ColorPickerViewModel(ISettingsUtils settingsUtils, Func<string, int> ipcMSGCallBackFunc)
{
if (SettingsUtils.SettingsExists(ColorPickerSettings.ModuleName))
_settingsUtils = settingsUtils ?? throw new ArgumentNullException(nameof(settingsUtils));
if (_settingsUtils.SettingsExists(ColorPickerSettings.ModuleName))
{
_colorPickerSettings = SettingsUtils.GetSettings<ColorPickerSettings>(ColorPickerSettings.ModuleName);
_colorPickerSettings = _settingsUtils.GetSettings<ColorPickerSettings>(ColorPickerSettings.ModuleName);
}
else
{
_colorPickerSettings = new ColorPickerSettings();
}

if (SettingsUtils.SettingsExists())
if (_settingsUtils.SettingsExists())
{
var generalSettings = SettingsUtils.GetSettings<GeneralSettings>();
var generalSettings = _settingsUtils.GetSettings<GeneralSettings>();
_isEnabled = generalSettings.Enabled.ColorPicker;
}

Expand All @@ -51,7 +54,7 @@ public bool IsEnabled
OnPropertyChanged(nameof(IsEnabled));

// grab the latest version of settings
var generalSettings = SettingsUtils.GetSettings<GeneralSettings>();
var generalSettings = _settingsUtils.GetSettings<GeneralSettings>();
generalSettings.Enabled.ColorPicker = value;
OutGoingGeneralSettings outgoing = new OutGoingGeneralSettings(generalSettings);
SendConfigMSG(outgoing.ToString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@
using System.Drawing;
using System.Runtime.CompilerServices;
using Microsoft.PowerToys.Settings.UI.Lib.Helpers;
using Microsoft.PowerToys.Settings.UI.Lib.Utilities;
using Microsoft.PowerToys.Settings.UI.Lib.ViewModels.Commands;

namespace Microsoft.PowerToys.Settings.UI.Lib.ViewModels
{
public class FancyZonesViewModel : Observable
{
private readonly ISettingsUtils _settingsUtils;

private const string ModuleName = "FancyZones";

public ButtonClickCommand LaunchEditorEventHandler { get; set; }
Expand All @@ -22,18 +25,19 @@ public class FancyZonesViewModel : Observable

private string settingsConfigFileFolder = string.Empty;

public FancyZonesViewModel(Func<string, int> ipcMSGCallBackFunc, string configFileSubfolder = "")
public FancyZonesViewModel(ISettingsUtils settingsUtils, Func<string, int> ipcMSGCallBackFunc, string configFileSubfolder = "")
{
settingsConfigFileFolder = configFileSubfolder;
_settingsUtils = settingsUtils ?? throw new ArgumentNullException(nameof(settingsUtils));

try
{
Settings = SettingsUtils.GetSettings<FancyZonesSettings>(GetSettingsSubPath());
Settings = _settingsUtils.GetSettings<FancyZonesSettings>(GetSettingsSubPath());
}
catch
{
Settings = new FancyZonesSettings();
SettingsUtils.SaveSettings(Settings.ToJsonString(), GetSettingsSubPath());
_settingsUtils.SaveSettings(Settings.ToJsonString(), GetSettingsSubPath());
}

LaunchEditorEventHandler = new ButtonClickCommand(LaunchEditor);
Expand Down Expand Up @@ -71,12 +75,12 @@ public FancyZonesViewModel(Func<string, int> ipcMSGCallBackFunc, string configFi
GeneralSettings generalSettings;
try
{
generalSettings = SettingsUtils.GetSettings<GeneralSettings>(string.Empty);
generalSettings = _settingsUtils.GetSettings<GeneralSettings>(string.Empty);
}
catch
{
generalSettings = new GeneralSettings();
SettingsUtils.SaveSettings(generalSettings.ToJsonString(), string.Empty);
_settingsUtils.SaveSettings(generalSettings.ToJsonString(), string.Empty);
}

_isEnabled = generalSettings.Enabled.FancyZones;
Expand Down Expand Up @@ -117,7 +121,7 @@ public bool IsEnabled
if (value != _isEnabled)
{
_isEnabled = value;
GeneralSettings generalSettings = SettingsUtils.GetSettings<GeneralSettings>(string.Empty);
GeneralSettings generalSettings = _settingsUtils.GetSettings<GeneralSettings>(string.Empty);
generalSettings.Enabled.FancyZones = value;
OutGoingGeneralSettings snd = new OutGoingGeneralSettings(generalSettings);

Expand Down

0 comments on commit 0f6428e

Please sign in to comment.