Skip to content

Fixed appsettings.json cann't be copied successfully #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Potion837 merged 3 commits into from
Nov 13, 2024
Merged

Fixed appsettings.json cann't be copied successfully #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7cf7c0
Fixed appsettings.json cann't be copied successfully
Potion837 Nov 12, 2024
14f5b49
Added login by qrcode & password
Potion837 Nov 13, 2024
bcd2963
Added login by qrcode & password
Potion837 Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions Login/QrCodeLogin.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
using Lagrange.Core;
using Lagrange.Core.Common;
using Lagrange.Core.Common.Interface;
using Lagrange.Core.Common.Interface.Api;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Console = System.Console;
using JsonSerializer = System.Text.Json.JsonSerializer;

namespace YounBot.Login;

public static class QrCodeLogin
{
public static async Task Login(YounBotApp app, BotConfig config, BotDeviceInfo deviceInfo, BotKeystore keystore)
{
Comment on lines +14 to +15
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add parameter validation and XML documentation.

The method lacks input validation and documentation which could lead to runtime issues and maintenance difficulties.

+/// <summary>
+/// Attempts to log in to the bot service using password authentication, falling back to QR code if necessary.
+/// </summary>
+/// <param name="app">The YounBot application instance.</param>
+/// <param name="config">The bot configuration.</param>
+/// <param name="deviceInfo">Device information for the bot.</param>
+/// <param name="keystore">The bot keystore.</param>
+/// <returns>A task representing the asynchronous login operation.</returns>
+/// <exception cref="ArgumentNullException">Thrown when any parameter is null.</exception>
 public static async Task Login(YounBotApp app, BotConfig config, BotDeviceInfo deviceInfo, BotKeystore keystore)
 {
+    ArgumentNullException.ThrowIfNull(app);
+    ArgumentNullException.ThrowIfNull(config);
+    ArgumentNullException.ThrowIfNull(deviceInfo);
+    ArgumentNullException.ThrowIfNull(keystore);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static async Task Login(YounBotApp app, BotConfig config, BotDeviceInfo deviceInfo, BotKeystore keystore)
{
/// <summary>
/// Attempts to log in to the bot service using password authentication, falling back to QR code if necessary.
/// </summary>
/// <param name="app">The YounBot application instance.</param>
/// <param name="config">The bot configuration.</param>
/// <param name="deviceInfo">Device information for the bot.</param>
/// <param name="keystore">The bot keystore.</param>
/// <returns>A task representing the asynchronous login operation.</returns>
/// <exception cref="ArgumentNullException">Thrown when any parameter is null.</exception>
public static async Task Login(YounBotApp app, BotConfig config, BotDeviceInfo deviceInfo, BotKeystore keystore)
{
ArgumentNullException.ThrowIfNull(app);
ArgumentNullException.ThrowIfNull(config);
ArgumentNullException.ThrowIfNull(deviceInfo);
ArgumentNullException.ThrowIfNull(keystore);

app.Client = BotFactory.Create(config, deviceInfo, keystore);
var client = app.Client;
Comment on lines +16 to +17
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for client creation.

The client creation could fail, but there's no error handling in place.

-    app.Client = BotFactory.Create(config, deviceInfo, keystore);
-    var client = app.Client;
+    try 
+    {
+        app.Client = BotFactory.Create(config, deviceInfo, keystore);
+        var client = app.Client;
+    }
+    catch (Exception ex)
+    {
+        throw new InvalidOperationException("Failed to create bot client.", ex);
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
app.Client = BotFactory.Create(config, deviceInfo, keystore);
var client = app.Client;
try
{
app.Client = BotFactory.Create(config, deviceInfo, keystore);
var client = app.Client;
}
catch (Exception ex)
{
throw new InvalidOperationException("Failed to create bot client.", ex);
}


client.Invoker.OnBotLogEvent += (_, @event) => {
Console.WriteLine(@event.ToString());
};

client.Invoker.OnBotOnlineEvent += (_, @event) =>
{
Console.WriteLine(@event.ToString());
client.UpdateKeystore();
File.WriteAllText(app.Configuration["ConfigPath:Keystore"] ?? "keystore.json", JsonSerializer.Serialize(keystore));
};
Comment on lines +19 to +28
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Multiple improvements needed for event handlers.

  1. Event handlers should be unsubscribable
  2. File operations need error handling
  3. Hardcoded fallback path should be configurable
+    private static readonly string DefaultKeystorePath = "keystore.json";
+    private static void OnBotLog(object? sender, BotLogEvent @event)
+    {
+        Console.WriteLine(@event.ToString());
+    }
+
+    private static void OnBotOnline(object? sender, BotOnlineEvent @event, IBotClient client, 
+        BotKeystore keystore, IConfiguration configuration)
+    {
+        Console.WriteLine(@event.ToString());
+        client.UpdateKeystore();
+        try
+        {
+            var keystorePath = configuration["ConfigPath:Keystore"] ?? DefaultKeystorePath;
+            File.WriteAllText(keystorePath, JsonSerializer.Serialize(keystore));
+        }
+        catch (Exception ex)
+        {
+            Console.WriteLine($"Failed to save keystore: {ex.Message}");
+        }
+    }
+
     // In Login method:
-    client.Invoker.OnBotLogEvent += (_, @event) => {
-        Console.WriteLine(@event.ToString());
-    };
+    client.Invoker.OnBotLogEvent += OnBotLog;
     
-    client.Invoker.OnBotOnlineEvent += (_, @event) =>
-    {
-        Console.WriteLine(@event.ToString());
-        client.UpdateKeystore();
-        File.WriteAllText(app.Configuration["ConfigPath:Keystore"] ?? "keystore.json", 
-            JsonSerializer.Serialize(keystore));
-    };
+    client.Invoker.OnBotOnlineEvent += (s, e) => 
+        OnBotOnline(s, e, client, keystore, app.Configuration);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
client.Invoker.OnBotLogEvent += (_, @event) => {
Console.WriteLine(@event.ToString());
};
client.Invoker.OnBotOnlineEvent += (_, @event) =>
{
Console.WriteLine(@event.ToString());
client.UpdateKeystore();
File.WriteAllText(app.Configuration["ConfigPath:Keystore"] ?? "keystore.json", JsonSerializer.Serialize(keystore));
};
private static readonly string DefaultKeystorePath = "keystore.json";
private static void OnBotLog(object? sender, BotLogEvent @event)
{
Console.WriteLine(@event.ToString());
}
private static void OnBotOnline(object? sender, BotOnlineEvent @event, IBotClient client,
BotKeystore keystore, IConfiguration configuration)
{
Console.WriteLine(@event.ToString());
client.UpdateKeystore();
try
{
var keystorePath = configuration["ConfigPath:Keystore"] ?? DefaultKeystorePath;
File.WriteAllText(keystorePath, JsonSerializer.Serialize(keystore));
}
catch (Exception ex)
{
Console.WriteLine($"Failed to save keystore: {ex.Message}");
}
}
client.Invoker.OnBotLogEvent += OnBotLog;
client.Invoker.OnBotOnlineEvent += (s, e) =>
OnBotOnline(s, e, client, keystore, app.Configuration);


if (!await app.Client.LoginByPassword())
{
var qrCode = await client.FetchQrCode();

if (qrCode != null)
{
await File.WriteAllBytesAsync("qrcode.png", qrCode.Value.QrCode);
await app.Client.LoginByQrCode();
}
}
Comment on lines +30 to +39
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve login flow with error handling, cleanup, and timeouts.

The login flow needs better error handling, QR code file cleanup, and timeout handling.

+    private static readonly string QrCodePath = "qrcode.png";
+    private static readonly TimeSpan LoginTimeout = TimeSpan.FromMinutes(2);
+
     // In Login method:
-    if (!await app.Client.LoginByPassword())
+    using var cts = new CancellationTokenSource(LoginTimeout);
+    try
     {
-        var qrCode = await client.FetchQrCode();
-
-        if (qrCode != null)
+        if (!await app.Client.LoginByPassword().WaitAsync(cts.Token))
         {
-            await File.WriteAllBytesAsync("qrcode.png", qrCode.Value.QrCode);
-            await app.Client.LoginByQrCode();
+            var qrCode = await client.FetchQrCode().WaitAsync(cts.Token);
+
+            if (qrCode != null)
+            {
+                try
+                {
+                    await File.WriteAllBytesAsync(QrCodePath, qrCode.Value.QrCode);
+                    await app.Client.LoginByQrCode().WaitAsync(cts.Token);
+                }
+                finally
+                {
+                    if (File.Exists(QrCodePath))
+                    {
+                        File.Delete(QrCodePath);
+                    }
+                }
+            }
+            else
+            {
+                throw new InvalidOperationException("Failed to fetch QR code.");
+            }
         }
     }
+    catch (OperationCanceledException)
+    {
+        throw new TimeoutException("Login operation timed out.");
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!await app.Client.LoginByPassword())
{
var qrCode = await client.FetchQrCode();
if (qrCode != null)
{
await File.WriteAllBytesAsync("qrcode.png", qrCode.Value.QrCode);
await app.Client.LoginByQrCode();
}
}
private static readonly string QrCodePath = "qrcode.png";
private static readonly TimeSpan LoginTimeout = TimeSpan.FromMinutes(2);
using var cts = new CancellationTokenSource(LoginTimeout);
try
{
if (!await app.Client.LoginByPassword().WaitAsync(cts.Token))
{
var qrCode = await client.FetchQrCode().WaitAsync(cts.Token);
if (qrCode != null)
{
try
{
await File.WriteAllBytesAsync(QrCodePath, qrCode.Value.QrCode);
await app.Client.LoginByQrCode().WaitAsync(cts.Token);
}
finally
{
if (File.Exists(QrCodePath))
{
File.Delete(QrCodePath);
}
}
}
else
{
throw new InvalidOperationException("Failed to fetch QR code.");
}
}
}
catch (OperationCanceledException)
{
throw new TimeoutException("Login operation timed out.");
}

}
}
27 changes: 11 additions & 16 deletions Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using YounBot.Login;

namespace YounBot;

class Program
{
static void Main(string[] args)
{
public static async Task Main() {
// thanks to Lagrange.OneBot
Console.OutputEncoding = Encoding.UTF8;
Console.InputEncoding = Encoding.UTF8;
Expand All @@ -21,10 +21,9 @@ static void Main(string[] args)

if (!File.Exists("appsettings.json"))
{
Console.WriteLine("No exist config file, create it now...");

var assm = Assembly.GetExecutingAssembly();
using var istr = assm.GetManifestResourceStream("Lagrange.OneBot.Resources.appsettings.json")!;
using var istr = assm.GetManifestResourceStream("YounBot.Resources.appsettings.json")!;
Copy link

@coderabbitai coderabbitai bot Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add proper error handling for resource stream access

The null-forgiving operator (!) assumes the embedded resource will always exist, which could lead to a NullReferenceException if the resource is missing. Consider adding proper error handling.

-            using var istr = assm.GetManifestResourceStream("YounBot.Resources.appsettings.json")!;
+            var resourceStream = assm.GetManifestResourceStream("YounBot.Resources.appsettings.json");
+            if (resourceStream == null)
+            {
+                throw new FileNotFoundException("Required embedded resource 'appsettings.json' was not found. Ensure it's properly embedded in the assembly.");
+            }
+            using var istr = resourceStream;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
using var istr = assm.GetManifestResourceStream("YounBot.Resources.appsettings.json")!;
var resourceStream = assm.GetManifestResourceStream("YounBot.Resources.appsettings.json");
if (resourceStream == null)
{
throw new FileNotFoundException("Required embedded resource 'appsettings.json' was not found. Ensure it's properly embedded in the assembly.");
}
using var istr = resourceStream;

using var temp = File.Create("appsettings.json");
istr.CopyTo(temp);

Expand All @@ -34,25 +33,21 @@ static void Main(string[] args)
Console.WriteLine("Please Edit the appsettings.json to set configs and press any key to continue");
Console.ReadLine();
}

// 创建IServiceCollection实例
var services = new ServiceCollection();


// 创建IConfiguration实例
var configurationBuilder = new ConfigurationBuilder()
.AddJsonFile("appsettings.json", optional: false, reloadOnChange: true)
.AddJsonFile("appsettings.json", false, true)
.AddEnvironmentVariables();
var configuration = configurationBuilder.Build();

// 创建YounBotAppBuilder实例
var appBuilder = new YounBotAppBuilder(services, configuration);

var appBuilder = new YounBotAppBuilder(configuration);
var app = appBuilder.Build();
// 配置应用程序
appBuilder
.ConfigureBots()
.ConfigureLogging(logging => logging.AddConsole());
appBuilder.ConfigureBots();

//登录
await QrCodeLogin.Login(app, appBuilder.GetConfig(), appBuilder.GetDeviceInfo(), appBuilder.GetKeystore());
Comment on lines +44 to +50
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for login process

The login process should include error handling to gracefully handle potential failures (network issues, invalid QR code, etc.).

        var appBuilder = new YounBotAppBuilder(configuration);
        var app = appBuilder.Build();
        appBuilder.ConfigureBots();

        //登录
-        await QrCodeLogin.Login(app, appBuilder.GetConfig(), appBuilder.GetDeviceInfo(), appBuilder.GetKeystore());
+        try 
+        {
+            await QrCodeLogin.Login(app, appBuilder.GetConfig(), appBuilder.GetDeviceInfo(), appBuilder.GetKeystore());
+        }
+        catch (Exception ex)
+        {
+            Console.WriteLine($"Login failed: {ex.Message}");
+            // Consider adding proper logging and graceful shutdown
+            Environment.Exit(1);
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var appBuilder = new YounBotAppBuilder(configuration);
var app = appBuilder.Build();
// 配置应用程序
appBuilder
.ConfigureBots()
.ConfigureLogging(logging => logging.AddConsole());
appBuilder.ConfigureBots();
//登录
await QrCodeLogin.Login(app, appBuilder.GetConfig(), appBuilder.GetDeviceInfo(), appBuilder.GetKeystore());
var appBuilder = new YounBotAppBuilder(configuration);
var app = appBuilder.Build();
// 配置应用程序
appBuilder.ConfigureBots();
//登录
try
{
await QrCodeLogin.Login(app, appBuilder.GetConfig(), appBuilder.GetDeviceInfo(), appBuilder.GetKeystore());
}
catch (Exception ex)
{
Console.WriteLine($"Login failed: {ex.Message}");
// Consider adding proper logging and graceful shutdown
Environment.Exit(1);
}


// 构建应用程序
var app = appBuilder.Build();
}
}
5 changes: 5 additions & 0 deletions YounBot.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,9 @@
<PackageReference Include="Microsoft.Extensions.Logging" Version="9.0.0-rc.2.24473.5" />
</ItemGroup>

<ItemGroup>
<None Remove="Resources\appsettings.json" />
<EmbeddedResource Include="Resources\appsettings.json" />
</ItemGroup>

</Project>
15 changes: 5 additions & 10 deletions YounBotApp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
using Microsoft.Extensions.Configuration;
using Lagrange.Core;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;

namespace YounBot;

public class YounBotApp
public class YounBotApp(YounBotAppBuilder appBuilder)
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add parameter validation and consider interface abstraction

  1. The appBuilder parameter should be validated to prevent null reference exceptions
  2. Consider extracting an interface for better testability
-public class YounBotApp(YounBotAppBuilder appBuilder)
+public class YounBotApp(YounBotAppBuilder appBuilder)
+{
+    public YounBotApp(YounBotAppBuilder appBuilder)
+    {
+        ArgumentNullException.ThrowIfNull(appBuilder);
+        Configuration = appBuilder.GetConfiguration();
+    }

Consider extracting an interface:

public interface IYounBotApp
{
    IConfiguration Configuration { get; }
    BotContext? Client { get; set; }
}

public class YounBotApp : IYounBotApp

{
public IServiceCollection Services;
public IConfiguration Configuration;

public YounBotApp(YounBotAppBuilder builder)
{
Services = builder.GetServices();
Configuration = builder.GetConfiguration();
}
public readonly IConfiguration Configuration = appBuilder.GetConfiguration();
public BotContext? Client;
Comment on lines +9 to +10
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Restrict mutability of the Client field to ensure thread safety

The Client field is being initialized via BotFactory.Create in Login/QrCodeLogin.cs. To maintain thread safety:

  • Make the Client property's setter internal to prevent external modification.
  • Ensure that BotFactory.Create handles thread-safe initialization.
🔗 Analysis chain

Address potential thread safety and initialization concerns with Client field

The Client field raises several concerns:

  1. It's publicly mutable without any thread safety mechanisms
  2. The initialization pattern is unclear
  3. There's no documentation about when it should be set or what null means

Consider this safer approach:

public readonly IConfiguration Configuration = appBuilder.GetConfiguration();
-public BotContext? Client;
+private BotContext? _client;
+public BotContext? Client 
+{
+    get => _client;
+    internal set
+    {
+        ArgumentNullException.ThrowIfNull(value);
+        Interlocked.Exchange(ref _client, value);
+    }
+}

Let's check how Client is used across the codebase:

Let's examine how the Client field is initialized in Login/QrCodeLogin.cs:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Client field usage to understand initialization patterns
rg -l "Client\s*[=\.]" --type csharp

Length of output: 59

Script:

#!/bin/bash
# Find all assignments to Client in Login/QrCodeLogin.cs
rg "^.*Client\s*=.*$" Login/QrCodeLogin.cs

Length of output: 114

}
68 changes: 28 additions & 40 deletions YounBotAppBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,46 +1,45 @@
using System.Text.Json.Serialization;
using Lagrange.Core;
using Lagrange.Core.Common;
using Lagrange.Core.Common.Interface;
using Lagrange.Core.Common.Interface.Api;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using JsonSerializer = System.Text.Json.JsonSerializer;

namespace YounBot;

public sealed class YounBotAppBuilder
public sealed class YounBotAppBuilder(IConfiguration configuration)
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Incorrect class declaration syntax

The class declaration public sealed class YounBotAppBuilder(IConfiguration configuration) uses constructor syntax directly in the class declaration, which is invalid for a regular class in C#. This syntax is typically used for records. To define a class with a constructor, you should separate the class declaration from the constructor.

Apply the following changes:

-public sealed class YounBotAppBuilder(IConfiguration configuration)
+public sealed class YounBotAppBuilder
+{
+    private readonly IConfiguration configuration;
+
+    public YounBotAppBuilder(IConfiguration configuration)
+    {
+        this.configuration = configuration;
+    }
+
+    // Rest of the class...
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public sealed class YounBotAppBuilder(IConfiguration configuration)
public sealed class YounBotAppBuilder
{
private readonly IConfiguration configuration;
public YounBotAppBuilder(IConfiguration configuration)
{
this.configuration = configuration;
}
// Rest of the class...

{
private IServiceCollection Services;

private IConfiguration Configuration;

public YounBotAppBuilder(IServiceCollection services, IConfiguration configuration)
{
Services = services;
Configuration = configuration;
}
private BotDeviceInfo _deviceInfo;
private BotKeystore _keystore;
private BotConfig _botConfig;

public IServiceCollection GetServices() => Services;
public IConfiguration GetConfiguration() => Configuration;

public YounBotAppBuilder ConfigureBots()
public IConfiguration GetConfiguration() => configuration;
public BotDeviceInfo GetDeviceInfo() => _deviceInfo;
public BotKeystore GetKeystore() => _keystore;
public BotConfig GetConfig() => _botConfig;

public void ConfigureBots()
{
string keystorePath = Configuration["ConfigPath:Keystore"] ?? "keystore.json";
string deviceInfoPath = Configuration["ConfigPath:DeviceInfo"] ?? "device.json";
string keystorePath = configuration["ConfigPath:Keystore"] ?? "keystore.json";
string deviceInfoPath = configuration["ConfigPath:DeviceInfo"] ?? "device.json";

bool isSuccess = Enum.TryParse<Protocols>(Configuration["Account:Protocol"], out var protocol);
var config = new BotConfig
bool isSuccess = Enum.TryParse<Protocols>(configuration["Account:Protocol"], out var protocol);

_botConfig = new BotConfig
{
Protocol = isSuccess ? protocol : Protocols.Linux,
AutoReconnect = bool.Parse(Configuration["Account:AutoReconnect"] ?? "true"),
UseIPv6Network = bool.Parse(Configuration["Account:UseIPv6Network"] ?? "false"),
GetOptimumServer = bool.Parse(Configuration["Account:GetOptimumServer"] ?? "true"),
AutoReLogin = bool.Parse(Configuration["Account:AutoReLogin"] ?? "true"),
AutoReconnect = bool.Parse(configuration["Account:AutoReconnect"] ?? "true"),
UseIPv6Network = bool.Parse(configuration["Account:UseIPv6Network"] ?? "false"),
GetOptimumServer = bool.Parse(configuration["Account:GetOptimumServer"] ?? "true"),
AutoReLogin = bool.Parse(configuration["Account:AutoReLogin"] ?? "true"),
Comment on lines +34 to +37
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use bool.TryParse to prevent exceptions

Using bool.Parse can throw an exception if the configuration value is not a valid boolean. To make the code more robust, use bool.TryParse and provide default values when parsing fails.

Apply this diff to safely parse boolean configuration values:

-AutoReconnect = bool.Parse(configuration["Account:AutoReconnect"] ?? "true"),
+if (!bool.TryParse(configuration["Account:AutoReconnect"], out var autoReconnect))
+{
+    autoReconnect = true; // Default value
+}
+AutoReconnect = autoReconnect;

-UseIPv6Network = bool.Parse(configuration["Account:UseIPv6Network"] ?? "false"),
+if (!bool.TryParse(configuration["Account:UseIPv6Network"], out var useIPv6Network))
+{
+    useIPv6Network = false; // Default value
+}
+UseIPv6Network = useIPv6Network;

-GetOptimumServer = bool.Parse(configuration["Account:GetOptimumServer"] ?? "true"),
+if (!bool.TryParse(configuration["Account:GetOptimumServer"], out var getOptimumServer))
+{
+    getOptimumServer = true; // Default value
+}
+GetOptimumServer = getOptimumServer;

-AutoReLogin = bool.Parse(configuration["Account:AutoReLogin"] ?? "true"),
+if (!bool.TryParse(configuration["Account:AutoReLogin"], out var autoReLogin))
+{
+    autoReLogin = true; // Default value
+}
+AutoReLogin = autoReLogin;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
AutoReconnect = bool.Parse(configuration["Account:AutoReconnect"] ?? "true"),
UseIPv6Network = bool.Parse(configuration["Account:UseIPv6Network"] ?? "false"),
GetOptimumServer = bool.Parse(configuration["Account:GetOptimumServer"] ?? "true"),
AutoReLogin = bool.Parse(configuration["Account:AutoReLogin"] ?? "true"),
if (!bool.TryParse(configuration["Account:AutoReconnect"], out var autoReconnect))
{
autoReconnect = true; // Default value
}
AutoReconnect = autoReconnect,
if (!bool.TryParse(configuration["Account:UseIPv6Network"], out var useIPv6Network))
{
useIPv6Network = false; // Default value
}
UseIPv6Network = useIPv6Network,
if (!bool.TryParse(configuration["Account:GetOptimumServer"], out var getOptimumServer))
{
getOptimumServer = true; // Default value
}
GetOptimumServer = getOptimumServer,
if (!bool.TryParse(configuration["Account:AutoReLogin"], out var autoReLogin))
{
autoReLogin = true; // Default value
}
AutoReLogin = autoReLogin,

};

BotKeystore keystore;

if (!File.Exists(keystorePath))
{
keystore = new BotKeystore();
_keystore = new BotKeystore();
string? directoryPath = Path.GetDirectoryName(keystorePath);
if (!string.IsNullOrEmpty(directoryPath))
{
Expand All @@ -49,14 +48,13 @@ public YounBotAppBuilder ConfigureBots()
}
else
{
keystore = JsonSerializer.Deserialize<BotKeystore>(File.ReadAllText(keystorePath)) ?? new BotKeystore();
_keystore = JsonSerializer.Deserialize<BotKeystore>(File.ReadAllText(keystorePath)) ?? new BotKeystore();
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add exception handling for JSON deserialization

Deserializing JSON files without exception handling can lead to application crashes if the files are corrupt or malformed. Wrap the deserialization calls in try-catch blocks to handle potential exceptions gracefully.

Apply this diff to handle exceptions during deserialization:

For _keystore deserialization:

- _keystore = JsonSerializer.Deserialize<BotKeystore>(File.ReadAllText(keystorePath)) ?? new BotKeystore();
+ try
+ {
+     _keystore = JsonSerializer.Deserialize<BotKeystore>(File.ReadAllText(keystorePath)) ?? new BotKeystore();
+ }
+ catch (Exception ex)
+ {
+     // Log the exception if necessary
+     // Initialize with a new instance to prevent null reference issues
+     _keystore = new BotKeystore();
+ }

For _deviceInfo deserialization:

- _deviceInfo = JsonSerializer.Deserialize<BotDeviceInfo>(File.ReadAllText(deviceInfoPath)) ?? BotDeviceInfo.GenerateInfo();
+ try
+ {
+     _deviceInfo = JsonSerializer.Deserialize<BotDeviceInfo>(File.ReadAllText(deviceInfoPath)) ?? BotDeviceInfo.GenerateInfo();
+ }
+ catch (Exception ex)
+ {
+     // Log the exception if necessary
+     // Generate new device info to prevent null reference issues
+     _deviceInfo = BotDeviceInfo.GenerateInfo();
+ }

Also applies to: 67-67

}

BotDeviceInfo deviceInfo;
if (!File.Exists(deviceInfoPath))
{
deviceInfo = BotDeviceInfo.GenerateInfo();
string json = JsonSerializer.Serialize(deviceInfo);
_deviceInfo = BotDeviceInfo.GenerateInfo();
string json = JsonSerializer.Serialize(_deviceInfo);
string? directoryPath = Path.GetDirectoryName(deviceInfoPath);
if (!string.IsNullOrEmpty(directoryPath))
{
Expand All @@ -66,18 +64,8 @@ public YounBotAppBuilder ConfigureBots()
}
else
{
deviceInfo = JsonSerializer.Deserialize<BotDeviceInfo>(File.ReadAllText(deviceInfoPath)) ?? BotDeviceInfo.GenerateInfo();
_deviceInfo = JsonSerializer.Deserialize<BotDeviceInfo>(File.ReadAllText(deviceInfoPath)) ?? BotDeviceInfo.GenerateInfo();
}

Services.AddSingleton(BotFactory.Create(config, deviceInfo, keystore));

return this;
}

public YounBotAppBuilder ConfigureLogging(Action<ILoggingBuilder> configureLogging)
{
Services.AddLogging(configureLogging);
return this;
}

public YounBotApp Build() => new(this);
Expand Down