Skip to content

Commit 2ec2978

Browse files
authored
Don't write default config file during init (#720)
* Don't write default config file during init Per #719, this can cause issues with multiple concurrent processes executing (for example, parallel MSBuild nodes in a solution). Retrospectively, it doesn't provide anything useful either. It's faster to just create the defaults in memory than to get file IO involved.
1 parent 4030c23 commit 2ec2978

File tree

2 files changed

+17
-48
lines changed

2 files changed

+17
-48
lines changed

src/LibraryManager/Configuration/Settings.cs

+4-10
Original file line numberDiff line numberDiff line change
@@ -64,24 +64,18 @@ protected Settings()
6464
}
6565
catch (JsonReaderException)
6666
{
67-
// If there were any errors, we'll reset the file
67+
// If there were any errors, we'll take the defaults
68+
// TODO: log a warning
6869
}
6970
}
7071

7172
if (_rootObject is null)
7273
{
73-
InitSettingsFile(ConfigFilePath);
74+
_rootObject = new JObject(new JProperty("config", new JObject()));
75+
_configObject = _rootObject["config"] as JObject;
7476
}
7577
}
7678

77-
private void InitSettingsFile(string configFilePath)
78-
{
79-
_rootObject = new JObject(new JProperty("config", new JObject()));
80-
_configObject = _rootObject["config"] as JObject;
81-
82-
SaveSettingsFile(configFilePath, _rootObject);
83-
}
84-
8579
/// <inheritdoc />
8680
public string UserDataRoot
8781
{

test/LibraryManager.Test/Configuration/SettingsTest.cs

+13-38
Original file line numberDiff line numberDiff line change
@@ -47,37 +47,28 @@ public static void Cleanup()
4747
}
4848

4949
[TestMethod]
50-
public void Constructor_FileDoesNotExist_WhenInitialized_CreateNewFile()
50+
public void Constructor_FileDoesNotExist()
5151
{
5252
if (File.Exists(TestFilePath))
5353
{
5454
File.Delete(TestFilePath);
5555
}
5656

57-
_ = new TestSettings(TestFilePath);
57+
var settings = new TestSettings(TestFilePath);
5858

59-
Assert.IsTrue(File.Exists(TestFilePath));
59+
// verify settings does not throw, does not have a well-known value
60+
Assert.IsFalse(settings.TryGetValue(Constants.HttpsProxy, out _));
6061
}
6162

6263
[TestMethod]
63-
public void Constructor_FileExistsButEmpty_WhenInitialized_PopulateFile()
64+
public void Constructor_FileExistsButEmpty()
6465
{
6566
File.WriteAllText(TestFilePath, "");
6667

67-
_ = new TestSettings(TestFilePath);
68+
var settings = new TestSettings(TestFilePath);
6869

69-
Assert.IsTrue(new FileInfo(TestFilePath).Length > 0);
70-
}
71-
72-
[TestMethod]
73-
public void Constructor_FileExistsAndIsValid_WhenInitialized_DoNotModify()
74-
{
75-
string fileText = @"{ ""foo"": {} }";
76-
File.WriteAllText(TestFilePath, fileText);
77-
78-
_ = new TestSettings(TestFilePath);
79-
80-
Assert.AreEqual(fileText, File.ReadAllText(TestFilePath));
70+
// verify settings does not throw, does not have a well-known value
71+
Assert.IsFalse(settings.TryGetValue(Constants.HttpsProxy, out _));
8172
}
8273

8374
[TestMethod]
@@ -86,14 +77,10 @@ public void Constructor_FileExistsButNotJsonObject_Initialize()
8677
string fileText = @"[]"; // valid JSON, but not an object
8778
File.WriteAllText(TestFilePath, fileText);
8879

89-
_ = new TestSettings(TestFilePath);
90-
91-
string actual = File.ReadAllText(TestFilePath);
92-
string expected = @"{
93-
""config"": {}
94-
}";
80+
var settings = new TestSettings(TestFilePath);
9581

96-
Assert.AreEqual(StringUtility.NormalizeNewLines(expected), StringUtility.NormalizeNewLines(actual));
82+
// verify settings does not throw, does not have a well-known value
83+
Assert.IsFalse(settings.TryGetValue(Constants.HttpsProxy, out _));
9784
}
9885

9986
[TestMethod]
@@ -121,19 +108,7 @@ public void TryGetValue_ValueDoesNotExist_ReturnFalseWithEmptyString()
121108
}
122109

123110
[TestMethod]
124-
public void TryGetValue_ValueExists_ReturnTrueWithValue()
125-
{
126-
string fileText = @"{ ""config"": { ""testKey"": ""testValue"" } }";
127-
File.WriteAllText(TestFilePath, fileText);
128-
129-
bool success = new TestSettings(TestFilePath).TryGetValue("testKey", out string value);
130-
131-
Assert.IsTrue(success);
132-
Assert.AreEqual("testValue", value);
133-
}
134-
135-
[TestMethod]
136-
public void TryGetValue_ValueExistsInDifferentCase_ReturnFalse()
111+
public void TryGetValue_SettingsNameIsCaseSensitive()
137112
{
138113
string fileText = @"{ ""config"": { ""testKey"": ""testValue"" } }";
139114
File.WriteAllText(TestFilePath, fileText);
@@ -212,7 +187,7 @@ public void SetValue_ValueExists_UpdateValue()
212187
}
213188

214189
[TestMethod]
215-
public void SetEncyptedValue_ValueIsDifferentFromOriginal()
190+
public void SetEncryptedValue_ValueIsDifferentFromOriginal()
216191
{
217192
var ut = new TestSettings(TestFilePath);
218193

0 commit comments

Comments
 (0)