From 92c4c6cddbbb973e49bfdc855e7c462291529288 Mon Sep 17 00:00:00 2001 From: Grant Holliday Date: Thu, 28 May 2026 14:14:07 +1000 Subject: [PATCH] Fix NativeAOT round-trip for tm7 file I/O The published AOT exe could neither read nor write .tm7 files: the reflection-based DataContractSerializer hit three AOT-incompatible patterns (Dictionary<,> falling back to ISerializable, Nullable member serialization via MakeGenericMethod, and EmitDefaultValue=false calling GetDefaultValue()). Model refactor: - Add SerializableKvpTypes.cs with three [DataContract] KVP types plus matching [CollectionDataContract] List wrappers; element names match what DCS auto-generates (KeyValueOfguidanyType, KeyValueOfstringThreat- pc_P0_PhOB, KeyValueOfstringstring) so wire compat with TMT is preserved. - Mark Borders / Lines / AllThreatsDictionary / SerializableThreat.Properties [IgnoreDataMember] and back each with a private [DataMember] List field synced via [OnSerializing] / [OnDeserialized] callbacks. Public Dictionary<,> API surface unchanged - no consumer touched. - Change bool? ThreatGenerationEnabled to bool (Nullable reflection- writer path is incompatible with AOT). Constructor still accepts bool? and coerces null to false. - Drop EmitDefaultValue=false from Zoom / StrokeThickness / StrokeDashArray (DCS's GetDefaultValue uses MakeGenericMethod which AOT cannot generate). Wire bytes now include these fields with default/null values. Build / serializer plumbing: - Switch IL3050/IL3053 from NoWarn to WarningsNotAsErrors so the BCL DCS warnings remain visible without failing the build. - Drop the dead string-typed DynamicDependency entries for KeyValue<,> closed generics (a failed experimental approach) and the analogous trimmer-root entry. Keep Dictionary<,> / EqualityComparer<,> preservation as defensive in case a future regression re-introduces a [DataMember] Dictionary<,>. Tests / CI: - Replace the byte-for-byte round-trip test (no longer achievable) with a structural-equivalence test. - Add PopulatedDictionaryTests covering populated AllThreatsDictionary, populated SerializableThreat.Properties, multiple add/remove cycles, null-dictionary serialization, and a wire-format guard that compares our KVP element names against DCS-generated names for the equivalent raw Dictionary<,>. The wire-format test caught a wrong-name guess during development. - Add AotExeIntegrationTests that shell out to the published AOT exe and validate populated-threat round-trip through the reflection writer (the JIT in-proc tests use code-gen and cannot exercise that path). Self-skip when the publish output is absent. - Add a Publish AOT step to the CI workflow ahead of Test so the integration tests actually run on every matrix leg. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 3 + src/Tm7.Cli/Tm7.Cli.csproj | 6 + .../SerializableDrawingSurfaceModel.cs | 42 ++- .../Tm7Model/SerializableElementType.cs | 4 +- src/Tm7.Cli/Tm7Model/SerializableKvpTypes.cs | 71 +++++ src/Tm7.Cli/Tm7Model/SerializableLine.cs | 4 +- src/Tm7.Cli/Tm7Model/SerializableModelData.cs | 30 ++- src/Tm7.Cli/Tm7Model/SerializableThreat.cs | 25 +- src/Tm7.Cli/Tm7XmlSerializer.cs | 30 +++ src/Tm7.Cli/TrimmerRoots.xml | 24 ++ tests/Tm7.Tests/AotExeIntegrationTests.cs | 168 ++++++++++++ tests/Tm7.Tests/PopulatedDictionaryTests.cs | 254 ++++++++++++++++++ tests/Tm7.Tests/RoundtripTests.cs | 35 ++- 13 files changed, 676 insertions(+), 20 deletions(-) create mode 100644 src/Tm7.Cli/Tm7Model/SerializableKvpTypes.cs create mode 100644 tests/Tm7.Tests/AotExeIntegrationTests.cs create mode 100644 tests/Tm7.Tests/PopulatedDictionaryTests.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7ab1664..de3f58c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,5 +33,8 @@ jobs: - name: Build run: dotnet build --no-restore --configuration Release + - name: Publish AOT + run: dotnet publish src/Tm7.Cli/Tm7.Cli.csproj --configuration Release --no-restore + - name: Test run: dotnet test --no-build --configuration Release --verbosity normal diff --git a/src/Tm7.Cli/Tm7.Cli.csproj b/src/Tm7.Cli/Tm7.Cli.csproj index b5df7f5..ab3a13c 100644 --- a/src/Tm7.Cli/Tm7.Cli.csproj +++ b/src/Tm7.Cli/Tm7.Cli.csproj @@ -17,6 +17,12 @@ so non-nullable properties may be uninitialized during deserialization. These suppressions apply to the Tm7Model serialization classes. --> CS8618;CS8625;CS8600;CS8603 + + $(WarningsNotAsErrors);IL3050;IL3053 gholliday TM7 CLI gholliday diff --git a/src/Tm7.Cli/Tm7Model/SerializableDrawingSurfaceModel.cs b/src/Tm7.Cli/Tm7Model/SerializableDrawingSurfaceModel.cs index 5ea3184..5054070 100644 --- a/src/Tm7.Cli/Tm7Model/SerializableDrawingSurfaceModel.cs +++ b/src/Tm7.Cli/Tm7Model/SerializableDrawingSurfaceModel.cs @@ -5,16 +5,28 @@ namespace Tm7.Cli.Model; [DataContract(Name = "DrawingSurfaceModel", IsReference = true, Namespace = "http://schemas.datacontract.org/2004/07/ThreatModeling.Model")] public class SerializableDrawingSurfaceModel : SerializableTaggable { + // The public API stays as Dictionary so consumers (renderer, commands, tests) + // are unaffected. DCS serialization is redirected to private list fields via + // [IgnoreDataMember] + [DataMember] + [OnSerializing]/[OnDeserialized] callbacks. + // This is required because DCS's reflection-based reader on NativeAOT cannot + // instantiate the internal System.Runtime.Serialization.KeyValue + // closed generic that backs CollectionDataContract for IDictionary<,>. + [IgnoreDataMember] + public Dictionary Borders { get; set; } = new(); + [DataMember(Name = "Borders")] - public Dictionary Borders { get; private set; } + private SerializableGuidObjectKvpList _bordersList = new(); [DataMember(Name = "Header")] public string Header { get; private set; } + [IgnoreDataMember] + public Dictionary Lines { get; set; } = new(); + [DataMember(Name = "Lines")] - public Dictionary Lines { get; private set; } + private SerializableGuidObjectKvpList _linesList = new(); - [DataMember(Name = "Zoom", EmitDefaultValue = false)] + [DataMember(Name = "Zoom")] public double Zoom { get; private set; } public SerializableDrawingSurfaceModel(Guid guid, string typeId, string genericTypeId, @@ -29,4 +41,28 @@ public SerializableDrawingSurfaceModel(Guid guid, string typeId, string genericT Zoom = zoom; Header = header; } + + [OnSerializing] + private void OnSerializingDictionaries(StreamingContext context) + { + _bordersList = Borders is null + ? new SerializableGuidObjectKvpList() + : new SerializableGuidObjectKvpList( + Borders.Select(kvp => new SerializableGuidObjectKvp { Key = kvp.Key, Value = kvp.Value })); + _linesList = Lines is null + ? new SerializableGuidObjectKvpList() + : new SerializableGuidObjectKvpList( + Lines.Select(kvp => new SerializableGuidObjectKvp { Key = kvp.Key, Value = kvp.Value })); + } + + [OnDeserialized] + private void OnDeserializedDictionaries(StreamingContext context) + { + Borders = _bordersList is null + ? new Dictionary() + : _bordersList.Where(e => e.Value is not null).ToDictionary(e => e.Key, e => e.Value!); + Lines = _linesList is null + ? new Dictionary() + : _linesList.Where(e => e.Value is not null).ToDictionary(e => e.Key, e => e.Value!); + } } diff --git a/src/Tm7.Cli/Tm7Model/SerializableElementType.cs b/src/Tm7.Cli/Tm7Model/SerializableElementType.cs index 429ac55..3328125 100644 --- a/src/Tm7.Cli/Tm7Model/SerializableElementType.cs +++ b/src/Tm7.Cli/Tm7Model/SerializableElementType.cs @@ -47,10 +47,10 @@ public class SerializableElementType : SerializableExtendable [DataMember(Name = "StencilConstraints")] public List StencilConstraints { get; private set; } - [DataMember(Name = "StrokeDashArray", EmitDefaultValue = false)] + [DataMember(Name = "StrokeDashArray")] public string StrokeDashArray { get; private set; } - [DataMember(Name = "StrokeThickness", EmitDefaultValue = false)] + [DataMember(Name = "StrokeThickness")] public double StrokeThickness { get; private set; } public SerializableElementType(bool isExtendable, string name, string id, string description, diff --git a/src/Tm7.Cli/Tm7Model/SerializableKvpTypes.cs b/src/Tm7.Cli/Tm7Model/SerializableKvpTypes.cs new file mode 100644 index 0000000..cd39a6c --- /dev/null +++ b/src/Tm7.Cli/Tm7Model/SerializableKvpTypes.cs @@ -0,0 +1,71 @@ +using System.Runtime.Serialization; + +namespace Tm7.Cli.Model; + +// User-defined key/value pair types that replace DataContractSerializer's internal +// System.Runtime.Serialization.KeyValue when serializing the dictionary-shaped +// members of the threat model. The internal KeyValue<,> type cannot be statically +// instantiated for arbitrary closed generics under NativeAOT, which causes +// "missing native code or metadata" failures at runtime. By substituting our own +// item types we keep the wire format identical while making the type graph +// fully analyzable by the AOT compiler. + +[DataContract(Name = "KeyValueOfguidanyType", + Namespace = "http://schemas.microsoft.com/2003/10/Serialization/Arrays")] +public class SerializableGuidObjectKvp +{ + [DataMember(Name = "Key", Order = 0)] + public Guid Key { get; set; } + + [DataMember(Name = "Value", Order = 1)] + public object? Value { get; set; } +} + +[CollectionDataContract(Name = "ArrayOfKeyValueOfguidanyType", + Namespace = "http://schemas.microsoft.com/2003/10/Serialization/Arrays", + ItemName = "KeyValueOfguidanyType")] +public class SerializableGuidObjectKvpList : List +{ + public SerializableGuidObjectKvpList() { } + public SerializableGuidObjectKvpList(IEnumerable items) : base(items) { } +} + +[DataContract(Name = "KeyValueOfstringThreatpc_P0_PhOB", + Namespace = "http://schemas.microsoft.com/2003/10/Serialization/Arrays")] +public class SerializableStringThreatKvp +{ + [DataMember(Name = "Key", Order = 0)] + public string? Key { get; set; } + + [DataMember(Name = "Value", Order = 1)] + public SerializableThreat? Value { get; set; } +} + +[CollectionDataContract(Name = "ArrayOfKeyValueOfstringThreatpc_P0_PhOB", + Namespace = "http://schemas.microsoft.com/2003/10/Serialization/Arrays", + ItemName = "KeyValueOfstringThreatpc_P0_PhOB")] +public class SerializableStringThreatKvpList : List +{ + public SerializableStringThreatKvpList() { } + public SerializableStringThreatKvpList(IEnumerable items) : base(items) { } +} + +[DataContract(Name = "KeyValueOfstringstring", + Namespace = "http://schemas.microsoft.com/2003/10/Serialization/Arrays")] +public class SerializableStringStringKvp +{ + [DataMember(Name = "Key", Order = 0)] + public string? Key { get; set; } + + [DataMember(Name = "Value", Order = 1)] + public string? Value { get; set; } +} + +[CollectionDataContract(Name = "ArrayOfKeyValueOfstringstring", + Namespace = "http://schemas.microsoft.com/2003/10/Serialization/Arrays", + ItemName = "KeyValueOfstringstring")] +public class SerializableStringStringKvpList : List +{ + public SerializableStringStringKvpList() { } + public SerializableStringStringKvpList(IEnumerable items) : base(items) { } +} diff --git a/src/Tm7.Cli/Tm7Model/SerializableLine.cs b/src/Tm7.Cli/Tm7Model/SerializableLine.cs index 1a7bc8e..b8ff208 100644 --- a/src/Tm7.Cli/Tm7Model/SerializableLine.cs +++ b/src/Tm7.Cli/Tm7Model/SerializableLine.cs @@ -26,10 +26,10 @@ public abstract class SerializableLine : SerializableTaggable [DataMember(Name = "SourceY")] public int Y0 { get; private set; } - [DataMember(Name = "StrokeDashArray", EmitDefaultValue = false)] + [DataMember(Name = "StrokeDashArray")] public string StrokeDashArray { get; private set; } - [DataMember(Name = "StrokeThickness", EmitDefaultValue = false)] + [DataMember(Name = "StrokeThickness")] public double StrokeThickness { get; private set; } [DataMember(Name = "TargetGuid")] diff --git a/src/Tm7.Cli/Tm7Model/SerializableModelData.cs b/src/Tm7.Cli/Tm7Model/SerializableModelData.cs index 6747f95..3112ffa 100644 --- a/src/Tm7.Cli/Tm7Model/SerializableModelData.cs +++ b/src/Tm7.Cli/Tm7Model/SerializableModelData.cs @@ -14,11 +14,16 @@ public class SerializableModelData [DataMember(Name = "Notes", Order = 4)] public List Notes { get; private set; } + // The public API keeps Dictionary; DCS sees the + // private backing list instead so AOT can fully analyze the type graph. + [IgnoreDataMember] + public Dictionary AllThreatsDictionary { get; set; } + [DataMember(Name = "ThreatInstances", Order = 5)] - public Dictionary AllThreatsDictionary { get; private set; } + private SerializableStringThreatKvpList _threatInstancesList = new(); [DataMember(Name = "ThreatGenerationEnabled", Order = 6)] - public bool? ThreatGenerationEnabled { get; private set; } + public bool ThreatGenerationEnabled { get; private set; } [DataMember(Name = "Validations", Order = 7)] public List Validations { get; private set; } @@ -47,11 +52,30 @@ public SerializableModelData( MetaInformation = metaInformation; Notes = notes.ToList(); AllThreatsDictionary = allThreatsDictionary; - ThreatGenerationEnabled = threatGenerationEnabled; + ThreatGenerationEnabled = threatGenerationEnabled ?? false; Validations = validations.ToList(); Version = version ?? "4.3"; KnowledgeBase = knowledgeBase; Profile = profile; } + [OnSerializing] + private void OnSerializingDictionaries(StreamingContext context) + { + _threatInstancesList = AllThreatsDictionary is null + ? new SerializableStringThreatKvpList() + : new SerializableStringThreatKvpList( + AllThreatsDictionary.Select(kvp => new SerializableStringThreatKvp { Key = kvp.Key, Value = kvp.Value })); + } + + [OnDeserialized] + private void OnDeserializedDictionaries(StreamingContext context) + { + AllThreatsDictionary = _threatInstancesList is null + ? new Dictionary() + : _threatInstancesList + .Where(e => e.Key is not null && e.Value is not null) + .ToDictionary(e => e.Key!, e => e.Value!); + } + } diff --git a/src/Tm7.Cli/Tm7Model/SerializableThreat.cs b/src/Tm7.Cli/Tm7Model/SerializableThreat.cs index 409613c..446f2b5 100644 --- a/src/Tm7.Cli/Tm7Model/SerializableThreat.cs +++ b/src/Tm7.Cli/Tm7Model/SerializableThreat.cs @@ -29,9 +29,13 @@ public class SerializableThreat [DataMember(Name = "Priority")] public string Priority { get; private set; } - [DataMember(Name = "Properties")] + // Public Dictionary preserved; DCS-visible backing list bypasses internal KeyValue<,>. + [IgnoreDataMember] public Dictionary Properties { get; set; } + [DataMember(Name = "Properties")] + private SerializableStringStringKvpList _propertiesList = new(); + [DataMember(Name = "SourceGuid")] public Guid SourceGuid { get; private set; } @@ -85,4 +89,23 @@ public SerializableThreat(int id, string typeId, Guid sourceGuid, Guid targetGui Upgraded = upgraded; Properties = properties; } + + [OnSerializing] + private void OnSerializingDictionaries(StreamingContext context) + { + _propertiesList = Properties is null + ? new SerializableStringStringKvpList() + : new SerializableStringStringKvpList( + Properties.Select(kvp => new SerializableStringStringKvp { Key = kvp.Key, Value = kvp.Value })); + } + + [OnDeserialized] + private void OnDeserializedDictionaries(StreamingContext context) + { + Properties = _propertiesList is null + ? new Dictionary() + : _propertiesList + .Where(e => e.Key is not null && e.Value is not null) + .ToDictionary(e => e.Key!, e => e.Value!); + } } diff --git a/src/Tm7.Cli/Tm7XmlSerializer.cs b/src/Tm7.Cli/Tm7XmlSerializer.cs index f8c6b1f..013eb8a 100644 --- a/src/Tm7.Cli/Tm7XmlSerializer.cs +++ b/src/Tm7.Cli/Tm7XmlSerializer.cs @@ -13,6 +13,17 @@ namespace Tm7.Cli; /// public static class Tm7XmlSerializer { + // Force AOT to keep concrete instantiations of Dictionary<,> and its EqualityComparer<> + // dependencies that DataContractSerializer's reflection reader will demand via ISerializable. + private static readonly object?[] _aotKeepAlive = + [ + EqualityComparer.Default, + EqualityComparer.Default, + new Dictionary(), + new Dictionary(), + new Dictionary(), + ]; + private static readonly Type[] KnownTypes = [ typeof(List), @@ -94,6 +105,25 @@ public static class Tm7XmlSerializer [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(SerializableAttributeValues))] [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(SerializableAvailableToBaseModels))] [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(SerializableKbVersion))] + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(SerializableGuidObjectKvp))] + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(SerializableGuidObjectKvpList))] + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(SerializableStringThreatKvp))] + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(SerializableStringThreatKvpList))] + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(SerializableStringStringKvp))] + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(SerializableStringStringKvpList))] + // The serializable model classes no longer expose Dictionary<,> as [DataMember]; + // those properties are [IgnoreDataMember] and backed by [DataMember] List fields + // (see SerializableKvpTypes.cs) so DCS never instantiates the BCL-internal + // System.Runtime.Serialization.KeyValue closed generics that AOT cannot generate + // code for. The Dictionary<,> / EqualityComparer<,> preservation below is defensive: + // if a future change re-introduces a [DataMember] Dictionary<,>, DCS will fall back + // to the ISerializable path and require Dictionary<,>.(SerializationInfo, + // StreamingContext) + the relevant EqualityComparer to exist at runtime. + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(Dictionary))] + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(Dictionary))] + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(Dictionary))] + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(EqualityComparer))] + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(EqualityComparer))] private static DataContractSerializer CreateSerializer() { return new DataContractSerializer(typeof(SerializableModelData), KnownTypes); diff --git a/src/Tm7.Cli/TrimmerRoots.xml b/src/Tm7.Cli/TrimmerRoots.xml index 557d0c8..56cec7f 100644 --- a/src/Tm7.Cli/TrimmerRoots.xml +++ b/src/Tm7.Cli/TrimmerRoots.xml @@ -61,6 +61,14 @@ + + + + + + + + @@ -69,4 +77,20 @@ + + + + + + + + + + diff --git a/tests/Tm7.Tests/AotExeIntegrationTests.cs b/tests/Tm7.Tests/AotExeIntegrationTests.cs new file mode 100644 index 0000000..1ca529a --- /dev/null +++ b/tests/Tm7.Tests/AotExeIntegrationTests.cs @@ -0,0 +1,168 @@ +using System.Diagnostics; +using System.Runtime.InteropServices; +using Tm7.Cli.Model; +using Tm7.Cli; +using Xunit; + +namespace Tm7.Tests; + +/// +/// Integration tests that invoke the published NativeAOT executable to confirm that the +/// AOT round-trip works for cases the in-process JIT tests cannot reach (the JIT path +/// uses code-gen, the AOT path uses the reflection-based writer/reader). These tests +/// are skipped when the AOT publish output is not present. +/// +public class AotExeIntegrationTests +{ + private static string RepoRoot() + { + // bin/Tm7.Tests/debug/ -> up 4 = repo root + return Path.GetFullPath(Path.Combine(AppContext.BaseDirectory, "..", "..", "..", "..")); + } + + private static string SamplePath(string name) => Path.Combine(RepoRoot(), "samples", name); + + private static string? AotExePath() + { + var exeName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "tm7.exe" : "tm7"; + // Check both publish layouts: + // release/ -> `dotnet publish -c Release` (no explicit RID; the + // CI workflow uses this layout). + // release_/ -> `dotnet publish -c Release -r `. + var candidates = new[] + { + Path.Combine(RepoRoot(), "artifacts", "publish", "Tm7.Cli", "release", exeName), + Path.Combine(RepoRoot(), "artifacts", "publish", "Tm7.Cli", $"release_{RuntimeInformation.RuntimeIdentifier}", exeName), + }; + return candidates.FirstOrDefault(File.Exists); + } + + private static (int ExitCode, string Stdout, string Stderr) Run(string exe, params string[] args) + { + var psi = new ProcessStartInfo + { + FileName = exe, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + CreateNoWindow = true, + WorkingDirectory = RepoRoot(), + }; + foreach (var a in args) psi.ArgumentList.Add(a); + using var p = Process.Start(psi)!; + var stdout = p.StandardOutput.ReadToEnd(); + var stderr = p.StandardError.ReadToEnd(); + p.WaitForExit(60_000); + return (p.ExitCode, stdout, stderr); + } + + private static SerializableThreat MakeThreat(int id) + { + return new SerializableThreat( + id: id, + typeId: "ThreatType.GenericInformation", + sourceGuid: Guid.NewGuid(), + targetGuid: Guid.NewGuid(), + flowGuid: Guid.NewGuid(), + drawingSurfaceGuid: Guid.NewGuid(), + state: ThreatState.NotApplicable, + interactionKey: "k-" + id, + priority: "High", + wide: false, + changedBy: "tester", + modifiedAt: DateTime.UtcNow, + upgraded: false, + properties: new Dictionary + { + ["Title"] = "T" + id, + ["Description"] = "D" + id, + }); + } + + [Fact] + public void AotExe_Open_LoadsTemplate() + { + var exe = AotExePath(); + Assert.SkipUnless(exe is not null, "AOT publish output not present; run `dotnet publish src/Tm7.Cli -c Release -r win-x64` first."); + + var (code, stdout, stderr) = Run(exe!, "open", SamplePath("template.tm7")); + Assert.True(code == 0, $"open exit={code} stderr={stderr}"); + Assert.Contains("Diagram 1", stdout); + } + + [Fact] + public void AotExe_AddEntity_RoundTripsAndPreservesExistingEntities() + { + var exe = AotExePath(); + Assert.SkipUnless(exe is not null, "AOT publish output not present; run `dotnet publish src/Tm7.Cli -c Release -r win-x64` first."); + + var work = Path.Combine(Path.GetTempPath(), $"tm7-aot-{Guid.NewGuid():N}.tm7"); + File.Copy(SamplePath("template.tm7"), work, overwrite: true); + try + { + var (code, _, stderr) = Run(exe!, + "add", "entity", work, + "--name", "AotAdded", + "--type-id", "StencilEllipse", + "--generic-type-id", "GE.P", + "--left", "10", "--top", "10"); + Assert.True(code == 0, $"add exit={code} stderr={stderr}"); + + // Reload via JIT and assert structural preservation: original 17 borders + 1 new. + var reloaded = Tm7File.Load(work); + Assert.Equal(18, reloaded.DrawingSurfaceList[0].Borders.Count); + } + finally + { + File.Delete(work); + } + } + + [Fact] + public void AotExe_PopulatedThreatsRoundTripThroughAddCommand() + { + // Most important AOT regression guard: JIT-produce a model with populated + // AllThreatsDictionary + SerializableThreat.Properties, run the AOT exe to + // load+modify+save it, then JIT-reload and verify the threats and properties + // were preserved through the AOT write path (which uses the reflection writer + // we had to refactor for AOT compatibility). + var exe = AotExePath(); + Assert.SkipUnless(exe is not null, "AOT publish output not present; run `dotnet publish src/Tm7.Cli -c Release -r win-x64` first."); + + var work = Path.Combine(Path.GetTempPath(), $"tm7-aot-threats-{Guid.NewGuid():N}.tm7"); + var model = Tm7File.Load(SamplePath("template.tm7")); + model.AllThreatsDictionary.Add("t1", MakeThreat(1)); + model.AllThreatsDictionary.Add("t2", MakeThreat(2)); + Tm7File.Save(model, work); + + try + { + // Sanity: JIT-written file is readable by JIT. + var preCheck = Tm7File.Load(work); + Assert.Equal(2, preCheck.AllThreatsDictionary.Count); + + // Run the AOT exe to load + modify + save the file. + var (code, _, stderr) = Run(exe!, + "add", "entity", work, + "--name", "AotProbe", + "--type-id", "StencilEllipse", + "--generic-type-id", "GE.P", + "--left", "5", "--top", "5"); + Assert.True(code == 0, $"AOT add failed exit={code} stderr={stderr}"); + + // Reload via JIT and confirm AOT preserved the threats and their properties. + var after = Tm7File.Load(work); + Assert.Equal(2, after.AllThreatsDictionary.Count); + Assert.True(after.AllThreatsDictionary.ContainsKey("t1")); + Assert.True(after.AllThreatsDictionary.ContainsKey("t2")); + var t1 = after.AllThreatsDictionary["t1"]; + Assert.Equal(1, t1.Id); + Assert.Equal("T1", t1.Properties["Title"]); + Assert.Equal("D1", t1.Properties["Description"]); + } + finally + { + File.Delete(work); + } + } +} diff --git a/tests/Tm7.Tests/PopulatedDictionaryTests.cs b/tests/Tm7.Tests/PopulatedDictionaryTests.cs new file mode 100644 index 0000000..48c8605 --- /dev/null +++ b/tests/Tm7.Tests/PopulatedDictionaryTests.cs @@ -0,0 +1,254 @@ +using Tm7.Cli.Model; +using Tm7.Cli; +using System.Runtime.Serialization; +using System.Xml; +using Xunit; + +namespace Tm7.Tests; + +public class PopulatedDictionaryTests +{ + private static string GetSamplePath(string filename) + { + var path = Path.Combine(AppContext.BaseDirectory, "..", "..", "..", "..", "samples", filename); + return Path.GetFullPath(path); + } + + private static SerializableModelData LoadTemplate() => Tm7File.Load(GetSamplePath("template.tm7")); + + private static SerializableThreat MakeThreat(int id, string title) + { + return new SerializableThreat( + id: id, + typeId: "ThreatType.GenericInformation", + sourceGuid: Guid.NewGuid(), + targetGuid: Guid.NewGuid(), + flowGuid: Guid.NewGuid(), + drawingSurfaceGuid: Guid.NewGuid(), + state: ThreatState.NotApplicable, + interactionKey: "key-" + id, + priority: "High", + wide: false, + changedBy: "tester", + modifiedAt: DateTime.UtcNow, + upgraded: false, + properties: new Dictionary + { + ["Title"] = title, + ["Description"] = "Threat description " + id, + ["Mitigation"] = "Mitigation " + id, + }); + } + + [Fact] + public void Roundtrip_PopulatedThreatInstances_PreservesEntries() + { + var model = LoadTemplate(); + var t1 = MakeThreat(1, "Threat one"); + var t2 = MakeThreat(2, "Threat two"); + model.AllThreatsDictionary.Add("threat-1", t1); + model.AllThreatsDictionary.Add("threat-2", t2); + + using var ms = new MemoryStream(); + Tm7XmlSerializer.Serialize(ms, model); + ms.Position = 0; + var reloaded = Tm7XmlSerializer.Deserialize(ms); + + Assert.Equal(2, reloaded.AllThreatsDictionary.Count); + Assert.True(reloaded.AllThreatsDictionary.ContainsKey("threat-1")); + Assert.True(reloaded.AllThreatsDictionary.ContainsKey("threat-2")); + + var r1 = reloaded.AllThreatsDictionary["threat-1"]; + Assert.Equal(1, r1.Id); + Assert.Equal("key-1", r1.InteractionKey); + Assert.Equal("High", r1.Priority); + Assert.Equal(ThreatState.NotApplicable, r1.State); + Assert.Equal("Threat one", r1.Properties["Title"]); + Assert.Equal("Threat description 1", r1.Properties["Description"]); + Assert.Equal("Mitigation 1", r1.Properties["Mitigation"]); + + var r2 = reloaded.AllThreatsDictionary["threat-2"]; + Assert.Equal(2, r2.Id); + Assert.Equal(3, r2.Properties.Count); + } + + [Fact] + public void Roundtrip_PopulatedThreatProperties_PreservesEntries() + { + var model = LoadTemplate(); + var t = MakeThreat(42, "Detailed threat"); + t.Properties["ExtraKey"] = "ExtraValue"; + t.Properties["Empty"] = ""; + model.AllThreatsDictionary.Add("only", t); + + using var ms = new MemoryStream(); + Tm7XmlSerializer.Serialize(ms, model); + ms.Position = 0; + var reloaded = Tm7XmlSerializer.Deserialize(ms); + + var rt = reloaded.AllThreatsDictionary["only"]; + Assert.Equal(5, rt.Properties.Count); + Assert.Equal("ExtraValue", rt.Properties["ExtraKey"]); + Assert.Equal("", rt.Properties["Empty"]); + } + + [Fact] + public void Roundtrip_PopulatedThreats_WireFormatMatchesAutoDictionary() + { + // Verify our custom KVP wrapper produces the same element-name shape as the + // built-in Dictionary would under JIT-DCS. If this ever + // diverges, files written by tm7-cli would be unreadable by the original TMT + // tooling and by any previous build of tm7-cli that used the raw Dictionary. + var referenceModel = new ReferenceModelWithRawDictionary + { + ThreatInstances = new Dictionary + { + ["k"] = MakeThreat(7, "Reference"), + }, + }; + var serializer = new DataContractSerializer( + typeof(ReferenceModelWithRawDictionary), + new[] { typeof(SerializableThreat) }); + using var refMs = new MemoryStream(); + serializer.WriteObject(refMs, referenceModel); + var refXml = System.Text.Encoding.UTF8.GetString(refMs.ToArray()); + + var ourModel = LoadTemplate(); + ourModel.AllThreatsDictionary.Clear(); + ourModel.AllThreatsDictionary.Add("k", MakeThreat(7, "Reference")); + using var ourMs = new MemoryStream(); + Tm7XmlSerializer.Serialize(ourMs, ourModel); + var ourXml = System.Text.Encoding.UTF8.GetString(ourMs.ToArray()); + + var refItemName = ExtractKvpItemNameInside(refXml, "ThreatInstances"); + var ourItemName = ExtractKvpItemNameInside(ourXml, "ThreatInstances"); + Assert.Equal(refItemName, ourItemName); + } + + [Fact] + public void Roundtrip_PopulatedThreatProperties_WireFormatMatchesAutoDictionary() + { + // Same wire-format check for Dictionary on SerializableThreat.Properties. + var serializer = new DataContractSerializer(typeof(Dictionary)); + using var refMs = new MemoryStream(); + serializer.WriteObject(refMs, new Dictionary { ["k"] = "v" }); + var refXml = System.Text.Encoding.UTF8.GetString(refMs.ToArray()); + var refItemName = ExtractFirstKvpItemName(refXml); + + var ourModel = LoadTemplate(); + ourModel.AllThreatsDictionary.Add("only", MakeThreat(1, "T")); + using var ourMs = new MemoryStream(); + Tm7XmlSerializer.Serialize(ourMs, ourModel); + var ourXml = System.Text.Encoding.UTF8.GetString(ourMs.ToArray()); + + // The string/string KVP element name appears verbatim inside the threat's Properties. + // Multiple elements exist (DisplayAttribute lists on Taggables, etc.), + // but only the string/string dictionary contributes the KeyValueOfstringstring name. + Assert.Contains(refItemName, ourXml); + } + + [Fact] + public void Roundtrip_Borders_WireFormatMatchesAutoDictionary() + { + // Same wire-format check for Dictionary on Borders/Lines. + var serializer = new DataContractSerializer(typeof(Dictionary)); + using var refMs = new MemoryStream(); + serializer.WriteObject(refMs, new Dictionary { [Guid.NewGuid()] = "x" }); + var refXml = System.Text.Encoding.UTF8.GetString(refMs.ToArray()); + + var ourModel = LoadTemplate(); + using var ourMs = new MemoryStream(); + Tm7XmlSerializer.Serialize(ourMs, ourModel); + var ourXml = System.Text.Encoding.UTF8.GetString(ourMs.ToArray()); + + var refItemName = ExtractFirstKvpItemName(refXml); + var ourItemName = ExtractKvpItemNameInside(ourXml, "Borders"); + Assert.Equal(refItemName, ourItemName); + } + + private static string ExtractKvpItemNameInside(string xml, string parentElement) + { + var openIdx = xml.IndexOf("<" + parentElement, StringComparison.Ordinal); + Assert.True(openIdx >= 0, $"Parent element <{parentElement}> not found"); + var closeIdx = xml.IndexOf("", openIdx, StringComparison.Ordinal); + // Empty self-closing element has no children; require a populated case. + Assert.True(closeIdx >= 0, $"Parent element <{parentElement}> is empty/self-closing"); + var slice = xml.Substring(openIdx, closeIdx - openIdx); + return ExtractFirstKvpItemName(slice); + } + + private static string ExtractFirstKvpItemName(string xml) + { + const string marker = "KeyValueOf"; + var idx = xml.IndexOf(marker, StringComparison.Ordinal); + Assert.True(idx >= 0, "No KVP item element found"); + var endIdx = xml.IndexOfAny(new[] { '>', ' ', '/' }, idx); + return xml.Substring(idx, endIdx - idx); + } + + [Fact] + public void Roundtrip_MultipleAddRemoveCycles() + { + var model = LoadTemplate(); + var surface = model.DrawingSurfaceList[0]; + var originalBorderCount = surface.Borders.Count; + + var newGuids = new List(); + for (int i = 0; i < 5; i++) + { + var g = Guid.NewGuid(); + newGuids.Add(g); + surface.Borders.Add(g, new SerializableStencilEllipse( + guid: g, typeId: "StencilEllipse", genericTypeId: "GE.P", + properties: new List + { + new SerializableStringDisplayAttribute("Name", "Name", $"Proc {i}") + }, + x: 10 * i, y: 10 * i, width: 100, height: 50, + strokeThickness: 1.0, strokeDashArray: "")); + } + + using var ms1 = new MemoryStream(); + Tm7XmlSerializer.Serialize(ms1, model); + ms1.Position = 0; + var reloaded = Tm7XmlSerializer.Deserialize(ms1); + Assert.Equal(originalBorderCount + 5, reloaded.DrawingSurfaceList[0].Borders.Count); + + // Remove 3 and save again + for (int i = 0; i < 3; i++) + reloaded.DrawingSurfaceList[0].Borders.Remove(newGuids[i]); + + using var ms2 = new MemoryStream(); + Tm7XmlSerializer.Serialize(ms2, reloaded); + ms2.Position = 0; + var reloaded2 = Tm7XmlSerializer.Deserialize(ms2); + Assert.Equal(originalBorderCount + 2, reloaded2.DrawingSurfaceList[0].Borders.Count); + } + + [Fact] + public void Serialize_NullDictionaries_DoesNotThrow() + { + // Public setters allow null on Borders/Lines/AllThreatsDictionary; serialization + // must not crash if a caller has nulled them out. + var model = LoadTemplate(); + var surface = model.DrawingSurfaceList[0]; + surface.Borders = null!; + surface.Lines = null!; + model.AllThreatsDictionary = null!; + + using var ms = new MemoryStream(); + var ex = Record.Exception(() => Tm7XmlSerializer.Serialize(ms, model)); + Assert.Null(ex); + } + + // Reference model class used for wire-format validation. Mirrors SerializableModelData + // but uses the raw Dictionary for the threat-instances + // member, so DCS auto-generates its own item element name. The test compares that + // name against the one our wrapper class emits. + [DataContract(Name = "ThreatModel", Namespace = "http://schemas.datacontract.org/2004/07/ThreatModeling.Model")] + public class ReferenceModelWithRawDictionary + { + [DataMember(Name = "ThreatInstances", Order = 5)] + public Dictionary ThreatInstances { get; set; } = new(); + } +} diff --git a/tests/Tm7.Tests/RoundtripTests.cs b/tests/Tm7.Tests/RoundtripTests.cs index d9cda0e..9665dc2 100644 --- a/tests/Tm7.Tests/RoundtripTests.cs +++ b/tests/Tm7.Tests/RoundtripTests.cs @@ -18,20 +18,37 @@ private static SerializableModelData LoadTemplate() => Tm7File.Load(GetSamplePath("template.tm7")); [Fact] - public void RoundTrip_ByteForByte() + public void RoundTrip_StructuralEquivalence() { var path = GetSamplePath("template.tm7"); - var originalBytes = File.ReadAllBytes(path); - var model = Tm7File.Load(path); + var original = Tm7File.Load(path); using var ms = new MemoryStream(); - Tm7XmlSerializer.Serialize(ms, model); - var roundTrippedBytes = ms.ToArray(); - - Assert.Equal(originalBytes.Length, roundTrippedBytes.Length); - Assert.True(originalBytes.AsSpan().SequenceEqual(roundTrippedBytes), - "Round-tripped bytes do not match the original file"); + Tm7XmlSerializer.Serialize(ms, original); + ms.Position = 0; + var roundTripped = Tm7XmlSerializer.Deserialize(ms); + + // EmitDefaultValue=false was removed for NativeAOT compatibility (the underlying + // XmlObjectSerializerWriteContext.GetDefaultValue() requires MakeGenericMethod), + // so the wire bytes are no longer identical. Verify the model structure round-trips. + Assert.Equal(original.Version, roundTripped.Version); + Assert.Equal(original.DrawingSurfaceList.Count, roundTripped.DrawingSurfaceList.Count); + for (int i = 0; i < original.DrawingSurfaceList.Count; i++) + { + var a = original.DrawingSurfaceList[i]; + var b = roundTripped.DrawingSurfaceList[i]; + Assert.Equal(a.Header, b.Header); + Assert.Equal(a.Borders.Count, b.Borders.Count); + Assert.Equal(a.Lines.Count, b.Lines.Count); + foreach (var key in a.Borders.Keys) + Assert.True(b.Borders.ContainsKey(key), $"Missing border {key} in surface {i}"); + foreach (var key in a.Lines.Keys) + Assert.True(b.Lines.ContainsKey(key), $"Missing line {key} in surface {i}"); + } + Assert.Equal(original.AllThreatsDictionary.Count, roundTripped.AllThreatsDictionary.Count); + Assert.Equal(original.MetaInformation.ThreatModelName, roundTripped.MetaInformation.ThreatModelName); + Assert.Equal(original.KnowledgeBase.Manifest.Name, roundTripped.KnowledgeBase.Manifest.Name); } [Fact]