Skip to content

Commit

Permalink
Drop immutability which doesn't add any value
Browse files Browse the repository at this point in the history 
Config writing is such a rare occurrence (compared with reading) that the additional complexity of making the config/document immutable doesn't bring much value. Quite the contrary, it's a source of hard to detect bugs.

The pattern of returning the same object for chaining is preserved (so the API is backs-compat), and the mutability is compatible with how ServiceCollection works, for example. The chaining is just for convenience.
  • Loading branch information
@kzu
kzu committed Jul 7, 2024
1 parent 24bd971 commit 5bf47f7
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 79 deletions.
5 changes: 1 addition & 4 deletions docs/api/index.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,9 @@ configuration level to use for persisting the value, by passing a `ConfigLevel`:
//[vs "alias"]
// comexp = run|community|exp
config = config.AddString("vs", "alias", "comexp", "run|community|exp", ConfigLevel.Global);
config.AddString("vs", "alias", "comexp", "run|community|exp", ConfigLevel.Global);
```

> IMPORTANT: the Config API is immutable, so if you make changes, you should update your reference
> to the newly updated Config, otherwise, subsequent changes would override prior ones.
You can explore the entire API in the [docs site](https://dotnetconfig.org/api/).

### Microsoft.Extensions.Configuration
Expand Down
5 changes: 1 addition & 4 deletions readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,12 +324,9 @@ configuration level to use for persisting the value, by passing a `ConfigLevel`:
//[vs "alias"]
// comexp = run|community|exp
config = config.AddString("vs", "alias", "comexp", "run|community|exp", ConfigLevel.Global);
config.AddString("vs", "alias", "comexp", "run|community|exp", ConfigLevel.Global);
```

> IMPORTANT: the Config API is immutable, so if you make changes, you should update your reference
> to the newly updated Config, otherwise, subsequent changes would override prior ones.
You can explore the entire API in the [docs site](https://dotnetconfig.org/api/).

### Microsoft.Extensions.Configuration
Expand Down
15 changes: 15 additions & 0 deletions src/Config.Tests/ConfigTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,21 @@ public void when_setting_variable_then_uses_tab()
Assert.Equal('\t', line[0]);
}

[Fact]
public void when_setting_muliplevariables_then_can_reuse_instance()
{
var file = Path.GetTempFileName();
var config = Config.Build(file);

config.SetString("section", "subsection", "foo", "bar");
config.SetString("section", "subsection", "bar", "baz");

var saved = Config.Build(file);

Assert.Equal("bar", saved.GetString("section", "subsection", "foo"));
Assert.Equal("baz", saved.GetString("section", "subsection", "bar"));
}

