fix(multivalue): NJ-3/NJ-4/NJ-5 review fixes
- NJ-3: widen per-row catch to Exception (an STJ encode failure can't abort startup); drop dead null-guard already excluded by the SQL filter - NJ-4: capture logger/instanceName in locals for the fire-and-forget normalize continuation (match the sibling pattern in this actor) - NJ-5: emit a warn-log when a malformed List value is imported verbatim; thread an optional ILogger<BundleImporter> to the sync re-import site
This commit is contained in:
@@ -54,10 +54,13 @@ public static class ListValueNormalizer
|
|||||||
rewritten++;
|
rewritten++;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
catch (FormatException ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
|
// Never abort startup for a single bad row. FormatException from Decode is the
|
||||||
|
// expected case; the broad catch also covers an unexpected serialize failure
|
||||||
|
// (e.g. a JsonException on a non-finite value) so one poison row can't crash boot.
|
||||||
logger?.LogWarning(ex,
|
logger?.LogWarning(ex,
|
||||||
"List value normalizer: skipping unparseable list value for TemplateAttribute {Id}.",
|
"List value normalizer: skipping unprocessable list value for TemplateAttribute {Id}.",
|
||||||
a.Id);
|
a.Id);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -71,14 +74,6 @@ public static class ListValueNormalizer
|
|||||||
|
|
||||||
foreach (var o in overrideRows)
|
foreach (var o in overrideRows)
|
||||||
{
|
{
|
||||||
if (o.ElementDataType is null)
|
|
||||||
{
|
|
||||||
logger?.LogDebug(
|
|
||||||
"List value normalizer: skipping InstanceAttributeOverride {Id} with no element type.",
|
|
||||||
o.Id);
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
var native = AttributeValueCodec.Encode(
|
var native = AttributeValueCodec.Encode(
|
||||||
@@ -89,10 +84,10 @@ public static class ListValueNormalizer
|
|||||||
rewritten++;
|
rewritten++;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
catch (FormatException ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
logger?.LogWarning(ex,
|
logger?.LogWarning(ex,
|
||||||
"List value normalizer: skipping unparseable list value for InstanceAttributeOverride {Id}.",
|
"List value normalizer: skipping unprocessable list value for InstanceAttributeOverride {Id}.",
|
||||||
o.Id);
|
o.Id);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -966,10 +966,12 @@ public class InstanceActor : ReceiveActor
|
|||||||
if (native != kvp.Value) // stored value was old-form → normalize on disk
|
if (native != kvp.Value) // stored value was old-form → normalize on disk
|
||||||
{
|
{
|
||||||
var key = kvp.Key;
|
var key = kvp.Key;
|
||||||
_storage.SetStaticOverrideAsync(_instanceUniqueName, key, native!)
|
var logger = _logger;
|
||||||
.ContinueWith(t => _logger.LogWarning(t.Exception?.GetBaseException(),
|
var instanceName = _instanceUniqueName;
|
||||||
|
_storage.SetStaticOverrideAsync(instanceName, key, native!)
|
||||||
|
.ContinueWith(t => logger.LogWarning(t.Exception?.GetBaseException(),
|
||||||
"Failed to normalize static override {Instance}.{Attr} to native JSON",
|
"Failed to normalize static override {Instance}.{Attr} to native JSON",
|
||||||
_instanceUniqueName, key),
|
instanceName, key),
|
||||||
TaskContinuationOptions.OnlyOnFaulted);
|
TaskContinuationOptions.OnlyOnFaulted);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
using System.IO.Compression;
|
using System.IO.Compression;
|
||||||
using System.Security.Cryptography;
|
using System.Security.Cryptography;
|
||||||
using System.Text.Json;
|
using System.Text.Json;
|
||||||
|
using Microsoft.Extensions.Logging;
|
||||||
using Microsoft.Extensions.Options;
|
using Microsoft.Extensions.Options;
|
||||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems;
|
using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems;
|
||||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
|
using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
|
||||||
@@ -70,6 +71,7 @@ public sealed class BundleImporter : IBundleImporter
|
|||||||
private readonly IOptions<TransportOptions> _options;
|
private readonly IOptions<TransportOptions> _options;
|
||||||
private readonly TimeProvider _timeProvider;
|
private readonly TimeProvider _timeProvider;
|
||||||
private readonly SemanticValidator _semanticValidator;
|
private readonly SemanticValidator _semanticValidator;
|
||||||
|
private readonly ILogger<BundleImporter>? _logger;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Initializes a new <see cref="BundleImporter"/> with all required dependencies.
|
/// Initializes a new <see cref="BundleImporter"/> with all required dependencies.
|
||||||
@@ -106,7 +108,8 @@ public sealed class BundleImporter : IBundleImporter
|
|||||||
IAuditService auditService,
|
IAuditService auditService,
|
||||||
IAuditCorrelationContext correlationContext,
|
IAuditCorrelationContext correlationContext,
|
||||||
ScadaBridgeDbContext dbContext,
|
ScadaBridgeDbContext dbContext,
|
||||||
SemanticValidator semanticValidator)
|
SemanticValidator semanticValidator,
|
||||||
|
ILogger<BundleImporter>? logger = null)
|
||||||
{
|
{
|
||||||
_bundleSerializer = bundleSerializer ?? throw new ArgumentNullException(nameof(bundleSerializer));
|
_bundleSerializer = bundleSerializer ?? throw new ArgumentNullException(nameof(bundleSerializer));
|
||||||
_manifestValidator = manifestValidator ?? throw new ArgumentNullException(nameof(manifestValidator));
|
_manifestValidator = manifestValidator ?? throw new ArgumentNullException(nameof(manifestValidator));
|
||||||
@@ -124,6 +127,7 @@ public sealed class BundleImporter : IBundleImporter
|
|||||||
_correlationContext = correlationContext ?? throw new ArgumentNullException(nameof(correlationContext));
|
_correlationContext = correlationContext ?? throw new ArgumentNullException(nameof(correlationContext));
|
||||||
_dbContext = dbContext ?? throw new ArgumentNullException(nameof(dbContext));
|
_dbContext = dbContext ?? throw new ArgumentNullException(nameof(dbContext));
|
||||||
_semanticValidator = semanticValidator ?? throw new ArgumentNullException(nameof(semanticValidator));
|
_semanticValidator = semanticValidator ?? throw new ArgumentNullException(nameof(semanticValidator));
|
||||||
|
_logger = logger;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
@@ -1120,7 +1124,7 @@ public sealed class BundleImporter : IBundleImporter
|
|||||||
// stores natively — otherwise an idempotent re-import of an old-form
|
// stores natively — otherwise an idempotent re-import of an old-form
|
||||||
// bundle would spuriously report a Value change.
|
// bundle would spuriously report a Value change.
|
||||||
var normalizedValue = ImportValueNormalizer.NormalizeListValue(
|
var normalizedValue = ImportValueNormalizer.NormalizeListValue(
|
||||||
attrDto.Value, attrDto.DataType, attrDto.ElementDataType);
|
attrDto.Value, attrDto.DataType, attrDto.ElementDataType, _logger, attrDto.Name);
|
||||||
if (existingByName.TryGetValue(attrDto.Name, out var current))
|
if (existingByName.TryGetValue(attrDto.Name, out var current))
|
||||||
{
|
{
|
||||||
// Update only if any field actually changed.
|
// Update only if any field actually changed.
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
using Microsoft.Extensions.Logging;
|
||||||
using ZB.MOM.WW.ScadaBridge.Commons.Types;
|
using ZB.MOM.WW.ScadaBridge.Commons.Types;
|
||||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
|
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
|
||||||
|
|
||||||
@@ -29,7 +30,14 @@ internal static class ImportValueNormalizer
|
|||||||
/// <param name="value">The attribute value as carried by the bundle DTO.</param>
|
/// <param name="value">The attribute value as carried by the bundle DTO.</param>
|
||||||
/// <param name="dataType">The attribute's declared data type.</param>
|
/// <param name="dataType">The attribute's declared data type.</param>
|
||||||
/// <param name="elementType">The List element type (null for scalars).</param>
|
/// <param name="elementType">The List element type (null for scalars).</param>
|
||||||
public static string? NormalizeListValue(string? value, DataType dataType, DataType? elementType)
|
/// <param name="logger">Optional logger; a warning is emitted when a malformed value is left as-is.</param>
|
||||||
|
/// <param name="attributeName">Optional attribute name for the diagnostic message.</param>
|
||||||
|
public static string? NormalizeListValue(
|
||||||
|
string? value,
|
||||||
|
DataType dataType,
|
||||||
|
DataType? elementType,
|
||||||
|
ILogger? logger = null,
|
||||||
|
string? attributeName = null)
|
||||||
{
|
{
|
||||||
if (dataType != DataType.List || string.IsNullOrEmpty(value))
|
if (dataType != DataType.List || string.IsNullOrEmpty(value))
|
||||||
{
|
{
|
||||||
@@ -41,10 +49,14 @@ internal static class ImportValueNormalizer
|
|||||||
return AttributeValueCodec.Encode(
|
return AttributeValueCodec.Encode(
|
||||||
AttributeValueCodec.Decode(value, DataType.List, elementType));
|
AttributeValueCodec.Decode(value, DataType.List, elementType));
|
||||||
}
|
}
|
||||||
catch (FormatException)
|
catch (FormatException ex)
|
||||||
{
|
{
|
||||||
// Leave malformed values exactly as imported; the DB normalizer is
|
// Leave malformed values exactly as imported; the DB normalizer is
|
||||||
// the backstop. Never abort the import for a single bad value.
|
// the backstop. Never abort the import for a single bad value.
|
||||||
|
logger?.LogWarning(ex,
|
||||||
|
"Bundle import: could not normalize List value for attribute {Attribute}; " +
|
||||||
|
"importing verbatim (the central DB normalizer is the backstop).",
|
||||||
|
attributeName ?? "(unknown)");
|
||||||
return value;
|
return value;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user