Skip to content

Commit c4c52e2

Browse files
ibetitsmikeibetitsmike
committed
PR review fixes
1 parent ced517e commit c4c52e2

File tree

6 files changed

+74
-74
lines changed

6 files changed

+74
-74
lines changed

‎App/App.xaml.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Diagnostics;
43
using System.IO;
54
using System.Threading;
65
using System.Threading.Tasks;

‎App/Models/SettingsGroup.cs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
using System;
2+
3+
namespace Coder.Desktop.App.Models;
4+
5+
6+
public interface ISettings<T> : ICloneable<T>
7+
{
8+
/// <summary>
9+
/// FileName where the settings are stored.
10+
/// </summary>
11+
static abstract string SettingsFileName { get; }
12+
13+
/// <summary>
14+
/// Gets the version of the settings schema.
15+
/// </summary>
16+
int Version { get; }
17+
}
18+
19+
public interface ICloneable<T>
20+
{
21+
/// <summary>
22+
/// Creates a deep copy of the settings object.
23+
/// </summary>
24+
/// <returns>A new instance of the settings object with the same values.</returns>
25+
T Clone();
26+
}
27+
28+
/// <summary>
29+
/// CoderConnect settings class that holds the settings for the CoderConnect feature.
30+
/// </summary>
31+
public class CoderConnectSettings : ISettings<CoderConnectSettings>
32+
{
33+
public static string SettingsFileName { get; } = "coder-connect-settings.json";
34+
public int Version { get; set; }
35+
public bool ConnectOnLaunch { get; set; }
36+
37+
/// <summary>
38+
/// CoderConnect current settings version. Increment this when the settings schema changes.
39+
/// In future iterations we will be able to handle migrations when the user has
40+
/// an older version.
41+
/// </summary>
42+
private const int VERSION = 1;
43+
44+
public CoderConnectSettings()
45+
{
46+
Version = VERSION;
47+
48+
ConnectOnLaunch = false;
49+
}
50+
51+
public CoderConnectSettings(int? version, bool connectOnLaunch)
52+
{
53+
Version = version ?? VERSION;
54+
55+
ConnectOnLaunch = connectOnLaunch;
56+
}
57+
58+
public CoderConnectSettings Clone()
59+
{
60+
return new CoderConnectSettings(Version, ConnectOnLaunch);
61+
}
62+
}

‎App/Services/SettingsManager.cs

Lines changed: 7 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,16 @@
1-
using Google.Protobuf.WellKnownTypes;
2-
using Serilog;
31
using System;
4-
using System.Collections.Generic;
52
using System.IO;
63
using System.Text.Json;
7-
using System.Text.Json.Serialization;
84
using System.Threading;
95
using System.Threading.Tasks;
10-
using System.Xml.Linq;
6+
using Coder.Desktop.App.Models;
117

128
namespace Coder.Desktop.App.Services;
139

1410
/// <summary>
1511
/// Settings contract exposing properties for app settings.
1612
/// </summary>
17-
public interface ISettingsManager<T> where T : ISettings, new()
13+
public interface ISettingsManager<T> where T : ISettings<T>, new()
1814
{
1915
/// <summary>
2016
/// Reads the settings from the file system.
@@ -23,26 +19,25 @@ namespace Coder.Desktop.App.Services;
2319
/// </summary>
2420
/// <param name="ct"></param>
2521
/// <returns></returns>
26-
public Task<T> Read(CancellationToken ct = default);
22+
Task<T> Read(CancellationToken ct = default);
2723
/// <summary>
2824
/// Writes the settings to the file system.
2925
/// </summary>
3026
/// <param name="settings">Object containing the settings.</param>
3127
/// <param name="ct"></param>
3228
/// <returns></returns>
33-
public Task Write(T settings, CancellationToken ct = default);
29+
Task Write(T settings, CancellationToken ct = default);
3430
}
3531

