Skip to content
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.

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

fix(logging): fix null object reference without logging configured #70

Merged
merged 6 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion APIMatic.Core.Test/TestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using APIMatic.Core.Test.MockTypes.Authentication;
using APIMatic.Core.Types;
using APIMatic.Core.Utilities.Logger.Configuration;
using Microsoft.Extensions.Logging.Abstractions;
using RichardSzalay.MockHttp;

namespace APIMatic.Core.Test
Expand Down Expand Up @@ -55,7 +56,7 @@ protected enum MockServer { Server1, Server2 }
("{language}", "my lang"),
("{version}", "1.*.*")
})
.LoggingConfig(SdkLoggingConfiguration.Default())
.LoggingConfig(null)
.ApiCallback(ApiCallBack)
.Build()
);
Expand Down
3 changes: 2 additions & 1 deletion APIMatic.Core.Test/Utilities/Logger/RequestTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ public void LogRequest_NotConfigured_NoLogSent()
var logger = new TestLogger();
var loggingConfiguration = LoggerHelper.GetSdkLoggingConfiguration(logger: NullLogger.Instance);
var sdkLogger = new SdkLogger(loggingConfiguration);
var request = new CoreRequest(HttpMethod.Post, "https://example.com/api/resource", null, null, null);
var request = new CoreRequest(HttpMethod.Post, "https://example.com/api/resource",
new Dictionary<string, string>(), null, null);

// Act
sdkLogger.LogRequest(request);
Expand Down
4 changes: 2 additions & 2 deletions APIMatic.Core/ApiCall.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class ApiCall<Request, Response, Context, ApiException, ReturnType, Respo
private readonly Func<Response, ResponseType, ReturnType> returnTypeCreator;
private Enum apiCallServer;
private RequestBuilder requestBuilder;
private readonly SdkLogger _sdkLogger;
private readonly ISdkLogger _sdkLogger;