[Fact]
public void when_setting_global_variable_then_writes_global_file()
{
Expand Down
6 changes: 3 additions & 3 deletions src/Config/Config.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ public static Config Build(ConfigLevel store) =>
/// </summary>
public ConfigLevel? Level =>
this is AggregateConfig ? null :
FilePath == GlobalLocation ? (ConfigLevel?)ConfigLevel.Global :
FilePath == SystemLocation ? (ConfigLevel?)ConfigLevel.System :
FilePath.EndsWith(UserExtension) ? (ConfigLevel?)ConfigLevel.Local : null;
FilePath == GlobalLocation ? ConfigLevel.Global :
FilePath == SystemLocation ? ConfigLevel.System :
FilePath.EndsWith(UserExtension) ? ConfigLevel.Local : null;

/// <summary>
/// Gets the section and optional subsection from the configuration.
Expand Down
4 changes: 2 additions & 2 deletions src/Config/Config.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ Usage:
var config = Config.Build();
var value = config.GetString("section", "subsection", "variable");

// Setting values, Config is immutable, so chain calls and update var
config = config
// Setting values
config
.SetString("section", "subsection", "variable", value)
.SetBoolean("section", "subsection", "enabled", true);
</Description>
Expand Down
28 changes: 19 additions & 9 deletions src/Config/ConfigDocument.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ record ConfigDocument : IEnumerable<ConfigEntry>

public ConfigLevel? Level { get; }

internal ImmutableList<Line> Lines { get; init; } = ImmutableList<Line>.Empty;
internal ImmutableList<Line> Lines { get; private set; } = ImmutableList<Line>.Empty;

public ConfigDocument Save()
{
Expand Down Expand Up @@ -99,7 +99,8 @@ public ConfigDocument Add(string section, string? subsection, string name, strin
lines = lines.Insert(index, Line.CreateVariable(filePath, index, sectionLine.Section, sectionLine.Subsection, name, value));
}

return this with { Lines = lines };
Lines = lines;
return this;
}

public ConfigDocument Set(string section, string? subsection, string name, string? value = null, ValueMatcher? valueMatcher = null)
Expand All @@ -119,7 +120,8 @@ public ConfigDocument Set(string section, string? subsection, string name, strin

if (variable != null)
{
return this with { Lines = Lines.Replace(variable, variable.WithValue(value)) };
Lines = Lines.Replace(variable, variable.WithValue(value));
return this;
}
else
{
Expand All @@ -143,7 +145,9 @@ public ConfigDocument Unset(string section, string? subsection, string name)
var variable = variables.FirstOrDefault();
if (variable != null)
{
return (this with { Lines = Lines.Remove(variable) }).CleanupSection(section, subsection);
Lines = Lines.Remove(variable);
CleanupSection(section, subsection);
return this;
}

return this;
Expand All @@ -164,7 +168,8 @@ public ConfigDocument SetAll(string section, string? subsection, string name, st
lines = lines.Replace(variable, variable.WithValue(value));
}

return this with { Lines = lines };
Lines = lines;
return this;
}

public ConfigDocument UnsetAll(string section, string? subsection, string name, ValueMatcher? valueMatcher = null)
Expand All @@ -185,7 +190,9 @@ public ConfigDocument UnsetAll(string section, string? subsection, string name,
lines = lines.Remove(variable);
}

return (this with { Lines = lines }).CleanupSection(section, subsection);
Lines = lines;
CleanupSection(section, subsection);
return this;
}

public ConfigDocument RemoveSection(string section, string? subsection = null)
Expand Down Expand Up @@ -216,7 +223,8 @@ public ConfigDocument RemoveSection(string section, string? subsection = null)
while (lines.Count > 0 && lines[^1].Kind == LineKind.None)
lines = lines.RemoveAt(lines.Count - 1);

return this with { Lines = lines };
Lines = lines;
return this;
}

public ConfigDocument RenameSection(string oldSection, string? oldSubsection, string newSection, string? newSubsection)
Expand Down Expand Up @@ -246,7 +254,8 @@ public ConfigDocument RenameSection(string oldSection, string? oldSubsection, st
}
}

return this with { Lines = lines };
Lines = lines;
return this;
}

/// <summary>
Expand All @@ -266,7 +275,8 @@ ConfigDocument CleanupSection(string section, string? subsection)
while (index < lines.Count && lines[index].Kind == LineKind.None)
lines = lines.RemoveAt(index);

return this with { Lines = lines };
Lines = lines;
return this;
}
}

Expand Down
121 changes: 64 additions & 57 deletions src/Config/FileConfig.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,32 +20,33 @@ public override Config AddBoolean(string section, string? subsection, string var
if (value)
{
// Shortcut notation.
return new FileConfig(FilePath,
document.Add(section, subsection, variable, null)
.Save());
document.Add(section, subsection, variable, null).Save();
}
else
{
return new FileConfig(FilePath,
document.Add(section, subsection, variable, "false")
.Save());
document.Add(section, subsection, variable, "false").Save();
}

return this;
}

public override Config AddDateTime(string section, string? subsection, string variable, DateTime value)
=> new FileConfig(FilePath, document
.Add(section, subsection, variable, value.ToString("O"))
.Save());
{
document.Add(section, subsection, variable, value.ToString("O")).Save();
return this;
}

public override Config AddNumber(string section, string? subsection, string variable, long value)
=> new FileConfig(FilePath, document
.Add(section, subsection, variable, value.ToString())
.Save());
{
document.Add(section, subsection, variable, value.ToString()).Save();
return this;
}

public override Config AddString(string section, string? subsection, string variable, string value)
=> new FileConfig(FilePath, document
.Add(section, subsection, variable, value)
.Save());
{
document.Add(section, subsection, variable, value).Save();
return this;
}

public override IEnumerable<ConfigEntry> GetAll(string section, string? subsection, string variable, string? valueRegex)
=> document.GetAll(section, subsection, variable, valueRegex);
Expand All @@ -69,78 +70,82 @@ public override IEnumerable<ConfigEntry> GetRegex(string nameRegex, string? valu
}

