refactor(historian): single-parse ExtractTagHistorize + review-nit tests/docs
Stop parsing TagConfig twice per tag on the deploy hot path: Phase7Composer's equipment-tag Select lambda is now block-bodied (captures isHistorized/historianTagname once), and DeploymentArtifact.BuildEquipmentTagPlans captures locals before result.Add. Add wrong-type-historianTagname InlineData to ExtractTagHistorizeTests. Extend the parity round-trip fixture with a 4th tag (isHistorized:false + JSON-null tagname) exercising the artifact-side private guard path. Align DeploymentArtifact's ExtractTagHistorize doc-comment with the composer-side phrasing (ExtractTagFullName / ExtractTagAlarm cross-reference).
This commit is contained in:
@@ -335,18 +335,22 @@ public static class Phase7Composer
|
|||||||
.OrderBy(t => t.EquipmentId, StringComparer.Ordinal)
|
.OrderBy(t => t.EquipmentId, StringComparer.Ordinal)
|
||||||
.ThenBy(t => t.FolderPath ?? string.Empty, StringComparer.Ordinal) // coalesce so the sort matches the artifact-decode side exactly
|
.ThenBy(t => t.FolderPath ?? string.Empty, StringComparer.Ordinal) // coalesce so the sort matches the artifact-decode side exactly
|
||||||
.ThenBy(t => t.Name, StringComparer.Ordinal)
|
.ThenBy(t => t.Name, StringComparer.Ordinal)
|
||||||
.Select(t => new EquipmentTagPlan(
|
.Select(t =>
|
||||||
TagId: t.TagId,
|
{
|
||||||
EquipmentId: t.EquipmentId!,
|
var (isHistorized, historianTagname) = ExtractTagHistorize(t.TagConfig);
|
||||||
DriverInstanceId: t.DriverInstanceId,
|
return new EquipmentTagPlan(
|
||||||
FolderPath: t.FolderPath ?? string.Empty,
|
TagId: t.TagId,
|
||||||
Name: t.Name,
|
EquipmentId: t.EquipmentId!,
|
||||||
DataType: t.DataType,
|
DriverInstanceId: t.DriverInstanceId,
|
||||||
FullName: ExtractTagFullName(t.TagConfig),
|
FolderPath: t.FolderPath ?? string.Empty,
|
||||||
Writable: t.AccessLevel == TagAccessLevel.ReadWrite,
|
Name: t.Name,
|
||||||
Alarm: ExtractTagAlarm(t.TagConfig),
|
DataType: t.DataType,
|
||||||
IsHistorized: ExtractTagHistorize(t.TagConfig).IsHistorized,
|
FullName: ExtractTagFullName(t.TagConfig),
|
||||||
HistorianTagname: ExtractTagHistorize(t.TagConfig).HistorianTagname))
|
Writable: t.AccessLevel == TagAccessLevel.ReadWrite,
|
||||||
|
Alarm: ExtractTagAlarm(t.TagConfig),
|
||||||
|
IsHistorized: isHistorized,
|
||||||
|
HistorianTagname: historianTagname);
|
||||||
|
})
|
||||||
.ToList();
|
.ToList();
|
||||||
|
|
||||||
// Per-equipment tag base = the shared substring-before-first-dot across each equipment's
|
// Per-equipment tag base = the shared substring-before-first-dot across each equipment's
|
||||||
|
|||||||
@@ -437,6 +437,7 @@ public static class DeploymentArtifact
|
|||||||
// ordinary equipment tags now (GalaxyMxGateway is a standard Equipment-kind driver).
|
// ordinary equipment tags now (GalaxyMxGateway is a standard Equipment-kind driver).
|
||||||
if (!equipmentNamespaces.Contains(nsId)) continue;
|
if (!equipmentNamespaces.Contains(nsId)) continue;
|
||||||
|
|
||||||
|
var (isHistorized, historianTagname) = ExtractTagHistorize(tagConfig);
|
||||||
result.Add(new EquipmentTagPlan(
|
result.Add(new EquipmentTagPlan(
|
||||||
TagId: tagId!,
|
TagId: tagId!,
|
||||||
EquipmentId: equipmentId!,
|
EquipmentId: equipmentId!,
|
||||||
@@ -447,8 +448,8 @@ public static class DeploymentArtifact
|
|||||||
FullName: ExtractTagFullName(tagConfig),
|
FullName: ExtractTagFullName(tagConfig),
|
||||||
Writable: writable,
|
Writable: writable,
|
||||||
Alarm: ExtractTagAlarm(tagConfig),
|
Alarm: ExtractTagAlarm(tagConfig),
|
||||||
IsHistorized: ExtractTagHistorize(tagConfig).IsHistorized,
|
IsHistorized: isHistorized,
|
||||||
HistorianTagname: ExtractTagHistorize(tagConfig).HistorianTagname));
|
HistorianTagname: historianTagname));
|
||||||
}
|
}
|
||||||
|
|
||||||
result.Sort((a, b) =>
|
result.Sort((a, b) =>
|
||||||
@@ -678,7 +679,8 @@ public static class DeploymentArtifact
|
|||||||
/// the <c>isHistorized</c> bool (absent / not a bool / non-object root / blank / malformed ⇒
|
/// the <c>isHistorized</c> bool (absent / not a bool / non-object root / blank / malformed ⇒
|
||||||
/// <c>false</c>) and the optional <c>historianTagname</c> string override (absent / not a string /
|
/// <c>false</c>) and the optional <c>historianTagname</c> string override (absent / not a string /
|
||||||
/// whitespace-or-empty ⇒ <c>null</c>, meaning the historian tagname defaults to the tag's FullName,
|
/// whitespace-or-empty ⇒ <c>null</c>, meaning the historian tagname defaults to the tag's FullName,
|
||||||
/// resolved later). The raw string value is used — not trimmed. Never throws. The live-edit side
|
/// resolved later). The raw string value is used — not trimmed — matching <c>ExtractTagFullName</c> /
|
||||||
|
/// <c>ExtractTagAlarm</c>. Never throws. The live-edit side
|
||||||
/// (<c>Phase7Composer.ExtractTagHistorize</c>) MUST parse identically (byte-parity).</summary>
|
/// (<c>Phase7Composer.ExtractTagHistorize</c>) MUST parse identically (byte-parity).</summary>
|
||||||
private static (bool IsHistorized, string? HistorianTagname) ExtractTagHistorize(string? tagConfig)
|
private static (bool IsHistorized, string? HistorianTagname) ExtractTagHistorize(string? tagConfig)
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -24,6 +24,8 @@ public class ExtractTagHistorizeTests
|
|||||||
[InlineData("[1,2]", false, null)]
|
[InlineData("[1,2]", false, null)]
|
||||||
// Wrong type for isHistorized (string, not bool) ⇒ false.
|
// Wrong type for isHistorized (string, not bool) ⇒ false.
|
||||||
[InlineData("{\"isHistorized\":\"yes\"}", false, null)]
|
[InlineData("{\"isHistorized\":\"yes\"}", false, null)]
|
||||||
|
// Wrong type for historianTagname (number, not string) ⇒ tagname null, flag still honoured.
|
||||||
|
[InlineData("{\"isHistorized\":true,\"historianTagname\":123}", true, null)]
|
||||||
public void ExtractTagHistorize_parses_or_returns_defaults(string? cfg, bool expectedHistorized, string? expectedTagname)
|
public void ExtractTagHistorize_parses_or_returns_defaults(string? cfg, bool expectedHistorized, string? expectedTagname)
|
||||||
{
|
{
|
||||||
var (isHistorized, historianTagname) = Phase7Composer.ExtractTagHistorize(cfg);
|
var (isHistorized, historianTagname) = Phase7Composer.ExtractTagHistorize(cfg);
|
||||||
|
|||||||
+29
-2
@@ -93,12 +93,30 @@ public sealed class DeploymentArtifactHistorizeParityTests
|
|||||||
AccessLevel = TagAccessLevel.ReadWrite,
|
AccessLevel = TagAccessLevel.ReadWrite,
|
||||||
TagConfig = "{\"FullName\":\"40003\"}",
|
TagConfig = "{\"FullName\":\"40003\"}",
|
||||||
};
|
};
|
||||||
|
// Well-formed but non-historized: explicit isHistorized:false + a JSON-null historianTagname
|
||||||
|
// (not a string token → falls through the ValueKind==String guard → tagname null).
|
||||||
|
// This exercises the artifact-side private ExtractTagHistorize through the real round-trip
|
||||||
|
// (it can't be unit-tested directly because it's private). A truly malformed TagConfig string
|
||||||
|
// would cause ExtractTagFullName to return "" and break the SequenceEqual on other fields, so
|
||||||
|
// we use a well-formed blob and note that the malformed/throws path is already covered by
|
||||||
|
// the composer unit test in ExtractTagHistorizeTests.
|
||||||
|
var notHistorizedNullNameTag = new Tag
|
||||||
|
{
|
||||||
|
TagId = "tag-not-hist-nullname",
|
||||||
|
DriverInstanceId = "drv-modbus",
|
||||||
|
EquipmentId = "eq-1",
|
||||||
|
FolderPath = null,
|
||||||
|
Name = "Temp",
|
||||||
|
DataType = "Float",
|
||||||
|
AccessLevel = TagAccessLevel.Read,
|
||||||
|
TagConfig = "{\"FullName\":\"40004\",\"isHistorized\":false,\"historianTagname\":null}",
|
||||||
|
};
|
||||||
|
|
||||||
var areas = new[] { area };
|
var areas = new[] { area };
|
||||||
var lines = new[] { line };
|
var lines = new[] { line };
|
||||||
var equipment = new[] { equip };
|
var equipment = new[] { equip };
|
||||||
var drivers = new[] { driver };
|
var drivers = new[] { driver };
|
||||||
var tags = new[] { histDefaultTag, histOverrideTag, plainTag };
|
var tags = new[] { histDefaultTag, histOverrideTag, plainTag, notHistorizedNullNameTag };
|
||||||
var namespaces = new[] { ns };
|
var namespaces = new[] { ns };
|
||||||
|
|
||||||
// ---- Side 1: the live-edit composer ----
|
// ---- Side 1: the live-edit composer ----
|
||||||
@@ -121,13 +139,14 @@ public sealed class DeploymentArtifactHistorizeParityTests
|
|||||||
ToSnapshot(histDefaultTag),
|
ToSnapshot(histDefaultTag),
|
||||||
ToSnapshot(histOverrideTag),
|
ToSnapshot(histOverrideTag),
|
||||||
ToSnapshot(plainTag),
|
ToSnapshot(plainTag),
|
||||||
|
ToSnapshot(notHistorizedNullNameTag),
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
var decoded = DeploymentArtifact.ParseComposition(blob);
|
var decoded = DeploymentArtifact.ParseComposition(blob);
|
||||||
|
|
||||||
// ---- Full byte-parity: every field, same order (positional-record value equality) ----
|
// ---- Full byte-parity: every field, same order (positional-record value equality) ----
|
||||||
decoded.EquipmentTags.Count.ShouldBe(3);
|
decoded.EquipmentTags.Count.ShouldBe(4);
|
||||||
decoded.EquipmentTags.SequenceEqual(composed.EquipmentTags).ShouldBeTrue();
|
decoded.EquipmentTags.SequenceEqual(composed.EquipmentTags).ShouldBeTrue();
|
||||||
|
|
||||||
// Spell out the Phase C fields per-tag so a divergence names the offending tag.
|
// Spell out the Phase C fields per-tag so a divergence names the offending tag.
|
||||||
@@ -148,6 +167,14 @@ public sealed class DeploymentArtifactHistorizeParityTests
|
|||||||
plain.HistorianTagname.ShouldBeNull();
|
plain.HistorianTagname.ShouldBeNull();
|
||||||
composed.EquipmentTags.Single(t => t.TagId == "tag-plain").IsHistorized.ShouldBeFalse();
|
composed.EquipmentTags.Single(t => t.TagId == "tag-plain").IsHistorized.ShouldBeFalse();
|
||||||
composed.EquipmentTags.Single(t => t.TagId == "tag-plain").HistorianTagname.ShouldBeNull();
|
composed.EquipmentTags.Single(t => t.TagId == "tag-plain").HistorianTagname.ShouldBeNull();
|
||||||
|
|
||||||
|
// 4th tag: explicit isHistorized:false + JSON-null historianTagname ⇒ (false, null) on both sides.
|
||||||
|
// Exercises the artifact-side private ExtractTagHistorize's null-JSON-token guard path.
|
||||||
|
var notHistorizedNullName = decoded.EquipmentTags.Single(t => t.TagId == "tag-not-hist-nullname");
|
||||||
|
notHistorizedNullName.IsHistorized.ShouldBeFalse();
|
||||||
|
notHistorizedNullName.HistorianTagname.ShouldBeNull();
|
||||||
|
composed.EquipmentTags.Single(t => t.TagId == "tag-not-hist-nullname").IsHistorized.ShouldBeFalse();
|
||||||
|
composed.EquipmentTags.Single(t => t.TagId == "tag-not-hist-nullname").HistorianTagname.ShouldBeNull();
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>The Pascal-case snapshot a <see cref="Tag"/> EF entity serialises to in the artifact
|
/// <summary>The Pascal-case snapshot a <see cref="Tag"/> EF entity serialises to in the artifact
|
||||||
|
|||||||
Reference in New Issue
Block a user