/// <summary>
/// Creates a new instance of ApiCall
Expand All @@ -54,7 +54,7 @@ public ApiCall(GlobalConfiguration configuration, ICompatibilityFactory<Request,
arraySerialization = serialization;
this.returnTypeCreator = returnTypeCreator;
responseHandler = new ResponseHandler<Request, Response, Context, ApiException, ResponseType>(compatibility, globalErrors);
_sdkLogger = new SdkLogger(configuration.SdkLoggingConfiguration);
_sdkLogger = SdkLoggerFactory.Create(configuration.SdkLoggingConfiguration);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class SdkLoggingConfiguration
/// <summary>
/// Gets or sets whether sensitive headers should be masked in logs.
/// </summary>
public bool MaskSensitiveHeaders { get; }
public bool MaskSensitiveHeaders { get; }

/// <summary>
/// Gets or sets the configuration for request logging.
Expand All @@ -34,11 +34,6 @@ public class SdkLoggingConfiguration
/// </summary>
public ResponseLoggingConfiguration ResponseLoggingConfiguration { get; }

/// <summary>
/// Gets a value indicating whether the logging configuration is fully set up.
/// </summary>
public bool IsConfigured => Logger != NullLogger.Instance;

private SdkLoggingConfiguration(ILogger logger, LogLevel? logLevel, bool maskSensitiveHeaders,
RequestLoggingConfiguration requestLoggingConfiguration,
ResponseLoggingConfiguration responseLoggingConfiguration)
Expand All @@ -49,19 +44,12 @@ private SdkLoggingConfiguration(ILogger logger, LogLevel? logLevel, bool maskSen
RequestLoggingConfiguration = requestLoggingConfiguration;
ResponseLoggingConfiguration = responseLoggingConfiguration;
}


public static SdkLoggingConfiguration Default() =>
new SdkLoggingConfiguration(NullLogger.Instance, null, true,
RequestLoggingConfiguration.Default(),
ResponseLoggingConfiguration.Default());

public static SdkLoggingConfiguration Console() =>
new SdkLoggingConfiguration(ConsoleLogger.Instance, null, true,
RequestLoggingConfiguration.Default(),
ResponseLoggingConfiguration.Default());


public static SdkLoggingConfiguration Builder(ILogger logger, LogLevel? logLevel, bool maskSensitiveHeaders,
RequestLoggingConfiguration requestLoggingConfiguration,
ResponseLoggingConfiguration responseLoggingConfiguration)
Expand Down
3 changes: 1 addition & 2 deletions APIMatic.Core/Utilities/Logger/ConsoleLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ public class ConsoleLogger : ILogger
/// Returns the shared instance of <see cref="T:Microsoft.Extensions.Logging.Abstractions.NullLogger" />.
/// </summary>
public static ConsoleLogger Instance { get; } = new ConsoleLogger();



/// <summary>
/// Initializes a new instance of the <see cref="T:Microsoft.Extensions.Logging.Abstractions.NullLogger" /> class.
/// </summary>
Expand Down
19 changes: 19 additions & 0 deletions APIMatic.Core/Utilities/Logger/ISdkLogger.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using APIMatic.Core.Types.Sdk;

namespace APIMatic.Core.Utilities.Logger
{
internal interface ISdkLogger
{
/// <summary>
/// Logs the details of a request.
/// </summary>
/// <param name="request">The request to be logged.</param>
void LogRequest(CoreRequest request);

/// <summary>
/// Logs the details of a response.
/// </summary>
/// <param name="response">The response to be logged.</param>
void LogResponse(CoreResponse response);
}
}
17 changes: 17 additions & 0 deletions APIMatic.Core/Utilities/Logger/NullSdkLogger.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using APIMatic.Core.Types.Sdk;

namespace APIMatic.Core.Utilities.Logger
{
internal class NullSdkLogger : ISdkLogger
{
public void LogRequest(CoreRequest request)
{
// Method intentionally left empty.
}

public void LogResponse(CoreResponse response)
{
// Method intentionally left empty.
}
}
}
6 changes: 1 addition & 5 deletions APIMatic.Core/Utilities/Logger/SdkLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@ namespace APIMatic.Core.Utilities.Logger
/// <summary>
/// Provides logging functionality for SDK operations.
/// </summary>
internal class SdkLogger
internal class SdkLogger : ISdkLogger
{
private readonly ILogger _logger;
private readonly Func<LogLevel, LogLevel> _getOverridenLogLevel;
private readonly RequestLoggingConfiguration _requestConfiguration;
private readonly ResponseLoggingConfiguration _responseConfiguration;
private readonly bool _isConfigured;
private readonly bool _maskSensitiveHeaders;

/// <summary>
Expand All @@ -28,7 +27,6 @@ public SdkLogger(SdkLoggingConfiguration loggingConfiguration)
_getOverridenLogLevel = level => loggingConfiguration.LogLevel.GetValueOrDefault(level);
_requestConfiguration = loggingConfiguration.RequestLoggingConfiguration;
_responseConfiguration = loggingConfiguration.ResponseLoggingConfiguration;
_isConfigured = loggingConfiguration.IsConfigured;
_maskSensitiveHeaders = loggingConfiguration.MaskSensitiveHeaders;
}

Expand All @@ -38,7 +36,6 @@ public SdkLogger(SdkLoggingConfiguration loggingConfiguration)
/// <param name="request">The request to be logged.</param>
public void LogRequest(CoreRequest request)
{
if (!_isConfigured) return;
var localLogLevel = _getOverridenLogLevel(LogLevel.Information);
var contentTypeHeader = request.Headers.GetContentType();
var url = _requestConfiguration.IncludeQueryInPath ? request.QueryUrl : ParseQueryPath(request.QueryUrl);
Expand Down Expand Up @@ -66,7 +63,6 @@ public void LogRequest(CoreRequest request)
/// <param name="response">The response to be logged.</param>
public void LogResponse(CoreResponse response)
{
if (!_isConfigured) return;
var localLogLevel = _getOverridenLogLevel(LogLevel.Information);
var contentTypeHeader = response.Headers.GetContentType();
var contentLengthHeader = response.Headers.GetContentLength();
Expand Down
12 changes: 12 additions & 0 deletions APIMatic.Core/Utilities/Logger/SdkLoggingFactory.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using APIMatic.Core.Utilities.Logger.Configuration;

namespace APIMatic.Core.Utilities.Logger
{
internal static class SdkLoggerFactory
{
public static ISdkLogger Create(SdkLoggingConfiguration sdkLoggingConfiguration) =>
sdkLoggingConfiguration == null
? (ISdkLogger)new NullSdkLogger()
: new SdkLogger(sdkLoggingConfiguration);
}
}
Loading