public override Config RemoveSection(string section, string? subsection)
=> new FileConfig(FilePath, document
.RemoveSection(section, subsection)
.Save());
{
document.RemoveSection(section, subsection).Save();
return this;
}

public override Config RenameSection(string oldSection, string? oldSubsection, string newSection, string? newSubsection)
=> new FileConfig(FilePath, document
.RenameSection(oldSection, oldSubsection, newSection, newSubsection)
.Save());
{
document.RenameSection(oldSection, oldSubsection, newSection, newSubsection).Save();
return this;
}

public override Config SetAllBoolean(string section, string? subsection, string variable, bool value, string? valueRegex)
{
if (value)
{
// Shortcut notation.
return new FileConfig(FilePath, document
.SetAll(section, subsection, variable, null, valueRegex)
.Save());
document.SetAll(section, subsection, variable, null, valueRegex).Save();
}
else
{
return new FileConfig(FilePath, document
.SetAll(section, subsection, variable, "false", valueRegex)
.Save());
document.SetAll(section, subsection, variable, "false", valueRegex).Save();
}

return this;
}

public override Config SetAllDateTime(string section, string? subsection, string variable, DateTime value, string? valueRegex)
=> new FileConfig(FilePath, document
.SetAll(section, subsection, variable, value.ToString("O"), valueRegex)
.Save());
{
document.SetAll(section, subsection, variable, value.ToString("O"), valueRegex).Save();
return this;
}

public override Config SetAllNumber(string section, string? subsection, string variable, long value, string? valueRegex)
=> new FileConfig(FilePath, document
.SetAll(section, subsection, variable, value.ToString(), valueRegex)
.Save());
{
document.SetAll(section, subsection, variable, value.ToString(), valueRegex).Save();
return this;
}

public override Config SetAllString(string section, string? subsection, string variable, string value, string? valueRegex)
=> new FileConfig(FilePath, document
.SetAll(section, subsection, variable, value, valueRegex)
.Save());
{
document.SetAll(section, subsection, variable, value, valueRegex).Save();
return this;
}

public override Config SetBoolean(string section, string? subsection, string variable, bool value, string? valueRegex)
{
if (value)
{
// Shortcut notation.
return new FileConfig(FilePath, document
.Set(section, subsection, variable, null, valueRegex)
.Save());
document.Set(section, subsection, variable, null, valueRegex).Save();
}
else
{
return new FileConfig(FilePath, document
.Set(section, subsection, variable, "false", valueRegex)
.Save());
document.Set(section, subsection, variable, "false", valueRegex).Save();
}

return this;
}

public override Config SetDateTime(string section, string? subsection, string variable, DateTime value, string? valueRegex)
=> new FileConfig(FilePath, document
.Set(section, subsection, variable, value.ToString("O"), valueRegex)
.Save());
{
document.Set(section, subsection, variable, value.ToString("O"), valueRegex).Save();
return this;
}

public override Config SetNumber(string section, string? subsection, string variable, long value, string? valueRegex)
=> new FileConfig(FilePath, document
.Set(section, subsection, variable, value.ToString(), valueRegex)
.Save());
{
document.Set(section, subsection, variable, value.ToString(), valueRegex).Save();
return this;
}

public override Config SetString(string section, string? subsection, string variable, string value, string? valueRegex)
=> new FileConfig(FilePath, document
.Set(section, subsection, variable, value, valueRegex)
.Save());
{
document.Set(section, subsection, variable, value, valueRegex).Save();
return this;
}

public override bool TryGetBoolean(string section, string? subsection, string variable, out bool value)
{
Expand Down Expand Up @@ -211,14 +216,16 @@ public override bool TryGetString(string section, string? subsection, string var
}

public override Config Unset(string section, string? subsection, string variable)
=> new FileConfig(FilePath, document
.Unset(section, subsection, variable)
.Save());
{
document.Unset(section, subsection, variable).Save();
return this;
}

public override Config UnsetAll(string section, string? subsection, string variable, string? valueMatcher)
=> new FileConfig(FilePath, document
.UnsetAll(section, subsection, variable, valueMatcher)
.Save());
{
document.UnsetAll(section, subsection, variable, valueMatcher).Save();
return this;
}

protected override IEnumerable<ConfigEntry> GetEntries() => document;

Expand Down

0 comments on commit 5bf47f7

Please sign in to comment.