3632
/// <summary>
3733
/// Implemention of <see cref="ISettingsManager"/> that persists settings to a JSON file
3834
/// located in the user's local application data folder.
3935
/// </summary>
40-
public sealed class SettingsManager<T> : ISettingsManager<T> where T : ISettings, new()
36+
public sealed class SettingsManager<T> : ISettingsManager<T> where T : ISettings<T>, new()
4137
{
4238
private readonly string _settingsFilePath;
4339
private readonly string _appName = "CoderDesktop";
4440
private string _fileName;
45-
private readonly object _lock = new();
4641

4742
private T? _cachedSettings;
4843

@@ -79,7 +74,7 @@ public async Task<T> Read(CancellationToken ct = default)
7974
if (_cachedSettings is not null)
8075
{
8176
// return cached settings if available
82-
return (T)_cachedSettings.Clone();
77+
return _cachedSettings.Clone();
8378
}
8479

8580
// try to get the lock with short timeout
@@ -98,7 +93,7 @@ public async Task<T> Read(CancellationToken ct = default)
9893
// deserialize; fall back to default(T) if empty or malformed
9994
var result = JsonSerializer.Deserialize<T>(json)!;
10095
_cachedSettings = result;
101-
return result;
96+
return _cachedSettings.Clone(); // return a fresh instance of the settings
10297
}
10398
catch (OperationCanceledException)
10499
{
@@ -148,57 +143,3 @@ await File.WriteAllTextAsync(_settingsFilePath, json, ct)
148143
}
149144
}
150145
}
151-
152-
public interface ISettings
153-
{
154-
/// <summary>
155-
/// FileName where the settings are stored.
156-
/// </summary>
157-
static abstract string SettingsFileName { get; }
158-
159-
/// <summary>
160-
/// Gets the version of the settings schema.
161-
/// </summary>
162-
int Version { get; }
163-
164-
ISettings Clone();
165-
}
166-
167-
/// <summary>
168-
/// CoderConnect settings class that holds the settings for the CoderConnect feature.
169-
/// </summary>
170-
public class CoderConnectSettings : ISettings
171-
{
172-
public static string SettingsFileName { get; } = "coder-connect-settings.json";
173-
public int Version { get; set; }
174-
public bool ConnectOnLaunch { get; set; }
175-
176-
/// <summary>
177-
/// CoderConnect current settings version. Increment this when the settings schema changes.
178-
/// In future iterations we will be able to handle migrations when the user has
179-
/// an older version.
180-
/// </summary>
181-
private const int VERSION = 1;
182-
183-
public CoderConnectSettings()
184-
{
185-
Version = VERSION;
186-
ConnectOnLaunch = false;
187-
}
188-
189-
public CoderConnectSettings(int? version, bool connectOnLogin)
190-
{
191-
Version = version ?? VERSION;
192-
ConnectOnLaunch = connectOnLogin;
193-
}
194-
195-
ISettings ISettings.Clone()
196-
{
197-
return Clone();
198-
}
199-
200-
public CoderConnectSettings Clone()
201-
{
202-
return new CoderConnectSettings(Version, ConnectOnLaunch);
203-
}
204-
}

‎App/ViewModels/SettingsViewModel.cs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1+
using Coder.Desktop.App.Models;
12
using Coder.Desktop.App.Services;
23
using CommunityToolkit.Mvvm.ComponentModel;
34
using Microsoft.Extensions.Logging;
4-
using Microsoft.UI.Dispatching;
5-
using Microsoft.UI.Xaml;
65
using System;
76

87
namespace Coder.Desktop.App.ViewModels;
@@ -29,9 +28,7 @@ public SettingsViewModel(ILogger<SettingsViewModel> logger, ISettingsManager<Cod
2928
_connectSettingsManager = settingsManager;
3029
_startupManager = startupManager;
3130
_logger = logger;
32-
// Application settings are loaded on application startup,
33-
// so we expect the settings to be available immediately.
34-
var settingsCache = settingsManager.Read();
31+
_connectSettings = settingsManager.Read().GetAwaiter().GetResult();
3532
StartOnLogin = startupManager.IsEnabled();
3633
ConnectOnLaunch = _connectSettings.ConnectOnLaunch;
3734

‎App/Views/Pages/TrayWindowLoginRequiredPage.xaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
</HyperlinkButton>
3737

3838
<HyperlinkButton
39-
Command="{x:Bind ViewModel.ExitCommand, Mode=OneWay}"
39+
Command="{x:Bind ViewModel.ExitCommand}"
4040
Margin="-12,-8,-12,-5"
4141
HorizontalAlignment="Stretch"
4242
HorizontalContentAlignment="Left">

‎Tests.App/Services/SettingsManagerTest.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using Coder.Desktop.App.Models;
12
using Coder.Desktop.App.Services;
23

34
namespace Coder.Desktop.Tests.App.Services;
@@ -24,7 +25,7 @@ public void TearDown()
2425
[Test]
2526
public void Save_Persists()
2627
{
27-
bool expected = true;
28+
var expected = true;
2829
var settings = new CoderConnectSettings
2930
{
3031
Version = 1,

0 commit comments

Comments
 (0)