Introduce ConfigManager 2/n
#11863
Introduce ConfigManager 2/n
#11863
Conversation
kiminuo
force-pushed
the
feature/2023-11-03-Faster-ConfigManager
branch
from
f28c65d
to
c97f6ec
Compare
…Faster-ConfigManager # Conflicts: # WalletWasabi.Daemon/WasabiAppBuilder.cs # WalletWasabi.Fluent/Models/UI/ApplicationSettings.cs # WalletWasabi/Services/ConfigWatcher.cs
WalletWasabi/Interfaces/IConfig.cs
Outdated
| bool AreDeepEqual(object otherConfig); | ||
|
|
||
| /// <summary>Check if the config file differs from the config if the file path of the config file is set, otherwise throw exception.</summary> | ||
| bool CheckFileChange(); |
There was a problem hiding this comment.
Moved to ConfigManager
| @@ -0,0 +1,5 @@ | |||
| namespace WalletWasabi.Interfaces; | |||
|
|
|||
| public interface IConfigNg | |||
There was a problem hiding this comment.
Less demanding IConfig. It's notable that it does not contain FilePath and other stuff.
| public Action ExecuteWhenChanged { get; } | ||
|
|
||
| protected override Task ActionAsync(CancellationToken cancel) | ||
| { | ||
| if (Config.CheckFileChange()) | ||
| if (ConfigManager.CheckFileChange(Config.FilePath, Config)) |
There was a problem hiding this comment.
Use the new ConfigManager.
WalletWasabi/Bases/ConfigBase.cs
Outdated
| @@ -38,26 +38,6 @@ public void AssertFilePathSet() | |||
| } | |||
| } | |||
|
|
|||
| /// <inheritdoc/> | |||
| public bool CheckFileChange() | |||
There was a problem hiding this comment.
Code from this file moved to ConfigManager.
| using WalletWasabi.JsonConverters; | ||
| using WalletWasabi.JsonConverters.Bitcoin; | ||
|
|
||
| namespace WalletWasabi.Daemon; | ||
|
|
||
| [JsonObject(MemberSerialization.OptIn)] | ||
| public class PersistentConfig : ConfigBase | ||
| public record PersistentConfig : IConfigNg |
There was a problem hiding this comment.
No longer derive from ConfigBase to avoid having FilePath and other stuff not needed and make PersistentConfig clean "data (C#) type"
| public PersistentConfig(string filePath) : base(filePath) | ||
| { | ||
| } |
| [JsonProperty(PropertyName = "Network")] | ||
| [JsonConverter(typeof(NetworkJsonConverter))] | ||
| public Network Network { get; set; } = Network.Main; | ||
|
|
||
| [DefaultValue(Constants.BackendUri)] | ||
| [JsonProperty(PropertyName = "MainNetBackendUri", DefaultValueHandling = DefaultValueHandling.Populate)] | ||
| public string MainNetBackendUri { get; private set; } = Constants.BackendUri; | ||
| public string MainNetBackendUri { get; init; } = Constants.BackendUri; |
There was a problem hiding this comment.
Having properites immutable is good because we don't want to change properties really. What happens (AFAIK) is that we want to store modified configs but we don't support changing config values during runtime.
|
|
||
| [DefaultValue("CoinJoinCoordinatorIdentifier")] | ||
| [JsonProperty(PropertyName = "CoordinatorIdentifier", DefaultValueHandling = DefaultValueHandling.Populate)] | ||
| public string CoordinatorIdentifier { get; set; } = "CoinJoinCoordinatorIdentifier"; | ||
|
|
||
| public void SetBitcoinP2pEndpoint(EndPoint endPoint) |
There was a problem hiding this comment.
…Faster-ConfigManager # Conflicts: # WalletWasabi/Bases/ConfigManager.cs
…2023-11-03-Faster-ConfigManager
| @@ -52,6 +52,10 @@ private void ValidateBitcoinP2PEndPoint(IValidationErrors errors) | |||
| { | |||
| errors.Add(ErrorSeverity.Error, "Invalid endpoint."); | |||
| } | |||
| else | |||
There was a problem hiding this comment.
Changes in this file are to fix #11868. It's not idiomatic Avalonia code. Suggestions are welcome. Can be a standalone PR.
| @@ -154,20 +131,19 @@ public EndPoint GetBitcoinP2pEndPoint() | |||
| throw new NotSupportedNetworkException(Network); | |||
| } | |||
|
|
|||
| public bool MigrateOldDefaultBackendUris() | |||
| public bool MigrateOldDefaultBackendUris([NotNullWhenAttribute(true)] out PersistentConfig? newConfig) | |||
There was a problem hiding this comment.
Now we return a new instance of PersistentConfig through the out parameter because PersistentConfig is immutable.
|
|
||
| if (!createIfMissing) | ||
| { | ||
| return LoadFile<TResponse>(filePath); |
There was a problem hiding this comment.
The same call is wrapped inside a try-catch block below. Shouldn't we do the same here?
There was a problem hiding this comment.
I'll address this in a follow-up PR with a test.
Follow-up to #11875
Fixes #11868
Precursor for #11764