Skip to content

Commit

Permalink
Merge pull request #15 from jonsagara/feature/log
Browse files Browse the repository at this point in the history
#14: Replace TextWriter with ILoggerFactory for logging to external l…
  • Loading branch information
jonsagara authored Dec 14, 2023
2 parents d8255f1 + 83b4ac1 commit d56b0fc
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 102 deletions.
19 changes: 10 additions & 9 deletions src/IdParser.Core/Barcode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Text.RegularExpressions;
using IdParser.Core.Constants;
using IdParser.Core.Parsers;
using Microsoft.Extensions.Logging;

namespace IdParser.Core;

Expand Down Expand Up @@ -53,8 +54,8 @@ public static class Barcode
/// No validation will be performed if none is specified and exceptions will not be thrown
/// for elements that do not match or do not adversely affect parsing.
/// </param>
/// <param name="log">The <see cref="TextWriter"/> to log to.</param>
public static BarcodeParseResult Parse(string rawPdf417Input, Validation validationLevel = Validation.Strict, TextWriter? log = null)
/// <param name="loggerFactory"><see cref="ILoggerFactory"/> to use to create an <see cref="ILogger"/> for logging.</param>
public static BarcodeParseResult Parse(string rawPdf417Input, Validation validationLevel = Validation.Strict, ILoggerFactory? loggerFactory = null)
{
ArgumentNullException.ThrowIfNull(rawPdf417Input);

Expand All @@ -63,15 +64,15 @@ public static BarcodeParseResult Parse(string rawPdf417Input, Validation validat
throw new ArgumentException($"The input is missing required header elements and is not a valid AAMVA format. Expected at least 31 characters. Received {rawPdf417Input.Length}.", nameof(rawPdf417Input));
}

using var logProxy = LogProxy.TryCreate(log);
ILogger? logger = loggerFactory?.CreateLogger(typeof(Barcode));

if (validationLevel == Validation.Strict)
{
ValidateHeaderFormat(rawPdf417Input);
}
else
{
rawPdf417Input = Fixes.TryToCorrectHeader(rawPdf417Input, logProxy);
rawPdf417Input = Fixes.TryToCorrectHeader(rawPdf417Input, loggerFactory);
}

var aamvaVersion = ParseAAMVAVersion(rawPdf417Input);
Expand All @@ -82,10 +83,10 @@ public static BarcodeParseResult Parse(string rawPdf417Input, Validation validat
var country = ParseCountry(idCard.IssuerIdentificationNumber, aamvaVersion, subfileRecords);
idCard.Address.Country = country;

var unhandledElementIds = PopulateIdCard(idCard, aamvaVersion, country, subfileRecords, logProxy);
var unhandledElementIds = PopulateIdCard(idCard, aamvaVersion, country, subfileRecords, logger);
if (unhandledElementIds.Count > 0)
{
logProxy?.WriteLine($"[{nameof(Barcode)}] One or more ElementIds were not handled by the ID or Driver's License parsers: {string.Join(", ", unhandledElementIds)}");
logger?.LogError($"One or more ElementIds were not handled by the ID or Driver's License parsers: {{UnhandledElementIds}}", string.Join(", ", unhandledElementIds));

Check warning on line 89 in src/IdParser.Core/Barcode.cs

View workflow job for this annotation

GitHub Actions / run_test

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogError(ILogger, string?, params object?[])'

Check warning on line 89 in src/IdParser.Core/Barcode.cs

View workflow job for this annotation

GitHub Actions / pack_nuget

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogError(ILogger, string?, params object?[])'
}

return new BarcodeParseResult(idCard, unhandledElementIds);
Expand Down Expand Up @@ -217,7 +218,7 @@ private static IdentificationCard GetIdCardInstance(string rawPdf417Input, AAMVA
return idCard;
}


private static readonly Regex _rxSubfile = new Regex("(DL|ID)([\\d\\w]{3,8})(DL|ID|Z\\w)([DZ][A-Z]{2})", RegexOptions.Compiled);

/// <summary>
Expand Down Expand Up @@ -340,7 +341,7 @@ private static Country ParseCountry(IssuerIdentificationNumber iin, AAMVAVersion
return IssuerMetadataHelper.GetCountry(iin);
}

private static IReadOnlyCollection<string> PopulateIdCard(IdentificationCard idCard, AAMVAVersion version, Country country, Dictionary<string, string> subfileRecords, LogProxy? logProxy)
private static IReadOnlyCollection<string> PopulateIdCard(IdentificationCard idCard, AAMVAVersion version, Country country, Dictionary<string, string> subfileRecords, ILogger? logger)
{
List<string> unhandledElementIds = new();

Expand Down Expand Up @@ -374,7 +375,7 @@ private static IReadOnlyCollection<string> PopulateIdCard(IdentificationCard idC
}
catch (Exception ex)
{
logProxy?.WriteLine($"[{nameof(Barcode)}] Unhandled exception in {nameof(PopulateIdCard)} while trying to parse element Id {elementId}: {ex}");
logger?.LogError(ex, $"Unhandled exception in {nameof(PopulateIdCard)} while trying to parse element Id {{ElementId}}", elementId);
throw;
}
}
Expand Down
32 changes: 18 additions & 14 deletions src/IdParser.Core/Fixes.cs
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
namespace IdParser.Core;
using Microsoft.Extensions.Logging;

namespace IdParser.Core;

internal static class Fixes
{
/// <summary>
/// If the header is invalid, try to correct it. Otherwise, return the string as-is.
/// </summary>
internal static string TryToCorrectHeader(string input, LogProxy? logProxy)
internal static string TryToCorrectHeader(string input, ILoggerFactory? loggerFactory)
{
var logger = loggerFactory?.CreateLogger(typeof(Fixes));

return input
.RemoveUndefinedCharacters()
.RemoveInvalidCharactersFromHeader(logProxy)
.FixIncorrectHeader(logProxy)
.RemoveIncorrectCarriageReturns(logProxy);
.RemoveInvalidCharactersFromHeader(logger)
.FixIncorrectHeader(logger)
.RemoveIncorrectCarriageReturns(logger);
}


Expand Down Expand Up @@ -54,14 +58,14 @@ private static string RemoveUndefinedCharacters(this string input)
/// Sometimes bad characters (e.g. @a ANSI) get into the header (usually through HID keyboard emulation).
/// Replace the header with what we are expecting.
/// </summary>
private static string RemoveInvalidCharactersFromHeader(this string input, LogProxy? logProxy)
private static string RemoveInvalidCharactersFromHeader(this string input, ILogger? logger)
{
input = input.TrimStart();

if (input[0] != '@')
{
// Text doesn't start with an input. Don't try to parse it further. Return it as-is.
logProxy?.WriteLine($"[{nameof(Fixes)}] Input doesn't start with the expected compliance indicator '{Barcode.ExpectedComplianceIndicator}'. Exiting {nameof(RemoveInvalidCharactersFromHeader)}.");
logger?.LogError($"Input doesn't start with the expected compliance indicator '{Barcode.ExpectedComplianceIndicator}'. Exiting {nameof(RemoveInvalidCharactersFromHeader)}.");

Check warning on line 68 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / run_test

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogError(ILogger, string?, params object?[])'

Check warning on line 68 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / run_test

The logging message template should not vary between calls to 'LoggerExtensions.LogError(ILogger, string?, params object?[])'

Check warning on line 68 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / pack_nuget

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogError(ILogger, string?, params object?[])'

Check warning on line 68 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / pack_nuget

The logging message template should not vary between calls to 'LoggerExtensions.LogError(ILogger, string?, params object?[])'
return input;
}

Expand All @@ -79,7 +83,7 @@ private static string RemoveInvalidCharactersFromHeader(this string input, LogPr
// The string "ANSI " exists in the text. Starting with the expected header value, append everything from
// the input string after the "ANSI " text.
// This ensures that the input text has a valid header.
logProxy?.WriteLine($"[{nameof(Fixes)}] Header contains '{Barcode.ExpectedFileType}'. Forcefully ensuring that the header is valid.");
logger?.LogInformation($"Header contains '{Barcode.ExpectedFileType}'. Forcefully ensuring that the header is valid.");

Check warning on line 86 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / run_test

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogInformation(ILogger, string?, params object?[])'

Check warning on line 86 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / pack_nuget

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogInformation(ILogger, string?, params object?[])'
return string.Concat(Barcode.ExpectedHeader, input.AsSpan(start: ixANSI + Barcode.ExpectedFileType.Length));
}

Expand All @@ -92,7 +96,7 @@ private static string RemoveInvalidCharactersFromHeader(this string input, LogPr
// Earlier versions of the spec must have had "AMMVA" instead of "ANSI " in the header. Starting with
// the current expected header value, append everything from the input string after the "AMMVA" text.
// This ensures that the input text has a valid header.
logProxy?.WriteLine($"[{nameof(Fixes)}] Header contains '{AAMVA}'. This is from an earlier spec. Replacing the old header with the current valid header text.");
logger?.LogInformation($"Header contains '{AAMVA}'. This is from an earlier spec. Replacing the old header with the current valid header text.");

Check warning on line 99 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / run_test

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogInformation(ILogger, string?, params object?[])'

Check warning on line 99 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / pack_nuget

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogInformation(ILogger, string?, params object?[])'
return string.Concat(Barcode.ExpectedHeader, input.AsSpan(start: aamvaPosition + AAMVA.Length));
}

Expand All @@ -104,7 +108,7 @@ private static string RemoveInvalidCharactersFromHeader(this string input, LogPr
/// HID keyboard emulation, especially entered via a web browser, tends to mutilate the header.
/// As long as part of the header is correct, this will fix the rest of it to make it parse-able.
/// </summary>
private static string FixIncorrectHeader(this string input, LogProxy? logProxy)
private static string FixIncorrectHeader(this string input, ILogger? logger)
{
if (input[0] == Barcode.ExpectedComplianceIndicator &&
input[1] == Barcode.ExpectedSegmentTerminator &&
Expand All @@ -114,7 +118,7 @@ private static string FixIncorrectHeader(this string input, LogProxy? logProxy)
{
// Header is expected to be "@\n\u0030\rANSI ", but is currently "@\r\n\u0030A". Insert "\r\n" in
// front of the A. We'll correct this later by removing incorrect "\r" characters.
logProxy?.WriteLine($"[{nameof(Fixes)}] Header is malformed, and starts with '@\\r\\n\\u0030A'. Changing it to '@\\r\\n\\u0030\\r\\nA'. A later method will remove incorrect \\r characters.");
logger?.LogWarning($"Header is malformed, and starts with '@\\r\\n\\u0030A'. Changing it to '@\\r\\n\\u0030\\r\\nA'. A later method will remove incorrect \\r characters.");

Check warning on line 121 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / run_test

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogWarning(ILogger, string?, params object?[])'

Check warning on line 121 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / pack_nuget

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogWarning(ILogger, string?, params object?[])'
return input.Insert(startIndex: 4, value: $"{Barcode.ExpectedSegmentTerminator}{Barcode.ExpectedDataElementSeparator}");
}

Expand All @@ -125,16 +129,16 @@ private static string FixIncorrectHeader(this string input, LogProxy? logProxy)
/// HID keyboard emulation (and some other methods) tend to replace the \r with \r\n, which is invalid and doesn't
/// conform to the AAMVA standard. This fixes it before attempting to parse the fields.
/// </summary>
private static string RemoveIncorrectCarriageReturns(this string input, LogProxy? logProxy)
private static string RemoveIncorrectCarriageReturns(this string input, ILogger? logger)
{
if (input.Contains("\r\n", StringComparison.Ordinal))
{
// Input contains CRLFs (\r\n). Remove all CRs (\r).
logProxy?.WriteLine($"[{nameof(Fixes)}] Scanned text contains \\r characters. Remove them.");
logger?.LogInformation($"Scanned text contains \\r characters. Removing them.");

Check warning on line 137 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / run_test

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogInformation(ILogger, string?, params object?[])'

Check warning on line 137 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / pack_nuget

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogInformation(ILogger, string?, params object?[])'
var inputWithoutCRs = input.Replace("\r", string.Empty, StringComparison.Ordinal);

// Add back the one CR (\r) that is required in the header.
logProxy?.WriteLine($"[{nameof(Fixes)}] Add the single allowed \\r character back to the header.");
logger?.LogInformation($"Adding the single allowed \\r character back to the header.");

Check warning on line 141 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / run_test

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogInformation(ILogger, string?, params object?[])'

Check warning on line 141 in src/IdParser.Core/Fixes.cs

View workflow job for this annotation

GitHub Actions / pack_nuget

For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogInformation(ILogger, string?, params object?[])'
return $"{inputWithoutCRs.AsSpan(start: 0, length: 3)}\r{inputWithoutCRs.AsSpan(start: 4)}";
}

Expand Down
1 change: 1 addition & 0 deletions src/IdParser.Core/IdParser.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="6.0.4" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
Expand Down
79 changes: 0 additions & 79 deletions src/IdParser.Core/LogProxy.cs

This file was deleted.

0 comments on commit d56b0fc

Please sign in to comment.