Skip to content

Commit

Permalink
fix(logging): fix null object reference without logging configured (#70)
Browse files Browse the repository at this point in the history
  • Loading branch information
hamzamahmood authored May 13, 2024
1 parent f2d6a70 commit 54b06c1
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 24 deletions.
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);
}
}

0 comments on commit 54b06c1

Please sign in to comment.