From a824822b1e0606e15b7d591d6e2e5b9570221a8f Mon Sep 17 00:00:00 2001 From: David Hudec Date: Tue, 1 Sep 2026 15:15:15 +0200 Subject: [PATCH 01/10] fix(data): keep every N:1 relationship between the same table pair A table with several lookups to one target rendered a single edge, while all its lookup columns still appeared - understating the model without looking broken. The duplicate guard keyed on (LeftSideTable, RighSideTable), ignoring which column the relationship ran through. The key now includes LeftSideRow. Genuine duplicates still collapse, which is what the guard is for. Co-Authored-By: Claude Opus 5 --- .../DataModelConverterService.cs | 2 +- .../DataModelConverterServiceTests.cs | 84 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs index 36a1d17c..afe27440 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs @@ -489,7 +489,7 @@ public static List ParseRelationships(List modules, List x.RowType == RowType.Primarykey)); - if (EntityRelationships.FirstOrDefault(x => x.LeftSideTable == entityRelationship.LeftSideTable && x.RighSideTable == entityRelationship.RighSideTable) == default) + if (EntityRelationships.FirstOrDefault(x => x.LeftSideTable == entityRelationship.LeftSideTable && x.LeftSideRow == entityRelationship.LeftSideRow && x.RighSideTable == entityRelationship.RighSideTable) == default) { EntityRelationships.Add(entityRelationship); } diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs new file mode 100644 index 00000000..fc3050c8 --- /dev/null +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs @@ -0,0 +1,84 @@ +using System.IO; +using System.Linq; +using System.Reflection; +using DotMake.CommandLine; +using TALXIS.CLI.Features.Data; +using System.Xml.Linq; +using TALXIS.CLI.Features.Data.DataModelConverter; +using Model = TALXIS.CLI.Features.Data.DataModelConverter.Model; +using Xunit; + +namespace TALXIS.CLI.Tests.Data.DataModelConverter; + +/// +/// Regression tests for four defects that made the converter silently lose or corrupt +/// model content. Each was found by diffing converter output against the source +/// declarations of real solutions; each test fails against the unfixed converter. +/// +public class DataModelConverterServiceTests +{ + private static XElement Entity(string logicalName, params string[] attributes) + { + var attrXml = string.Join("", attributes); + return XElement.Parse($""" + + {logicalName} + + + + primarykey + {attrXml} + + + + + """); + } + + private static string Attr(string name, string type) => + $"""{type}"""; + + private static string Lookup(string name) => Attr(name, "lookup"); + + private static XElement OneToMany(string name, string child, string childAttr, string parent) => + XElement.Parse($""" + + OneToMany + {child} + {parent} + {childAttr} + + """); + + private static Model.Module ModuleWith(XElement[] entities, XElement[]? relationships = null) + { + var module = new Model.Module(); + module.entities.AddRange(entities); + if (relationships != null) module.relationships.AddRange(relationships); + return module; + } + + // ---- Defect 1: relationships deduped on the table pair, not the column ------------- + + [Fact] + public void TwoLookupsBetweenSameTablePair_BothProduceRelationships() + { + var module = ModuleWith( + [Entity("account"), Entity("contoso_project", Lookup("contoso_ownerid"), Lookup("contoso_billtoid"))], + [ + OneToMany("rel_owner", "contoso_project", "contoso_ownerid", "account"), + OneToMany("rel_billto", "contoso_project", "contoso_billtoid", "account"), + ]); + + var model = DataModelConverterService.ParseModules([module]); + + var toAccount = model.relationships + .Where(r => r.RighSideTable?.LogicalName == "account") + .Select(r => r.LeftSideRow?.Name) + .ToList(); + + Assert.Equal(2, toAccount.Count); + Assert.Contains("contoso_ownerid", toAccount); + Assert.Contains("contoso_billtoid", toAccount); + } +} From 544a229d0da24fb7e4dc04ec182cf83261504df7 Mon Sep 17 00:00:00 2001 From: David Hudec Date: Tue, 1 Sep 2026 15:15:36 +0200 Subject: [PATCH 02/10] fix(data): keep picklist columns whose option set cannot be resolved Required, form-visible columns disappeared from the output with no warning, and a module whose only contribution was such an attribute read as contributing nothing. Rows of an optionset kind were deleted outright when their OptionSetName did not resolve. Three causes seen in real solutions: the global option set declares , it is declared in a different module, or it is platform-owned. The row is kept and only OptionSetName is cleared. That is what ToDbDiagramNotation prefers over RowType, so leaving it set would reference an Enum that was never emitted; RowType is left alone so sql and edmx keep their own handling for the kind. Co-Authored-By: Claude Opus 5 --- .../DataModelConverterService.cs | 19 +++++++++------ .../DataModelConverterServiceTests.cs | 23 +++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs index afe27440..c32d871f 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs @@ -362,15 +362,20 @@ public static ParsedModel ParseModules(List modules) List EntityTables = ParseEntities(modules); List EntityOptionSets = ParseOptionSets(modules); - // Remove optionset rows without optionsets defined + // Downgrade optionset rows whose optionset is not resolvable here (declared with no + // options, owned by another module, or platform-owned) rather than dropping the column. var validOptionSetNames = EntityOptionSets.Select(x => x.LocalizedName).ToHashSet(StringComparer.OrdinalIgnoreCase); - foreach (var entity in EntityTables) + foreach (var row in EntityTables + .SelectMany(entity => entity.Rows) + .Where(row => + row.RowType is (RowType.Picklist or RowType.Multiselectoptionset or RowType.State or RowType.Status or RowType.Bit) + && !validOptionSetNames.Contains(row.OptionSetName))) { - entity.Rows = [.. entity.Rows - .Where(row => - row.RowType is not (RowType.Picklist or RowType.Multiselectoptionset or RowType.State or RowType.Status or RowType.Bit) - || validOptionSetNames.Contains(row.OptionSetName) - )]; + // Clearing OptionSetName is enough and is all that is needed: it is what + // ToDbDiagramNotation prefers over RowType, so leaving it set would make the + // column reference an Enum that was never emitted. RowType is deliberately + // left alone so each translator keeps its own handling for the kind. + row.OptionSetName = string.Empty; } // Fill in setnames where missing with placeholder logical names diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs index fc3050c8..97b345e3 100644 --- a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs @@ -81,4 +81,27 @@ public void TwoLookupsBetweenSameTablePair_BothProduceRelationships() Assert.Contains("contoso_ownerid", toAccount); Assert.Contains("contoso_billtoid", toAccount); } + + // ---- Defect 3: a column vanishes when its option set will not resolve -------------- + + [Fact] + public void PicklistWithUnresolvableOptionSet_KeepsColumnInsteadOfDroppingIt() + { + var picklist = """ + + picklist + contoso_never_declared_anywhere + + """; + var module = ModuleWith([Entity("contoso_thing", picklist)]); + + var table = DataModelConverterService.ParseModules([module]) + .tables.Single(t => t.LogicalName == "contoso_thing"); + + var row = table.Rows.SingleOrDefault(r => r.Name == "contoso_statuscode"); + Assert.NotNull(row); + // Cleared so the column cannot reference an Enum that was never emitted; + // RowType is deliberately left alone so each translator keeps its own handling. + Assert.True(string.IsNullOrEmpty(row!.OptionSetName)); + } } From e91f29429bbc07add09f6dc95918a4cabfb9f68e Mon Sep 17 00:00:00 2001 From: David Hudec Date: Tue, 1 Sep 2026 15:15:57 +0200 Subject: [PATCH 03/10] fix(data): make conversion reproducible The same unchanged solution converted to a different file on every run, at identical length - so a generated diagram could not be committed, diffed, or compared across a model change. Two causes. Module seeded Colorhex from new Random(). And three file enumerations used Directory.GetFiles, which guarantees no ordering, so table, relationship and enum order followed the filesystem. Colour now derives from the module name with FNV-1a - not string.GetHashCode, which is randomised per process on .NET Core - and all three enumerations are ordered ordinally. Everyone's colours change; nothing could have depended on the old values. Co-Authored-By: Claude Opus 5 --- .../DataModelConverterService.cs | 10 +++++--- .../DataModelConverter/Model/Module.cs | 23 +++++++++++++++---- .../DataModelConverterServiceTests.cs | 14 +++++++++++ 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs index c32d871f..f4c887c6 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs @@ -259,7 +259,11 @@ public static ParsedModel ParseModelFolder(string folderPath) Module module = new(); // Get files named Entity.xml in subfolders - var entityFiles = Directory.GetFiles(folderPath, "Entity.xml", SearchOption.AllDirectories); + // Ordered: Directory.GetFiles gives no ordering guarantee, so without this the + // table, relationship and enum order in the output varies by filesystem and the + // result cannot be committed or diffed. + var entityFiles = Directory.GetFiles(folderPath, "Entity.xml", SearchOption.AllDirectories) + .OrderBy(f => f, StringComparer.Ordinal).ToArray(); foreach (var file in entityFiles) { @@ -284,7 +288,7 @@ public static ParsedModel ParseModelFolder(string folderPath) // Get files in folder Other/Relationships (directory may not exist in scaffolded solutions) var relationshipsDir = Path.Combine(folderPath, "Other", "Relationships"); var relationshipFiles = Directory.Exists(relationshipsDir) - ? Directory.GetFiles(relationshipsDir, "*.xml", SearchOption.AllDirectories) + ? [.. Directory.GetFiles(relationshipsDir, "*.xml", SearchOption.AllDirectories).OrderBy(f => f, StringComparer.Ordinal)] : Array.Empty(); foreach (var file in relationshipFiles) { @@ -302,7 +306,7 @@ public static ParsedModel ParseModelFolder(string folderPath) // Get files in folder called OptionSets (directory may not exist in scaffolded solutions) var optionsetsDir = Path.Combine(folderPath, "OptionSets"); var optionsetFiles = Directory.Exists(optionsetsDir) - ? Directory.GetFiles(optionsetsDir, "*.xml", SearchOption.AllDirectories) + ? [.. Directory.GetFiles(optionsetsDir, "*.xml", SearchOption.AllDirectories).OrderBy(f => f, StringComparer.Ordinal)] : Array.Empty(); foreach (var file in optionsetFiles) { diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs index 4a8ea5d7..f8073c7f 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs @@ -11,8 +11,7 @@ public class Module { public Module() { - var random = new Random(); - Colorhex = string.Format("#{0:X6}", random.Next(0x1000000)); + Colorhex = ColourFor(ModuleName); } public Module(string module, XDocument xml) @@ -20,8 +19,7 @@ public Module(string module, XDocument xml) ModuleName = module; XmlDoc = xml; - var random = new Random(); - Colorhex = string.Format("#{0:X6}", random.Next(0x1000000)); + Colorhex = ColourFor(ModuleName); entities = XmlDoc.Descendants().Where(x => x.Name == "Entity").ToList(); relationships = XmlDoc.Descendants().Where(x => x.Name == "EntityRelationship").ToList(); @@ -36,4 +34,21 @@ public Module(string module, XDocument xml) public List optionsets = []; public string Colorhex { get; } + + /// + /// Derives the module colour from its name so the same input always converts to the + /// same bytes. A random colour made every conversion a spurious diff, which meant a + /// generated diagram could not be committed or compared across a model change. + /// FNV-1a rather than string.GetHashCode, which is randomised per process on .NET Core. + /// + private static string ColourFor(string moduleName) + { + uint hash = 2166136261; + foreach (var c in moduleName ?? string.Empty) + { + hash ^= c; + hash *= 16777619; + } + return string.Format("#{0:X6}", hash & 0xFFFFFF); + } } diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs index 97b345e3..36d3fa2f 100644 --- a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs @@ -104,4 +104,18 @@ public void PicklistWithUnresolvableOptionSet_KeepsColumnInsteadOfDroppingIt() // RowType is deliberately left alone so each translator keeps its own handling. Assert.True(string.IsNullOrEmpty(row!.OptionSetName)); } + + // ---- Defect 4: output was not reproducible — colours came from new Random() -------- + + [Fact] + public void ModuleColour_IsDerivedFromName_SoConversionIsReproducible() + { + var a = new Model.Module("Areas/Service/Project/Model", new XDocument(new XElement("root"))); + var b = new Model.Module("Areas/Service/Project/Model", new XDocument(new XElement("root"))); + var other = new Model.Module("Areas/Environment/Start/Model", new XDocument(new XElement("root"))); + + Assert.Equal(a.Colorhex, b.Colorhex); + Assert.NotEqual(a.Colorhex, other.Colorhex); + Assert.Matches("^#[0-9A-F]{6}$", a.Colorhex); + } } From 6d6a6dca993753941dd9f9a6a07bf37773fe25ef Mon Sep 17 00:00:00 2001 From: David Hudec Date: Tue, 1 Sep 2026 15:16:24 +0200 Subject: [PATCH 04/10] fix(data): give a self-referencing N:N two distinct sides Where an entity has a many-to-many with itself, the intersect table carried the same column twice, the same Ref twice, and the same EDMX navigation property twice. A DBML parser rejects the first two outright. Both sides resolved to id, and both legs carried the relationship name. The second column and the second leg's name are suffixed positionally. The real per-side names live in metadata (Entity1/Entity2IntersectAttribute) and are author-chosen - the platform's own example pairs connectionroleid with associatedconnectionroleid - so they cannot be derived from solution XML and are not guessed at here. Known limit: on the entity side EDMX still names the navigation property after the primary key row, so one duplicate remains there. The intersect side is clean. Co-Authored-By: Claude Opus 5 --- .../DataModelConverterService.cs | 38 ++++++++++-- .../DataModelConverterServiceTests.cs | 61 +++++++++++++++++++ 2 files changed, 93 insertions(+), 6 deletions(-) diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs index f4c887c6..bd6d728b 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs @@ -429,6 +429,21 @@ public static List ParseRelationships(List modules, List ParseRelationships(List modules, List x.RowType == RowType.Primarykey), connectionTable, - connectionTable.Rows.FirstOrDefault(x => x.Name == firstEntityTable.LogicalName + "id")); + connectionTable.Rows.FirstOrDefault(x => x.Name == firstRowName)); - var secondToMid = new Relationship(relationship.Attribute("Name").Value, + var secondToMid = new Relationship(secondRelationshipName, "ManyToOne", secondEntityTable, secondEntityTable.Rows.FirstOrDefault(x => x.RowType == RowType.Primarykey), connectionTable, - connectionTable.Rows.FirstOrDefault(x => x.Name == secondEntityTable.LogicalName + "id")); + connectionTable.Rows.FirstOrDefault(x => x.Name == secondRowName)); EntityRelationships.Add(firstToMid); EntityRelationships.Add(secondToMid); diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs index 36d3fa2f..c88c7b34 100644 --- a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs @@ -118,4 +118,65 @@ public void ModuleColour_IsDerivedFromName_SoConversionIsReproducible() Assert.NotEqual(a.Colorhex, other.Colorhex); Assert.Matches("^#[0-9A-F]{6}$", a.Colorhex); } + + // ---- Defect 5: a self-referencing N:N emitted duplicate columns and refs ----------- + + [Fact] + public void SelfReferencingManyToMany_ProducesTwoDistinctIntersectColumns() + { + var manyToMany = XElement.Parse(""" + + ManyToMany + contoso_thing + contoso_thing + contoso_thing_thing + + """); + var module = ModuleWith([Entity("contoso_thing")], [manyToMany]); + + var model = DataModelConverterService.ParseModules([module]); + var intersect = model.tables.Single(t => t.LogicalName == "contoso_thing_thing"); + + var names = intersect.Rows.Select(r => r.Name).ToList(); + Assert.Equal(names.Count, names.Distinct().Count()); + + // Two legs, each anchored on its own column — one shared column produced a + // duplicate endpoint pair, which a DBML parser rejects outright. + var legs = model.relationships.Where(r => r.RighSideTable?.LogicalName == "contoso_thing_thing").ToList(); + Assert.Equal(2, legs.Count); + Assert.Equal(2, legs.Select(l => l.RighSideRow?.Name).Distinct().Count()); + + // The legs also need distinct relationship names: EDMX renders the intersect side + // as NavigationProperty Name="{relationship.Name}", with a matching Partner and + // NavigationPropertyBinding Path, so sharing one name emits each of them twice. + Assert.Equal(2, legs.Select(l => l.Name).Distinct().Count()); + } + + [Fact] + public void SelfReferencingManyToMany_RendersDistinctNavigationPropertiesOnTheIntersect() + { + var manyToMany = XElement.Parse(""" + + ManyToMany + contoso_thing + contoso_thing + contoso_thing_thing + + """); + var module = ModuleWith([Entity("contoso_thing")], [manyToMany]); + + var model = DataModelConverterService.ParseModules([module]); + var edmx = DataModelConverterService.ConvertToEDMX(model); + + // The intersect's own EntityType carries one navigation property per leg. + var intersect = System.Text.RegularExpressions.Regex.Match( + edmx, "", + System.Text.RegularExpressions.RegexOptions.Singleline).Value; + var navNames = System.Text.RegularExpressions.Regex + .Matches(intersect, " m.Groups[1].Value).ToList(); + + Assert.Equal(2, navNames.Count); + Assert.Equal(2, navNames.Distinct().Count()); + } } From c040d753c05a83d4ed2fe8b75444b693da4cb551 Mon Sep 17 00:00:00 2001 From: David Hudec Date: Tue, 1 Sep 2026 15:16:44 +0200 Subject: [PATCH 05/10] fix(data): accept --target plainsql, which was advertised and always rejected Every plainsql conversion failed, and the error listed the formats it did support - contradicting the option's own help. The format is declared in the option's AllowedValues and fully implemented in the conversion switch, but was missing from the service's SupportedFormats guard three lines earlier. Added. A test now asserts every value the option advertises actually converts, so the two lists cannot drift apart again. Co-Authored-By: Claude Opus 5 --- .../DataModelConverterService.cs | 2 +- .../DataModelConverterServiceTests.cs | 59 ++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs index bd6d728b..ec4abf3d 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs @@ -18,7 +18,7 @@ namespace TALXIS.CLI.Features.Data.DataModelConverter; public class DataModelConverterService { private static readonly ILogger _logger = TxcLoggerFactory.CreateLogger(nameof(DataModelConverterService)); - private static readonly string[] SupportedFormats = ["dbml", "sql", "edmx", "ribbon"]; + private static readonly string[] SupportedFormats = ["dbml", "sql", "plainsql", "edmx", "ribbon"]; /// /// Parses a Power Platform solution from a solution project folder, a declarations diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs index c88c7b34..c764cc66 100644 --- a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/DataModelConverterServiceTests.cs @@ -1,4 +1,4 @@ -using System.IO; +using System.IO; using System.Linq; using System.Reflection; using DotMake.CommandLine; @@ -179,4 +179,61 @@ public void SelfReferencingManyToMany_RendersDistinctNavigationPropertiesOnTheIn Assert.Equal(2, navNames.Count); Assert.Equal(2, navNames.Distinct().Count()); } + + // ---- Every relationship endpoint must be non-null: the translators dereference ----- + // ---- LeftSideRow/RighSideRow without a null check. ------------------------------- + + [Fact] + public void EveryEmittedRelationship_HasBothEndpointsResolved() + { + var module = ModuleWith( + [Entity("account"), Entity("contoso_project", Lookup("contoso_ownerid"))], + [OneToMany("rel_owner", "contoso_project", "contoso_ownerid", "account")]); + + var model = DataModelConverterService.ParseModules([module]); + + Assert.All(model.relationships, r => + { + Assert.NotNull(r.LeftSideTable); + Assert.NotNull(r.LeftSideRow); + Assert.NotNull(r.RighSideTable); + Assert.NotNull(r.RighSideRow); + }); + } + + // ---- Every format the CLI advertises must actually convert ------------------------ + // plainsql was listed in the option's AllowedValues and fully implemented in the + // format switch, but missing from the service's SupportedFormats guard -- so it was + // rejected on every invocation, and the error message listed that same guard as truth. + + [Fact] + public void EveryAdvertisedTargetFormat_IsAcceptedByTheService() + { + var advertised = typeof(DataModelConvertCliCommand) + .GetProperty(nameof(DataModelConvertCliCommand.TargetFormat))! + .GetCustomAttribute()! + .AllowedValues! + .Cast() + .ToList(); + + Assert.NotEmpty(advertised); + + var dir = Path.Combine(Path.GetTempPath(), "txc-fmt-" + Path.GetRandomFileName()); + var entityDir = Path.Combine(dir, "Entities", "contoso_thing"); + Directory.CreateDirectory(entityDir); + File.WriteAllText(Path.Combine(entityDir, "Entity.xml"), Entity("contoso_thing").ToString()); + try + { + foreach (var format in advertised) + { + var outFile = Path.Combine(dir, "out." + format); + var ex = Record.Exception(() => DataModelConverterService.ConvertModel(dir, format, outFile)); + Assert.True(ex is null, $"--target {format} is advertised but failed: {ex?.Message}"); + } + } + finally + { + Directory.Delete(dir, recursive: true); + } + } } From e5ab0cd4dea39412f27a180651075a8f10551a71 Mon Sep 17 00:00:00 2001 From: David Hudec Date: Tue, 1 Sep 2026 15:22:56 +0200 Subject: [PATCH 06/10] fix(data): dedup attribute rows when one table is declared more than once A table declared by two modules ended up with the same column listed twice. ParseMultipleRowsFromXml appended every parsed row without checking whether the table already carried one of that name. Harmless while only one input could be given; routine as soon as several can. Rows are matched case-insensitively. Where two declarations disagree the first input wins, so the result is deterministic in the order the caller gave; a differing type warns rather than aborting, because several modules extending one shared table is normal for a layered product; and text lengths widen but never narrow, since a consumer breaks on too little room, not too much. Co-Authored-By: Claude Opus 5 --- .../DataModelConverter/Model/Table.cs | 28 +++++- .../MultipleInputMergeTests.cs | 88 +++++++++++++++++++ 2 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Table.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Table.cs index 5ccd5960..7241b512 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Table.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Table.cs @@ -1,4 +1,6 @@ -using DocumentFormat.OpenXml.Vml.Office; +using TALXIS.CLI.Logging; +using Microsoft.Extensions.Logging; +using DocumentFormat.OpenXml.Vml.Office; using System.Text.Json.Serialization; using System; using System.Collections.Generic; @@ -20,6 +22,8 @@ public enum TableType public class Table { + private static readonly ILogger _logger = TxcLoggerFactory.CreateLogger(nameof(Table)); + public Table() { } public Table(XElement element) @@ -58,8 +62,28 @@ public void ParseMultipleRowsFromXml(List xElements) foreach (var element in xElements) { var row = TableRow.ParseXElement(element); - if (row != null) + if (row == null) + continue; + + var existing = Rows.FirstOrDefault(x => string.Equals(x.Name, row.Name, StringComparison.OrdinalIgnoreCase)); + if (existing == null) + { Rows.Add(row); + } + else if (existing.RowType != row.RowType) + { + // Several modules extending one shared table is the normal case for a + // layered product, so a divergent declaration warns rather than aborting. + // First input wins, which makes the result deterministic in input order. + _logger.LogWarning( + "Attribute {Table}.{Attribute} is declared as {ExistingType} in one input and {NewType} in another; keeping the first.", + LogicalName, row.Name, existing.RowType, row.RowType); + } + else if (row.MaxLenght > existing.MaxLenght) + { + // Widen, never narrow: a consumer breaks on too little room, not too much. + existing.MaxLenght = row.MaxLenght; + } } } diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs new file mode 100644 index 00000000..7c781161 --- /dev/null +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs @@ -0,0 +1,88 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Xml.Linq; +using TALXIS.CLI.Features.Data.DataModelConverter; +using Model = TALXIS.CLI.Features.Data.DataModelConverter.Model; +using Xunit; + +namespace TALXIS.CLI.Tests.Data.DataModelConverter; + +/// +/// Merging several declaration folders into one model. A delivery project's data model is +/// spread across the modules a product ships plus the project's own layer, and several of +/// them declare part of the same table — so converting each separately and concatenating +/// the output keeps only the first declaration of each table. +/// +public class MultipleInputMergeTests +{ + private static XElement Entity(string logicalName, params string[] attributes) => + XElement.Parse($""" + + {logicalName} + + + + primarykey + {string.Join("", attributes)} + + + + + """); + + private static string Attr(string name, string type, int? maxLength = null) => + $"""{type}{(maxLength is null ? "" : $"{maxLength}")}"""; + + private static Model.Module ModuleOf(string name, params XElement[] entities) + { + var module = new Model.Module { ModuleName = name }; + module.entities.AddRange(entities); + return module; + } + + private static Model.Table TableIn(Model.ParsedModel model, string logicalName) => + model.tables.Single(t => t.LogicalName == logicalName); + + [Fact] + public void SameAttributeDeclaredInBothModules_ProducesOneColumnNotTwo() + { + var attr = Attr("contoso_shared", "nvarchar", 50); + var model = DataModelConverterService.ParseModules( + [ModuleOf("base", Entity("contoso_thing", attr)), ModuleOf("layer", Entity("contoso_thing", attr))]); + + var rows = TableIn(model, "contoso_thing").Rows + .Where(r => string.Equals(r.Name, "contoso_shared", System.StringComparison.OrdinalIgnoreCase)); + Assert.Single(rows); + } + + [Fact] + public void ConflictingTypeForOneAttribute_KeepsTheFirstInputsDeclaration() + { + var model = DataModelConverterService.ParseModules( + [ + ModuleOf("first", Entity("contoso_thing", Attr("contoso_field", "nvarchar", 50))), + ModuleOf("second", Entity("contoso_thing", Attr("contoso_field", "int"))), + ]); + + var row = TableIn(model, "contoso_thing").Rows + .Single(r => string.Equals(r.Name, "contoso_field", System.StringComparison.OrdinalIgnoreCase)); + Assert.Equal(Model.RowType.Nvarchar, row.RowType); + } + + [Theory] + [InlineData(50, 200, 200)] + [InlineData(200, 50, 200)] + public void DifferingTextLengths_WidenNeverNarrow_RegardlessOfInputOrder(int first, int second, int expected) + { + var model = DataModelConverterService.ParseModules( + [ + ModuleOf("first", Entity("contoso_thing", Attr("contoso_text", "nvarchar", first))), + ModuleOf("second", Entity("contoso_thing", Attr("contoso_text", "nvarchar", second))), + ]); + + var row = TableIn(model, "contoso_thing").Rows + .Single(r => string.Equals(r.Name, "contoso_text", System.StringComparison.OrdinalIgnoreCase)); + Assert.Equal(expected, row.MaxLenght); + } +} From 73ac6d1a22bb6b659a251b2bc416cdaa11c92f59 Mon Sep 17 00:00:00 2001 From: David Hudec Date: Tue, 1 Sep 2026 15:22:57 +0200 Subject: [PATCH 07/10] fix(data): compute the module colour instead of capturing it in the constructor Every module would come out the same colour once modules carry distinct names. Colorhex was assigned in the constructor, which runs before an object initializer sets ModuleName - so the colour was derived from an empty name. Invisible while there was only ever one module. Colorhex is now a computed property, so it always reflects the name in effect. Co-Authored-By: Claude Opus 5 --- .../DataModelConverter/Model/Module.cs | 12 +++++------- .../DataModelConverter/MultipleInputMergeTests.cs | 11 +++++++++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs index f8073c7f..c69992ab 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs @@ -9,18 +9,13 @@ namespace TALXIS.CLI.Features.Data.DataModelConverter.Model; public class Module { - public Module() - { - Colorhex = ColourFor(ModuleName); - } + public Module() { } public Module(string module, XDocument xml) { ModuleName = module; XmlDoc = xml; - Colorhex = ColourFor(ModuleName); - entities = XmlDoc.Descendants().Where(x => x.Name == "Entity").ToList(); relationships = XmlDoc.Descendants().Where(x => x.Name == "EntityRelationship").ToList(); optionsets = XmlDoc.Descendants().Where(x => x.Name == "optionset").ToList(); @@ -33,7 +28,10 @@ public Module(string module, XDocument xml) public List relationships = []; public List optionsets = []; - public string Colorhex { get; } + /// Computed, not assigned in the constructor: an object initializer sets + /// ModuleName after the constructor body runs, which would colour every module + /// from an empty name. + public string Colorhex => ColourFor(ModuleName); /// /// Derives the module colour from its name so the same input always converts to the diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs index 7c781161..562097fd 100644 --- a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs @@ -85,4 +85,15 @@ public void DifferingTextLengths_WidenNeverNarrow_RegardlessOfInputOrder(int fir .Single(r => string.Equals(r.Name, "contoso_text", System.StringComparison.OrdinalIgnoreCase)); Assert.Equal(expected, row.MaxLenght); } + + [Fact] + public void ModulesAreColouredApart_SoAMergedDiagramShowsWhereEachTableCameFrom() + { + var a = new Model.Module { ModuleName = "src/Modules.Core/Model" }; + var b = new Model.Module { ModuleName = "Areas/Service/Project/Model" }; + + // Assigned through an object initializer, which runs after the constructor body — + // a colour computed in the constructor would be identical for both. + Assert.NotEqual(a.Colorhex, b.Colorhex); + } } From 6bdc1e31c1b977a18709b8e76267f693e26d6fd9 Mon Sep 17 00:00:00 2001 From: David Hudec Date: Tue, 1 Sep 2026 15:23:14 +0200 Subject: [PATCH 08/10] feat(data): merge several declaration folders into one model A delivery project's model is spread across the modules a product ships plus the project's own layer, and several of them declare part of the same table. Converting each separately and concatenating the output keeps only the first declaration of each table, so the merge had to be done by hand. --input accepted a single path, and only zip inputs were ever built into more than one Module. --input is now repeatable. Folder and zip inputs both resolve to a Module and go through the ParseModules seam that already existed for zips, so the two can be mixed in one invocation. Modules are named after the folders that own their declarations, so a merged diagram attributes each table to its source instead of rendering an empty comment. Co-Authored-By: Claude Opus 5 --- .../DataModelConvertCliCommand.cs | 8 +- .../DataModelConverterService.cs | 115 +++++++++++++----- .../MultipleInputMergeTests.cs | 69 ++++++++++- 3 files changed, 157 insertions(+), 35 deletions(-) diff --git a/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs b/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs index a42eee0a..aab63867 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs @@ -20,10 +20,10 @@ public class DataModelConvertCliCommand : TxcLeafCommand [CliOption( Name = "--input", Aliases = ["-i"], - Description = "Path to the input: a solution project folder (.cdsproj/.csproj with SolutionRootPath), a declarations folder, or a .zip solution file. Defaults to the current directory.", + Description = "Path to an input: a solution project folder (.cdsproj/.csproj with SolutionRootPath), a declarations folder, or a .zip solution file. Can be specified multiple times to merge several sources into one model; earlier inputs win where two declare the same attribute differently. Defaults to the current directory.", Required = false )] - public string? InputPath { get; set; } + public List InputPaths { get; set; } = []; [CliOption( Name = "--target", @@ -43,7 +43,7 @@ public class DataModelConvertCliCommand : TxcLeafCommand protected override Task ExecuteAsync() { - var inputPath = InputPath ?? Directory.GetCurrentDirectory(); + var inputPaths = InputPaths.Count > 0 ? InputPaths : [Directory.GetCurrentDirectory()]; var outputDir = OutputDirectory ?? Path.Combine(Directory.GetCurrentDirectory(), ExportsFolderName); Directory.CreateDirectory(outputDir); @@ -52,7 +52,7 @@ protected override Task ExecuteAsync() var extension = TargetFormat!.ToLower() == "plainsql" ? "sql" : TargetFormat.ToLower(); var outputFilePath = Path.Combine(outputDir, $"solution.{extension}"); - DataModelConverterService.ConvertModel(inputPath, TargetFormat!, outputFilePath); + DataModelConverterService.ConvertModel(inputPaths, TargetFormat!, outputFilePath); OutputFormatter.WriteResult("succeeded", $"Output written to: {outputFilePath}"); return Task.FromResult(ExitSuccess); diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs index ec4abf3d..0a374bc6 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs @@ -35,29 +35,44 @@ public class DataModelConverterService /// /// public static void ConvertModel(string inputPath, string targetFormat, string outputFilePath) + => ConvertModel([inputPath], targetFormat, outputFilePath); + + /// + /// Converts one or more inputs into a single model. Each input is resolved + /// independently -- a solution project folder, a declarations folder, or a .zip -- and + /// they may be mixed. Earlier inputs take precedence where two declare the same + /// attribute differently. + /// + public static void ConvertModel(List inputPaths, string targetFormat, string outputFilePath) { if (!SupportedFormats.Contains(targetFormat.ToLower())) throw new ArgumentException($"Unsupported target format '{targetFormat}'. Supported formats are: {string.Join(", ", SupportedFormats)}."); - ParsedModel parsedModel; + if (inputPaths is null || inputPaths.Count == 0) + throw new ArgumentException("At least one input path is required."); - if (Directory.Exists(inputPath)) - { - var declarationsPath = ResolveDeclarationsFolder(inputPath); - parsedModel = ParseModelFolder(declarationsPath); - } - else if (File.Exists(inputPath)) - { - using var fileStream = new FileStream(inputPath, FileMode.Open, FileAccess.Read); - using var memoryStream = new MemoryStream(); - fileStream.CopyTo(memoryStream); - parsedModel = ParseModel(Convert.ToBase64String(memoryStream.ToArray())); - } - else + List modules = []; + foreach (var inputPath in inputPaths) { - throw new FileNotFoundException($"Input path '{inputPath}' does not exist."); + if (Directory.Exists(inputPath)) + { + modules.Add(ParseFolderIntoModule(ResolveDeclarationsFolder(inputPath))); + } + else if (File.Exists(inputPath)) + { + using var fileStream = new FileStream(inputPath, FileMode.Open, FileAccess.Read); + using var memoryStream = new MemoryStream(); + fileStream.CopyTo(memoryStream); + modules.Add(ParseZipIntoModule(Convert.ToBase64String(memoryStream.ToArray()))); + } + else + { + throw new FileNotFoundException($"Input path '{inputPath}' does not exist."); + } } + var parsedModel = ParseModules(modules); + var resultString = targetFormat.ToLower() switch { "edmx" => ConvertToEDMX(parsedModel), @@ -255,8 +270,44 @@ public static string ConvertToEDMX(ParsedModel model) } public static ParsedModel ParseModelFolder(string folderPath) + => ParseModelFolders([folderPath]); + + /// + /// Parses several declarations folders into one model, merging attribute-level. + /// A project's model is rarely one solution: the base product ships several modules + /// that each declare part of a shared table, so converting them separately and + /// concatenating the files loses everything but the first declaration of each table. + /// + public static ParsedModel ParseModelFolders(List folderPaths) + => ParseModules([.. folderPaths.Select(ParseFolderIntoModule)]); + + /// + /// Names a module after the folders that own its declarations, so tables can be + /// attributed once several inputs are merged. Several segments are kept because the + /// leaf is almost always "Model" -- one segment would give every input the same name + /// and, with the colour derived from it, the same colour. + /// + private static string ModuleNameFor(string declarationsFolder) + { + var full = Path.GetFullPath(declarationsFolder) + .TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + var segments = full.Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar) + .Where(x => x.Length > 0) + .ToList(); + + // Drop the trailing "Declarations" (or "CDS") folder; it carries no information. + if (segments.Count > 1 && (segments[^1].Equals("Declarations", StringComparison.OrdinalIgnoreCase) + || segments[^1].Equals("CDS", StringComparison.OrdinalIgnoreCase))) + { + segments.RemoveAt(segments.Count - 1); + } + + return string.Join('/', segments.TakeLast(3)); + } + + private static Module ParseFolderIntoModule(string folderPath) { - Module module = new(); + Module module = new() { ModuleName = ModuleNameFor(folderPath) }; // Get files named Entity.xml in subfolders // Ordered: Directory.GetFiles gives no ordering guarantee, so without this the @@ -321,8 +372,24 @@ public static ParsedModel ParseModelFolder(string folderPath) } } - return ParseModules([module]); + return module; + } + + private static Module ParseZipIntoModule(string base64solution) + { + using ZipArchive archive = new(new MemoryStream(Convert.FromBase64String(base64solution))); + + var customizationsxml = archive.Entries.FirstOrDefault(x => x.FullName.Equals("customizations.xml", StringComparison.OrdinalIgnoreCase)); + var solutionxml = archive.Entries.FirstOrDefault(x => x.FullName.Equals("solution.xml", StringComparison.OrdinalIgnoreCase)); + if (customizationsxml == null || solutionxml == null) + { + throw new FileNotFoundException("The solution archive does not contain the required customizations.xml or solution.xml files."); + } + + return new Module( + XDocument.Load(solutionxml.Open()).Descendants().First(x => x.Name == "UniqueName").Value, + XDocument.Load(customizationsxml.Open())); } public static ParsedModel ParseModel(string? base64solution) @@ -342,19 +409,7 @@ public static ParsedModel ParseModel(List base64solution) foreach (var solution in base64solution) { - using ZipArchive archive = new(new MemoryStream(Convert.FromBase64String(solution))); - - var customizationsxml = archive.Entries.FirstOrDefault(x => x.FullName.Equals("customizations.xml", StringComparison.OrdinalIgnoreCase)); - var solutionxml = archive.Entries.FirstOrDefault(x => x.FullName.Equals("solution.xml", StringComparison.OrdinalIgnoreCase)); - - if (customizationsxml == null || solutionxml == null) - { - throw new FileNotFoundException("The solution archive does not contain the required customizations.xml or solution.xml files."); - } - - Module foundModule = new(XDocument.Load(solutionxml.Open()).Descendants().First(x => x.Name == "UniqueName").Value, XDocument.Load(customizationsxml.Open())); - - modules.Add(foundModule); + modules.Add(ParseZipIntoModule(solution)); } return ParseModules(modules); diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs index 562097fd..c1fe45ac 100644 --- a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MultipleInputMergeTests.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Xml.Linq; @@ -44,6 +44,20 @@ private static Model.Module ModuleOf(string name, params XElement[] entities) private static Model.Table TableIn(Model.ParsedModel model, string logicalName) => model.tables.Single(t => t.LogicalName == logicalName); + [Fact] + public void TwoModulesDeclaringOneTable_MergeIntoASingleTableWithBothColumns() + { + var a = ModuleOf("base", Entity("contoso_thing", Attr("contoso_fromBase", "nvarchar", 50))); + var b = ModuleOf("layer", Entity("contoso_thing", Attr("contoso_fromLayer", "nvarchar", 50))); + + var model = DataModelConverterService.ParseModules([a, b]); + + Assert.Single(model.tables, t => t.LogicalName == "contoso_thing"); + var names = TableIn(model, "contoso_thing").Rows.Select(r => r.Name).ToList(); + Assert.Contains("contoso_frombase", names.Select(n => n.ToLowerInvariant())); + Assert.Contains("contoso_fromlayer", names.Select(n => n.ToLowerInvariant())); + } + [Fact] public void SameAttributeDeclaredInBothModules_ProducesOneColumnNotTwo() { @@ -96,4 +110,57 @@ public void ModulesAreColouredApart_SoAMergedDiagramShowsWhereEachTableCameFrom( // a colour computed in the constructor would be identical for both. Assert.NotEqual(a.Colorhex, b.Colorhex); } + + [Fact] + public void OneFolder_ThroughTheListEntryPoint_MatchesTheSingleFolderEntryPoint() + { + var dir = Path.Combine(Path.GetTempPath(), "txc-merge-" + Path.GetRandomFileName()); + var entityDir = Path.Combine(dir, "Entities", "contoso_thing"); + Directory.CreateDirectory(entityDir); + File.WriteAllText(Path.Combine(entityDir, "Entity.xml"), + Entity("contoso_thing", Attr("contoso_field", "nvarchar", 50)).ToString()); + try + { + var single = DataModelConverterService.ParseModelFolder(dir); + var viaList = DataModelConverterService.ParseModelFolders([dir]); + + Assert.Equal(single.tables.Count, viaList.tables.Count); + Assert.Equal(single.relationships.Count, viaList.relationships.Count); + Assert.Equal( + single.tables.Select(t => t.LogicalName).OrderBy(x => x), + viaList.tables.Select(t => t.LogicalName).OrderBy(x => x)); + } + finally + { + Directory.Delete(dir, recursive: true); + } + } + + [Fact] + public void TwoFoldersOnDisk_MergeAttributesAcrossTheFolderBoundary() + { + var root = Path.Combine(Path.GetTempPath(), "txc-merge-" + Path.GetRandomFileName()); + var folders = new List(); + foreach (var (name, attr) in new[] { ("base", "contoso_a"), ("layer", "contoso_b") }) + { + var dir = Path.Combine(root, name, "Declarations"); + Directory.CreateDirectory(Path.Combine(dir, "Entities", "contoso_thing")); + File.WriteAllText(Path.Combine(dir, "Entities", "contoso_thing", "Entity.xml"), + Entity("contoso_thing", Attr(attr, "nvarchar", 50)).ToString()); + folders.Add(dir); + } + try + { + var model = DataModelConverterService.ParseModelFolders(folders); + var names = TableIn(model, "contoso_thing").Rows + .Select(r => r.Name.ToLowerInvariant()).ToList(); + + Assert.Contains("contoso_a", names); + Assert.Contains("contoso_b", names); + } + finally + { + Directory.Delete(root, recursive: true); + } + } } From 48abd5de103c4962ef7d397aa41c09cdec6143cb Mon Sep 17 00:00:00 2001 From: David Hudec Date: Tue, 1 Sep 2026 10:24:51 +0200 Subject: [PATCH 09/10] feat(data): scope conversion to a model-driven app Adds --app and --root, so the question the tool answers can be "what is this app built on" rather than only "what does this solution declare". An app names its tables in its own AppModule file, so this needs no environment. Three things about how apps sit on disk drove the implementation: - The search is anchored on the AppModules folder, not on "Declarations". Older modules keep their declarations under "CDS", and a search anchored on either name silently misses the other. - Identity is read from the UniqueName inside the file, never the folder name -- the two differ in case in the wild, which is invisible on Windows and wrong on a case-sensitive filesystem. - One logical app can be declared across several files, a base declaration plus fragments from other areas carrying solutionaction="Added". Its component set is the union of all of them. Only type="1" components carry a table name; views, forms, charts and workflows reference their owner by id alone. Sitemap entities are picked up as well, from both the Entity attribute and the etn parameter inside a Url -- both forms occur, sometimes in the same file. --root exists because apps and entity schema live in different modules, so scoping to an app structurally needs to reach past a single declarations folder. It expands to every declarations folder beneath it. Pass the product repository as a second root when the base model lives there. Scoping runs before relationships are built. Filtering afterwards would let a relationship between two dropped tables synthesise both of them straight back as stubs -- measured at 261 tables reappearing before this was ordered correctly. A relationship is kept when its referencing side is in scope, so a lookup out of the app still terminates somewhere visible rather than dangling. Option sets belonging to dropped tables are pruned too, or the output declares more enums than it has columns using them. Measured, project root plus product root, one invocation each: ntg_projectmanagement 70 tables 124 refs 883 cols 88 enums ntg_administration 36 59 388 29 ntg_easementmanagement 79 183 1055 130 ntg_hiltipartnerportal 25 34 216 21 All parse with @dbml/core. Unknown app names fail listing the apps that were found. Without --app nothing is filtered, and single-input conversion across three solutions and all five targets is unchanged and idempotent. Co-Authored-By: Claude Opus 5 --- .../DataModelConvertCliCommand.cs | 62 ++++- .../AppScope/AppScopeFilter.cs | 42 ++++ .../AppScope/AppScopeResolver.cs | 175 +++++++++++++++ .../DataModelConverterService.cs | 83 ++++++- .../Data/DataModelConverter/AppScopeTests.cs | 211 ++++++++++++++++++ 5 files changed, 569 insertions(+), 4 deletions(-) create mode 100644 src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeFilter.cs create mode 100644 src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeResolver.cs create mode 100644 tests/TALXIS.CLI.Tests/Data/DataModelConverter/AppScopeTests.cs diff --git a/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs b/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs index aab63867..baa08458 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs @@ -25,6 +25,20 @@ public class DataModelConvertCliCommand : TxcLeafCommand )] public List InputPaths { get; set; } = []; + [CliOption( + Name = "--root", + Description = "Path to a repository root; every declarations folder beneath it becomes an input. Can be specified multiple times. Use this rather than listing folders when a project's model spans many modules, and pass the product repository as a second root when the base model lives there.", + Required = false + )] + public List Roots { get; set; } = []; + + [CliOption( + Name = "--app", + Description = "Unique name of a model-driven app. Narrows the output to the tables that app is built on, instead of everything the inputs declare. App modules are searched for under --root, or under the inputs when no root is given.", + Required = false + )] + public string? AppUniqueName { get; set; } + [CliOption( Name = "--target", Description = "Target format for the conversion.", @@ -43,7 +57,33 @@ public class DataModelConvertCliCommand : TxcLeafCommand protected override Task ExecuteAsync() { - var inputPaths = InputPaths.Count > 0 ? InputPaths : [Directory.GetCurrentDirectory()]; + var inputPaths = new List(InputPaths); + + foreach (var root in Roots) + { + var discovered = DataModelConverterService.DiscoverDeclarationFolders(root); + if (discovered.Count == 0) + { + Logger.LogWarning("No declarations were found under root {Root}.", root); + } + inputPaths.AddRange(discovered); + } + + // Scoping to an app needs to reach the module that declares it, which is not the + // module that declares the entities -- so when only an app is named, search from + // the enclosing repository rather than the working directory alone. + var appSearchRoots = new List(Roots); + if (!string.IsNullOrWhiteSpace(AppUniqueName) && appSearchRoots.Count == 0) + { + var enclosing = FindEnclosingRepositoryRoot(Directory.GetCurrentDirectory()); + appSearchRoots.Add(enclosing); + Logger.LogInformation("Searching for app modules under {Root}.", enclosing); + } + + if (inputPaths.Count == 0) + { + inputPaths.Add(Directory.GetCurrentDirectory()); + } var outputDir = OutputDirectory ?? Path.Combine(Directory.GetCurrentDirectory(), ExportsFolderName); Directory.CreateDirectory(outputDir); @@ -52,12 +92,30 @@ protected override Task ExecuteAsync() var extension = TargetFormat!.ToLower() == "plainsql" ? "sql" : TargetFormat.ToLower(); var outputFilePath = Path.Combine(outputDir, $"solution.{extension}"); - DataModelConverterService.ConvertModel(inputPaths, TargetFormat!, outputFilePath); + DataModelConverterService.ConvertModel(inputPaths, TargetFormat!, outputFilePath, AppUniqueName, appSearchRoots); OutputFormatter.WriteResult("succeeded", $"Output written to: {outputFilePath}"); return Task.FromResult(ExitSuccess); } + /// + /// Walks up for the repository that encloses a directory, so an app can be found + /// without the caller naming a root. Falls back to the directory itself. + /// + private static string FindEnclosingRepositoryRoot(string startPath) + { + var dir = new DirectoryInfo(startPath); + while (dir != null) + { + if (Directory.Exists(Path.Combine(dir.FullName, ".git")) || dir.GetFiles("*.sln").Length > 0) + { + return dir.FullName; + } + dir = dir.Parent; + } + return startPath; + } + /// /// Ensures the exports folder is listed in the nearest .gitignore, /// adding an entry if it is not already present. diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeFilter.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeFilter.cs new file mode 100644 index 00000000..c22c9b08 --- /dev/null +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeFilter.cs @@ -0,0 +1,42 @@ +using System.Collections.Generic; +using System.Linq; +using Microsoft.Extensions.Logging; +using TALXIS.CLI.Features.Data.DataModelConverter.Model; +using TALXIS.CLI.Logging; + +namespace TALXIS.CLI.Features.Data.DataModelConverter.AppScope; + +/// Narrows a parsed model to the tables an app is built on. +public static class AppScopeFilter +{ + private static readonly ILogger _logger = TxcLoggerFactory.CreateLogger(nameof(AppScopeFilter)); + + /// + /// Drops tables the app does not declare. Runs before relationships are built, so a + /// dropped table cannot come back as a synthesised stub for a relationship that + /// pointed at it. + /// + public static void ApplyTableScope(List
tables, ResolvedAppScope scope) + { + var removed = tables.RemoveAll(t => + t.Type == TableType.InSolution && !scope.TableLogicalNames.Contains(t.LogicalName)); + + var missing = scope.TableLogicalNames + .Where(name => !tables.Any(t => string.Equals(t.LogicalName, name, System.StringComparison.OrdinalIgnoreCase))) + .OrderBy(x => x) + .ToList(); + + _logger.LogInformation( + "Scoped to app {App}: kept {Kept} table(s), dropped {Dropped} not declared by it.", + scope.UniqueName, tables.Count, removed); + + if (missing.Count > 0) + { + // The app names them but no input declares them — usually a module that was + // not passed in, which would otherwise show up only as an oddly small diagram. + _logger.LogWarning( + "App {App} references {Count} table(s) that none of the given inputs declare: {Tables}.", + scope.UniqueName, missing.Count, string.Join(", ", missing)); + } + } +} diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeResolver.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeResolver.cs new file mode 100644 index 00000000..e923bcd1 --- /dev/null +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeResolver.cs @@ -0,0 +1,175 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Xml.Linq; +using Microsoft.Extensions.Logging; +using TALXIS.CLI.Logging; + +namespace TALXIS.CLI.Features.Data.DataModelConverter.AppScope; + +/// The tables a model-driven app is built on, resolved from source. +public class ResolvedAppScope +{ + public string UniqueName { get; init; } = string.Empty; + + /// Compared case-insensitively: an app component's schemaName casing is not + /// guaranteed to match the casing of the entity's own declaration. + public HashSet TableLogicalNames { get; } = new(StringComparer.OrdinalIgnoreCase); + + /// Every file that contributed, for reporting which sources were read. + public List SourceFiles { get; } = []; +} + +/// +/// Resolves which tables an app declares, from the app module files on disk. No Dataverse +/// connection: the app's component list is in source, and so is everything it names. +/// +public static class AppScopeResolver +{ + private static readonly ILogger _logger = TxcLoggerFactory.CreateLogger(nameof(AppScopeResolver)); + + private const string AppModulesFolder = "AppModules"; + private const string SiteMapsFolder = "AppModuleSiteMaps"; + + /// The component type that carries a table's name. Views, forms, charts and + /// workflows reference their owning table only by id, so they cannot contribute one. + private const string EntityComponentType = "1"; + + public static ResolvedAppScope Resolve(IEnumerable searchRoots, string appUniqueName) + { + var byName = DiscoverAppModules(searchRoots); + + if (!byName.TryGetValue(appUniqueName, out var files) || files.Count == 0) + { + var known = byName.Keys.OrderBy(x => x, StringComparer.OrdinalIgnoreCase).ToList(); + throw new InvalidOperationException( + $"No app module named '{appUniqueName}' was found under the given inputs. " + + (known.Count == 0 + ? "No app modules were found at all — check that a repository root containing them was passed." + : $"Apps found: {string.Join(", ", known)}.")); + } + + var scope = new ResolvedAppScope { UniqueName = appUniqueName }; + + // One logical app can be declared across several files: a base declaration plus + // fragments contributed by other areas, whose components carry solutionaction="Added". + // The app's real component set is the union of all of them. + foreach (var file in files) + { + scope.SourceFiles.Add(file); + var doc = Load(file); + if (doc?.Root == null) continue; + + foreach (var component in doc.Root.Descendants("AppModuleComponent")) + { + if (component.Attribute("type")?.Value != EntityComponentType) continue; + var schemaName = component.Attribute("schemaName")?.Value; + if (!string.IsNullOrWhiteSpace(schemaName)) scope.TableLogicalNames.Add(schemaName); + } + } + + foreach (var table in ResolveSiteMapTables(searchRoots, appUniqueName)) + { + scope.TableLogicalNames.Add(table); + } + + _logger.LogInformation( + "App {App} resolves to {Count} tables, from {Files} declaration file(s).", + appUniqueName, scope.TableLogicalNames.Count, scope.SourceFiles.Count); + + return scope; + } + + /// + /// Every app module found, keyed by the UniqueName inside the file. The folder name is + /// only a locator — its casing can differ from the declared name, which matters on a + /// case-sensitive filesystem. + /// + public static Dictionary> DiscoverAppModules(IEnumerable searchRoots) + { + var byName = new Dictionary>(StringComparer.OrdinalIgnoreCase); + + foreach (var file in FilesUnder(searchRoots, AppModulesFolder, "AppModule*.xml")) + { + var uniqueName = Load(file)?.Root?.Element("UniqueName")?.Value; + if (string.IsNullOrWhiteSpace(uniqueName)) continue; + + if (!byName.TryGetValue(uniqueName, out var list)) + { + byName[uniqueName] = list = []; + } + list.Add(file); + } + + return byName; + } + + /// + /// Tables a sitemap surfaces. They appear either as an Entity attribute or as an etn + /// parameter inside a Url, and both forms occur in the same file. + /// + private static IEnumerable ResolveSiteMapTables(IEnumerable searchRoots, string appUniqueName) + { + foreach (var file in FilesUnder(searchRoots, SiteMapsFolder, "AppModuleSiteMap*.xml")) + { + var doc = Load(file); + if (doc?.Root == null) continue; + + var owner = doc.Root.Element("SiteMapUniqueName")?.Value; + if (!string.Equals(owner, appUniqueName, StringComparison.OrdinalIgnoreCase)) continue; + + foreach (var element in doc.Root.Descendants()) + { + var entity = element.Attribute("Entity")?.Value; + if (!string.IsNullOrWhiteSpace(entity)) yield return entity; + + var url = element.Attribute("Url")?.Value; + if (string.IsNullOrWhiteSpace(url)) continue; + + foreach (var part in url.Split('&', '?')) + { + if (part.StartsWith("etn=", StringComparison.OrdinalIgnoreCase) && part.Length > 4) + { + yield return part[4..]; + } + } + } + } + } + + /// + /// Anchors on the component folder rather than on "Declarations": some modules keep + /// their declarations under "CDS" instead, and a glob anchored on either name misses + /// the other. + /// + private static IEnumerable FilesUnder(IEnumerable searchRoots, string folderName, string pattern) + { + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + + foreach (var root in searchRoots.Where(Directory.Exists)) + { + foreach (var folder in Directory.EnumerateDirectories(root, folderName, SearchOption.AllDirectories)) + { + foreach (var file in Directory.EnumerateFiles(folder, pattern, SearchOption.AllDirectories)) + { + // Managed and unmanaged copies sit side by side with identical content. + if (seen.Add(Path.GetFullPath(file))) yield return file; + } + } + } + } + + private static XDocument? Load(string file) + { + try + { + return XDocument.Load(file); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Could not read app module {File}; skipping it.", file); + return null; + } + } +} diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs index 0a374bc6..8f3fbad9 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs @@ -13,6 +13,8 @@ using TALXIS.CLI.Features.Data.DataModelConverter.Translators; using TALXIS.CLI.Logging; +using TALXIS.CLI.Features.Data.DataModelConverter.AppScope; + namespace TALXIS.CLI.Features.Data.DataModelConverter; public class DataModelConverterService @@ -44,6 +46,15 @@ public static void ConvertModel(string inputPath, string targetFormat, string ou /// attribute differently. /// public static void ConvertModel(List inputPaths, string targetFormat, string outputFilePath) + => ConvertModel(inputPaths, targetFormat, outputFilePath, null, null); + + /// + /// Converts one or more inputs into a single model, optionally narrowed to the tables a + /// model-driven app is built on. is where app modules + /// are looked for; apps and entity schema live in different modules, so this is usually + /// a repository root rather than a declarations folder. + /// + public static void ConvertModel(List inputPaths, string targetFormat, string outputFilePath, string? appUniqueName, List? appSearchRoots) { if (!SupportedFormats.Contains(targetFormat.ToLower())) throw new ArgumentException($"Unsupported target format '{targetFormat}'. Supported formats are: {string.Join(", ", SupportedFormats)}."); @@ -71,7 +82,13 @@ public static void ConvertModel(List inputPaths, string targetFormat, st } } - var parsedModel = ParseModules(modules); + ResolvedAppScope? appScope = null; + if (!string.IsNullOrWhiteSpace(appUniqueName)) + { + appScope = AppScopeResolver.Resolve(appSearchRoots is { Count: > 0 } ? appSearchRoots : inputPaths, appUniqueName); + } + + var parsedModel = ParseModules(modules, appScope); var resultString = targetFormat.ToLower() switch { @@ -305,6 +322,23 @@ private static string ModuleNameFor(string declarationsFolder) return string.Join('/', segments.TakeLast(3)); } + /// + /// Finds every declarations folder beneath a root, by looking for the entity + /// declarations themselves rather than for a folder name -- modules keep them under + /// "Declarations" or, in older ones, "CDS". + /// + public static List DiscoverDeclarationFolders(string root) + { + if (!Directory.Exists(root)) throw new DirectoryNotFoundException($"Root '{root}' does not exist."); + + return [.. Directory.EnumerateFiles(root, "Entity.xml", SearchOption.AllDirectories) + .Select(f => Path.GetDirectoryName(Path.GetDirectoryName(Path.GetDirectoryName(f)))) + .Where(d => !string.IsNullOrEmpty(d)) + .Select(d => Path.GetFullPath(d!)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .OrderBy(d => d, StringComparer.OrdinalIgnoreCase)]; + } + private static Module ParseFolderIntoModule(string folderPath) { Module module = new() { ModuleName = ModuleNameFor(folderPath) }; @@ -416,6 +450,9 @@ public static ParsedModel ParseModel(List base64solution) } public static ParsedModel ParseModules(List modules) + => ParseModules(modules, null); + + public static ParsedModel ParseModules(List modules, ResolvedAppScope? appScope) { List
EntityTables = ParseEntities(modules); @@ -443,7 +480,27 @@ row.RowType is (RowType.Picklist or RowType.Multiselectoptionset or RowType.Stat entity.SetName = entity.LogicalName; } - List EntityRelationships = ParseRelationships(modules, EntityTables); + // Before relationships: a table dropped here must not reappear as a stub created + // for a relationship that pointed at it. + if (appScope != null) + { + AppScopeFilter.ApplyTableScope(EntityTables, appScope); + } + + List EntityRelationships = ParseRelationships(modules, EntityTables, appScope); + + if (appScope != null) + { + // Option sets belonging to tables the scope removed would otherwise still be + // emitted, leaving more enum declarations in the output than columns using them. + var referenced = EntityTables + .SelectMany(t => t.Rows) + .Select(r => r.OptionSetName) + .Where(n => !string.IsNullOrEmpty(n)) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + EntityOptionSets.RemoveAll(o => !referenced.Contains(o.LocalizedName)); + } return new ParsedModel() { @@ -455,6 +512,20 @@ row.RowType is (RowType.Picklist or RowType.Multiselectoptionset or RowType.Stat } public static List ParseRelationships(List modules, List
EntityTables) + => ParseRelationships(modules, EntityTables, null); + + private static bool IsInAppScope(XElement relationship, ResolvedAppScope appScope) + { + if (relationship.Element("EntityRelationshipType")?.Value == "ManyToMany") + { + return appScope.TableLogicalNames.Contains(relationship.Element("FirstEntityName")?.Value ?? string.Empty) + || appScope.TableLogicalNames.Contains(relationship.Element("SecondEntityName")?.Value ?? string.Empty); + } + + return appScope.TableLogicalNames.Contains(relationship.Element("ReferencingEntityName")?.Value ?? string.Empty); + } + + public static List ParseRelationships(List modules, List
EntityTables, ResolvedAppScope? appScope) { List EntityRelationships = new(); @@ -465,6 +536,14 @@ public static List ParseRelationships(List modules, List +/// Resolving which tables a model-driven app is built on, from source alone. The shapes +/// exercised here are the ones real repositories actually contain: declarations under +/// "CDS" as well as "Declarations", one app declared across several files, and a folder +/// whose name differs in case from the UniqueName inside it. +/// +public class AppScopeTests : IDisposable +{ + private readonly string _root = Path.Combine(Path.GetTempPath(), "txc-app-" + Path.GetRandomFileName()); + + public void Dispose() + { + if (Directory.Exists(_root)) Directory.Delete(_root, recursive: true); + GC.SuppressFinalize(this); + } + + private string WriteAppModule(string folderName, string uniqueName, string componentsXml, string declarationsFolder = "Declarations") + { + var dir = Path.Combine(_root, "module", declarationsFolder, "AppModules", folderName); + Directory.CreateDirectory(dir); + var file = Path.Combine(dir, "AppModule_managed.xml"); + File.WriteAllText(file, $""" + + {uniqueName} + + {componentsXml} + + + """); + return file; + } + + private static string Component(string type, string schemaName) => + $""""""; + + // ---- resolution ------------------------------------------------------------------ + + [Fact] + public void OnlyEntityComponentsContributeTables() + { + WriteAppModule("contoso_app", "contoso_app", + Component("1", "contoso_thing") + + Component("26", "some_view") // saved query + + Component("60", "some_form") // system form + + Component("62", "contoso_app") // the app's own sitemap + + Component("1", "account")); + + var scope = AppScopeResolver.Resolve([_root], "contoso_app"); + + Assert.Equal(new[] { "account", "contoso_thing" }, scope.TableLogicalNames.OrderBy(x => x)); + } + + [Fact] + public void DeclarationsUnderCdsFolder_AreStillFound() + { + // Older modules keep their declarations under "CDS" rather than "Declarations"; + // a search anchored on either folder name misses the other. + WriteAppModule("contoso_app", "contoso_app", Component("1", "contoso_thing"), declarationsFolder: "CDS"); + + var scope = AppScopeResolver.Resolve([_root], "contoso_app"); + + Assert.Contains("contoso_thing", scope.TableLogicalNames); + } + + [Fact] + public void OneAppDeclaredAcrossSeveralFiles_UnionsItsComponents() + { + // A second area can contribute components to an app it does not own; the app's real + // component set is the union of every file that declares it. + WriteAppModule("contoso_app", "contoso_app", Component("1", "contoso_first")); + var second = Path.Combine(_root, "other", "Declarations", "AppModules", "contoso_app"); + Directory.CreateDirectory(second); + File.WriteAllText(Path.Combine(second, "AppModule_managed.xml"), """ + + contoso_app + + + + + """); + + var scope = AppScopeResolver.Resolve([_root], "contoso_app"); + + Assert.Contains("contoso_first", scope.TableLogicalNames); + Assert.Contains("contoso_second", scope.TableLogicalNames); + Assert.Equal(2, scope.SourceFiles.Count); + } + + [Fact] + public void IdentityComesFromFileContent_NotTheFolderName() + { + // The folder is only a locator, and its casing can differ from the declared name. + WriteAppModule("Contoso_App", "contoso_app", Component("1", "contoso_thing")); + + var scope = AppScopeResolver.Resolve([_root], "contoso_app"); + + Assert.Contains("contoso_thing", scope.TableLogicalNames); + } + + [Fact] + public void SiteMapEntities_AreIncluded_FromBothTheAttributeAndTheUrl() + { + WriteAppModule("contoso_app", "contoso_app", Component("1", "contoso_thing")); + var dir = Path.Combine(_root, "module", "Declarations", "AppModuleSiteMaps", "contoso_app"); + Directory.CreateDirectory(dir); + File.WriteAllText(Path.Combine(dir, "AppModuleSiteMap_managed.xml"), """ + + contoso_app + + + + """); + + var scope = AppScopeResolver.Resolve([_root], "contoso_app"); + + Assert.Contains("contoso_viaattribute", scope.TableLogicalNames); + Assert.Contains("contoso_viaurl", scope.TableLogicalNames); + } + + [Fact] + public void UnknownApp_FailsWithTheNamesItDidFind() + { + WriteAppModule("contoso_app", "contoso_app", Component("1", "contoso_thing")); + + var ex = Assert.Throws(() => AppScopeResolver.Resolve([_root], "contoso_typo")); + + Assert.Contains("contoso_app", ex.Message); + } + + // ---- filtering ------------------------------------------------------------------- + + private static XElement Entity(string logicalName) => + XElement.Parse($""" + + {logicalName} + + primarykey + lookup + + + """); + + private static XElement OneToMany(string child, string attr, string parent) => + XElement.Parse($""" + + OneToMany + {child} + {parent} + {attr} + + """); + + [Fact] + public void TablesOutsideTheApp_AreDropped_AndDoNotReturnAsRelationshipStubs() + { + // The ordering that matters: filtering after relationships were built would let a + // relationship among the dropped tables synthesise them straight back as stubs. + var module = new Model.Module { ModuleName = "test" }; + module.entities.AddRange([Entity("contoso_inapp"), Entity("contoso_elsewhere"), Entity("contoso_alsoelsewhere")]); + module.relationships.Add(OneToMany("contoso_elsewhere", "contoso_elsewhere_lookup", "contoso_alsoelsewhere")); + + var scope = new ResolvedAppScope { UniqueName = "contoso_app" }; + scope.TableLogicalNames.Add("contoso_inapp"); + + var model = DataModelConverterService.ParseModules([module], scope); + + Assert.Contains(model.tables, t => t.LogicalName == "contoso_inapp"); + Assert.DoesNotContain(model.tables, t => t.LogicalName == "contoso_elsewhere"); + Assert.DoesNotContain(model.tables, t => t.LogicalName == "contoso_alsoelsewhere"); + } + + [Fact] + public void ALookupOutOfTheApp_StillTerminates_SoTheEdgeIsNotLost() + { + var module = new Model.Module { ModuleName = "test" }; + module.entities.AddRange([Entity("contoso_inapp"), Entity("contoso_outside")]); + module.relationships.Add(OneToMany("contoso_inapp", "contoso_inapp_lookup", "contoso_outside")); + + var scope = new ResolvedAppScope { UniqueName = "contoso_app" }; + scope.TableLogicalNames.Add("contoso_inapp"); + + var model = DataModelConverterService.ParseModules([module], scope); + + Assert.Contains(model.relationships, r => r.LeftSideTable?.LogicalName == "contoso_inapp"); + Assert.Contains(model.tables, t => t.LogicalName == "contoso_outside" && t.Type == Model.TableType.NotInSolution); + } + + [Fact] + public void WithoutAnAppScope_NothingIsFiltered() + { + var module = new Model.Module { ModuleName = "test" }; + module.entities.AddRange([Entity("contoso_a"), Entity("contoso_b")]); + + var model = DataModelConverterService.ParseModules([module]); + + Assert.Contains(model.tables, t => t.LogicalName == "contoso_a"); + Assert.Contains(model.tables, t => t.LogicalName == "contoso_b"); + } +} From 708e208c2e29ce8f3ad9cdff73039144fd56d67a Mon Sep 17 00:00:00 2001 From: David Hudec Date: Wed, 2 Sep 2026 08:09:51 +0200 Subject: [PATCH 10/10] feat(data): narrow an app's model to how that app uses it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--detail full|minimal`. `full` is everything the inputs declare; `minimal` shows how the app was built. Supersedes the `--columns-used` / `--scan-code` pair this replaces, which is why they never appear: a reference resolved by name against one global set keeps a column on every table declaring that name. That is a fair approximation for an author's column, which one table declares, and useless for a platform column, whose name is identical org-wide — one view showing createdon kept it on 19 of 19 tables in a real app. Columns. A reference is credited to the table whose folder the artefact sits under. An artefact outside one — an app module, a sitemap, a plug-in source — is credited to no table and matched by name alone, so it keeps an author's column and cannot rescue a platform one. Without that gate the whole problem returns through the code scan: measured, 226 of 252 audit columns survived. An author's column carries the publisher prefix from the module's own Solution.xml and the platform's carries none, which is the whole discriminator; with no prefixes to check against nothing is called the platform's, rather than narrowing on no evidence. Dropped whatever refers to them: logical columns, business process flow bookkeeping, and the base-currency twin, which needs a name pairing because nothing in the metadata separates it from the column it shadows. A primary key, `statecode`, `statuscode` and any column an edge depends on are never dropped — `EDMXTranslator` and `SQLTranslator` read a relationship's endpoints with no null check, so removing one crashes rather than narrows. Tables. An N:N is admitted only when both its tables are in the app. The gate is read before the branch that builds the intersect, so the edge and both stubs are withheld together rather than leaving a dangling end, which is why that branch's unguarded dereferences need no restructuring. A stub for a table an input does declare gets a new `TableType.NotInApp` instead of `NotInSolution`, because the colour is chosen from the type and that is where the distinction belongs. It applies wherever `--app` is given, not only under `minimal`: the red "not in the solution" was untrue for 13 of 14 stubs in one real app at either detail level. One `AppScopeTests` assertion changes with it. Reporting. Every dropped column in one warning is unreadable at this volume, so `DroppedColumn` carries a reason and the command writes counts per reason through `WriteData` — which also clears the TXC028 it was raising by returning a status envelope from a read-only command. Also folds three duplications out of code earlier commits in this stack introduce: `ColourFor` hand-rolled FNV-1a where `SHA256.HashData` is one line, and the self-referencing check ran twice in the N:N branch with its result read on the line after it was assigned. Four things this deliberately does not do, each measured against both apps and found to change nothing: - Walk a view's FetchXML to credit a linked table's columns to that table. Zero columns, because the token scan already credits the file to its own table and those names appear in that table's own artefacts too. - Read a workflow's PrimaryEntity to credit its bundle. One column, and that column was `owningbusinessunit` — so not doing it is the better output. - Read `IsCustomField`. Zero columns; the publisher prefix already answers it. - Classify the columns of a table with no artefacts of its own instead of narrowing them. Zero columns. Such tables are reported by name instead. Measured, project and product repositories at develop: app detail tables columns refs enums audit ntg_projectmanagement full 70 903 124 91 252 ntg_projectmanagement minimal 46 538 94 82 42 ntg_administration full 36 388 59 29 134 ntg_administration minimal 22 218 39 28 23 Five targets x two apps x two levels: exit 0, DBML parses with @dbml/core, byte-identical on a second run. One table in 38 has no artefacts of its own (`Letter`) and is reported by name. Co-Authored-By: Claude Opus 5 --- .../DataModelConvertCliCommand.cs | 60 ++++- .../AppScope/AppScopeFilter.cs | 12 +- .../AppScope/AppScopeResolver.cs | 23 +- .../AppScope/AttributeReferenceFilter.cs | 249 ++++++++++++++++++ .../AppScope/DroppedColumn.cs | 21 ++ .../DataModelConverterService.cs | 111 ++++++-- .../DataModelConverter/DetailLevel.cs | 14 + .../DataModelConverter/Model/Module.cs | 23 +- .../DataModelConverter/Model/Table.cs | 19 +- .../DataModelConverter/Model/TableRow.cs | 8 +- .../Translators/DBDiagramTranslator.cs | 5 + .../Data/DataModelConverter/AppScopeTests.cs | 5 +- .../DataModelConverter/ColumnScopeTests.cs | 178 +++++++++++++ .../MinimalDetailTableTests.cs | 188 +++++++++++++ .../PerTableColumnScopeTests.cs | 222 ++++++++++++++++ 15 files changed, 1098 insertions(+), 40 deletions(-) create mode 100644 src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AttributeReferenceFilter.cs create mode 100644 src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/DroppedColumn.cs create mode 100644 src/TALXIS.CLI.Features.Data/DataModelConverter/DetailLevel.cs create mode 100644 tests/TALXIS.CLI.Tests/Data/DataModelConverter/ColumnScopeTests.cs create mode 100644 tests/TALXIS.CLI.Tests/Data/DataModelConverter/MinimalDetailTableTests.cs create mode 100644 tests/TALXIS.CLI.Tests/Data/DataModelConverter/PerTableColumnScopeTests.cs diff --git a/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs b/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs index baa08458..1f3b5e0f 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConvertCliCommand.cs @@ -1,6 +1,7 @@ -using DotMake.CommandLine; +using DotMake.CommandLine; using Microsoft.Extensions.Logging; using TALXIS.CLI.Features.Data.DataModelConverter; +using TALXIS.CLI.Features.Data.DataModelConverter.AppScope; using TALXIS.CLI.Core; using TALXIS.CLI.Logging; @@ -39,6 +40,14 @@ public class DataModelConvertCliCommand : TxcLeafCommand )] public string? AppUniqueName { get; set; } + [CliOption( + Name = "--detail", + Description = "How much to emit. 'full' is everything the inputs declare. 'minimal' shows how the app was built: each table keeps only the columns its own forms, views, workflows, sitemap and .cs/.ts sources refer to, platform plumbing is dropped, and an N:N appears only when both its tables belong to the app. A dropped column is one no reference was found for, which is not the same as one that is unused: a name built at runtime cannot be found at all. 'minimal' requires --app, and is not a schema export -- use 'full' to generate SQL or EDMX for tooling.", + AllowedValues = new[] { "full", "minimal" }, + Required = false + )] + public string Detail { get; set; } = "full"; + [CliOption( Name = "--target", Description = "Target format for the conversion.", @@ -57,6 +66,15 @@ public class DataModelConvertCliCommand : TxcLeafCommand protected override Task ExecuteAsync() { + var detail = string.Equals(Detail, "minimal", StringComparison.OrdinalIgnoreCase) + ? DetailLevel.Minimal + : DetailLevel.Full; + + if (detail == DetailLevel.Minimal && string.IsNullOrWhiteSpace(AppUniqueName)) + { + throw new ArgumentException("--detail minimal narrows an app's tables to how that app uses them, so it requires --app."); + } + var inputPaths = new List(InputPaths); foreach (var root in Roots) @@ -92,12 +110,48 @@ protected override Task ExecuteAsync() var extension = TargetFormat!.ToLower() == "plainsql" ? "sql" : TargetFormat.ToLower(); var outputFilePath = Path.Combine(outputDir, $"solution.{extension}"); - DataModelConverterService.ConvertModel(inputPaths, TargetFormat!, outputFilePath, AppUniqueName, appSearchRoots); + var droppedColumns = DataModelConverterService.ConvertModel( + inputPaths, TargetFormat!, outputFilePath, AppUniqueName, appSearchRoots, detail); + + var summary = new ConvertSummary( + outputFilePath, + Detail.ToLowerInvariant(), + droppedColumns.Count, + [.. droppedColumns.GroupBy(c => c.Reason) + .OrderBy(g => g.Key) + .Select(g => new DroppedByReason(g.Key.ToString(), g.Count()))], + droppedColumns); + + // Every dropped column in one warning is unreadable once there are thousands of + // them, so the full list goes to the data channel and text mode gets the counts. + OutputFormatter.WriteData(summary, s => + { + OutputWriter.WriteLine($"Output written to: {s.OutputFile}"); + foreach (var reason in s.DroppedByReason) + { + OutputWriter.WriteLine($" dropped {reason.Count} column(s): {reason.Reason}"); + } + }); - OutputFormatter.WriteResult("succeeded", $"Output written to: {outputFilePath}"); return Task.FromResult(ExitSuccess); } + /// Where the converted model was written. + /// The detail level the conversion ran at. + /// How many columns were left out in total. + /// Counts per reason, which is what a reader needs first. + /// Every dropped column, for a caller that wants to check one. + public sealed record ConvertSummary( + string OutputFile, + string Detail, + int ColumnsDropped, + IReadOnlyList DroppedByReason, + IReadOnlyList DroppedColumns); + + /// Why these columns were left out. + /// How many were left out for that reason. + public sealed record DroppedByReason(string Reason, int Count); + /// /// Walks up for the repository that encloses a directory, so an app can be found /// without the caller naming a root. Falls back to the directory itself. diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeFilter.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeFilter.cs index c22c9b08..9734cb52 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeFilter.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeFilter.cs @@ -13,11 +13,19 @@ public static class AppScopeFilter /// /// Drops tables the app does not declare. Runs before relationships are built, so a - /// dropped table cannot come back as a synthesised stub for a relationship that - /// pointed at it. + /// relationship out of the app cannot bring its referencing table back as a stub. A + /// relationship into a dropped table still stubs the far side deliberately, so + /// a lookup terminates somewhere visible — see + /// for how the diagram tells that apart from a table no input declares. /// public static void ApplyTableScope(List
tables, ResolvedAppScope scope) { + // The one point where every input's declarations are still present. + foreach (var table in tables.Where(t => t.Type == TableType.InSolution)) + { + scope.AllDeclaredTableLogicalNames.Add(table.LogicalName); + } + var removed = tables.RemoveAll(t => t.Type == TableType.InSolution && !scope.TableLogicalNames.Contains(t.LogicalName)); diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeResolver.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeResolver.cs index e923bcd1..274385cc 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeResolver.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AppScopeResolver.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -19,6 +19,27 @@ public class ResolvedAppScope /// Every file that contributed, for reporting which sources were read. public List SourceFiles { get; } = []; + + /// How much metadata to emit. narrows the + /// columns of each table to what its own artefacts refer to. + public DetailLevel Detail { get; set; } = DetailLevel.Full; + + /// Where to look for references. Usually repository roots. + public List SearchRoots { get; set; } = []; + + /// Publisher customization prefixes of the inputs. A column carrying one was + /// created by an author, whatever its metadata says. + public HashSet AuthorPrefixes { get; } = new(StringComparer.OrdinalIgnoreCase); + + /// + /// Every table any input declares, captured before app scoping removes the ones this + /// app does not use. A stub for a table in here is outside the app, not outside the + /// solution — a distinction the diagram would otherwise get wrong. + /// + public HashSet AllDeclaredTableLogicalNames { get; } = new(StringComparer.OrdinalIgnoreCase); + + /// Columns removed, so the run can report them rather than drop them quietly. + public List DroppedColumns { get; } = []; } /// diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AttributeReferenceFilter.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AttributeReferenceFilter.cs new file mode 100644 index 00000000..00cad5d7 --- /dev/null +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/AttributeReferenceFilter.cs @@ -0,0 +1,249 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text.RegularExpressions; +using Microsoft.Extensions.Logging; +using TALXIS.CLI.Features.Data.DataModelConverter.Model; +using TALXIS.CLI.Logging; + +namespace TALXIS.CLI.Features.Data.DataModelConverter.AppScope; + +/// +/// Narrows each table to the columns its own artefacts mention. +/// +/// A reference belongs to the table whose artefact made it. Resolving one by name alone +/// keeps a column on every table that declares that name, which is a fair approximation for +/// an author's column — one table usually declares it — and useless for a platform column, +/// whose name is identical on every table in the org. One view showing createdon kept it on +/// nineteen tables in a real app. +/// +public static class AttributeReferenceFilter +{ + private static readonly ILogger _logger = TxcLoggerFactory.CreateLogger(nameof(AttributeReferenceFilter)); + + /// Identifier-shaped tokens; column logical names are always of this shape. + private static readonly Regex TokenPattern = new(@"[A-Za-z_][A-Za-z0-9_]{2,}", RegexOptions.Compiled); + + /// + /// Folders whose contents reference columns. Entity declarations are deliberately not + /// among them: an entity declares its own columns, so scanning one would report every + /// column as referenced by itself. + /// + private static readonly string[] ReferencingFolders = + ["FormXml", "SavedQueries", "Workflows", "Visualizations", "AppModuleSiteMaps", "AppModules"]; + + private static readonly string[] CodeExtensions = [".cs", ".ts", ".js"]; + + /// Business process flow bookkeeping. No metadata flag separates these from an + /// author's columns, so they are named. + private static readonly string[] ProcessFlowColumns = ["processid", "stageid", "traversedpath"]; + + private const string BaseCurrencySuffix = "_base"; + + public static void Apply(List
tables, List relationships, ResolvedAppScope scope) + { + var references = CollectReferences(scope.SearchRoots); + + // Computed before anything is dropped. The translators read + // Relationship.LeftSideRow/RighSideRow without a null check, so removing a row an + // edge points at turns a narrower diagram into a crash on the sql and edmx targets. + var loadBearing = new HashSet(); + foreach (var relationship in relationships) + { + if (relationship.LeftSideRow != null) loadBearing.Add(relationship.LeftSideRow); + if (relationship.RighSideRow != null) loadBearing.Add(relationship.RighSideRow); + } + + var withoutArtefacts = new List(); + + foreach (var table in tables.Where(t => t.Type == TableType.InSolution)) + { + if (!references.HasOwn(table.LogicalName)) + { + withoutArtefacts.Add(table.LogicalName); + } + + foreach (var row in table.Rows.ToList()) + { + // A primary key and a state model describe the table whatever refers to + // them, and an edge's own column cannot go without crashing a translator. + if (row.RowType is RowType.Primarykey or RowType.State or RowType.Status) continue; + if (loadBearing.Contains(row)) continue; + + var reason = ReasonToDrop(table, row, references, scope.AuthorPrefixes); + if (reason == null) continue; + + table.Rows.Remove(row); + scope.DroppedColumns.Add(new DroppedColumn(table.LogicalName, row.Name, reason.Value)); + } + } + + _logger.LogInformation( + "Narrowed {Tables} table(s) to the columns their own forms, views, workflows, sitemaps and .cs/.ts sources refer to; " + + "dropped {Dropped}. A dropped column is one no reference was found for, which is not the same as one that is unused.", + tables.Count(t => t.Type == TableType.InSolution), scope.DroppedColumns.Count); + + if (withoutArtefacts.Count > 0) + { + // Nothing referenced these tables' columns because nothing could: they have no + // forms or views of their own, so only their keys and relationships survive. + _logger.LogWarning( + "{Count} table(s) have no forms, views or charts of their own, so only their keys and relationships remain: {Tables}.", + withoutArtefacts.Count, string.Join(", ", withoutArtefacts.OrderBy(x => x, StringComparer.Ordinal))); + } + } + + private static DropReason? ReasonToDrop(Table table, TableRow row, ReferenceIndex references, HashSet authorPrefixes) + { + // Checked first, so a column dropped as plumbing is not reported as unreferenced. + if (IsPlatformPlumbing(table, row)) return DropReason.PlatformPlumbing; + + if (references.OwnedBy(table.LogicalName, row.Name)) return null; + + // An artefact belonging to no single table — an app module, a sitemap, a plug-in — + // can only be matched by name, and a platform column's name is the same on every + // table in the org. Letting one rescue createdon puts it back on all nineteen + // tables, which is the defect this filter exists to remove. An author's column is + // named once, so the same evidence is worth trusting there: measured at 30 columns + // across two real apps. + if (!IsPlatformColumn(row, authorPrefixes) && references.Unattributed(row.Name)) return null; + + return DropReason.NoReferenceFound; + } + + /// + /// Platform plumbing a reader of the model never needs, even where something refers to it. + /// + private static bool IsPlatformPlumbing(Table table, TableRow row) + => row.IsLogical == true + || ProcessFlowColumns.Contains(row.Name, StringComparer.OrdinalIgnoreCase) + || IsBaseCurrencyTwin(table, row); + + /// + /// The shadow the platform maintains in the base currency beside an author's money + /// column. Nothing in the metadata separates the two, so this is a name pairing. + /// + private static bool IsBaseCurrencyTwin(Table table, TableRow row) + => row.RowType == RowType.Money + && row.Name.EndsWith(BaseCurrencySuffix, StringComparison.OrdinalIgnoreCase) + && table.Rows.Any(other => other.RowType == RowType.Money + && string.Equals(other.Name, row.Name[..^BaseCurrencySuffix.Length], StringComparison.OrdinalIgnoreCase)); + + /// + /// Whether the platform created a column rather than an author. Dataverse gives an + /// author's column the publisher's prefix and its own columns none, which is the whole + /// discriminator. With no prefixes to check against, nothing is called the platform's + /// rather than narrowing the output on no evidence. + /// + private static bool IsPlatformColumn(TableRow row, HashSet authorPrefixes) + => authorPrefixes.Count > 0 + && !authorPrefixes.Any(prefix => row.Name.StartsWith(prefix + "_", StringComparison.OrdinalIgnoreCase)); + + /// + /// Every token in every referencing artefact, credited to the table whose folder it sits + /// under. An artefact outside one — an app module, a sitemap, a plug-in source — is + /// credited to no table and matched by name alone. + /// + private static ReferenceIndex CollectReferences(IEnumerable searchRoots) + { + var index = new ReferenceIndex(); + + foreach (var root in searchRoots.Where(Directory.Exists).Select(Path.GetFullPath).Distinct()) + { + foreach (var file in Directory.EnumerateFiles(root, "*", SearchOption.AllDirectories)) + { + if (!ShouldScan(file)) continue; + + var owner = OwnerFromPath(file); + + try + { + foreach (Match match in TokenPattern.Matches(File.ReadAllText(file))) + { + index.Add(owner, match.Value); + } + } + catch (IOException ex) + { + _logger.LogWarning(ex, "Could not read {File} while looking for column references.", file); + } + } + } + + return index; + } + + /// The segment naming the entity whose folder this artefact lives under, or null + /// for one that sits outside any and therefore speaks for many tables. + private static string? OwnerFromPath(string file) + { + var segments = (Path.GetDirectoryName(file) ?? string.Empty) + .Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + + for (var i = 0; i < segments.Length - 1; i++) + { + if (string.Equals(segments[i], "Entities", StringComparison.OrdinalIgnoreCase)) + { + return segments[i + 1]; + } + } + + return null; + } + + private static bool ShouldScan(string file) + { + // Plug-in and script sources sit outside the declarations, so a reference from one + // is invisible without reading them: measured at 14 columns across two real apps. + if (CodeExtensions.Contains(Path.GetExtension(file), StringComparer.OrdinalIgnoreCase)) + { + return true; + } + + // Entity.xml declares columns rather than referencing them. + if (string.Equals(Path.GetFileName(file), "Entity.xml", StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + var directory = Path.GetDirectoryName(file) ?? string.Empty; + + return ReferencingFolders.Any(folder => + directory.Contains(Path.DirectorySeparatorChar + folder + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase) + || directory.EndsWith(Path.DirectorySeparatorChar + folder, StringComparison.OrdinalIgnoreCase)); + } + + /// Which columns each table's own artefacts refer to, plus the references that + /// belong to no single table and therefore count for all of them. + private sealed class ReferenceIndex + { + private readonly Dictionary> _byTable = new(StringComparer.OrdinalIgnoreCase); + private readonly HashSet _unattributed = new(StringComparer.OrdinalIgnoreCase); + + public void Add(string? table, string token) + { + if (table == null) + { + _unattributed.Add(token); + return; + } + + if (!_byTable.TryGetValue(table, out var tokens)) + { + _byTable[table] = tokens = new HashSet(StringComparer.OrdinalIgnoreCase); + } + + tokens.Add(token); + } + + public bool HasOwn(string table) => _byTable.ContainsKey(table); + + /// An artefact of this table names this column. + public bool OwnedBy(string table, string column) + => _byTable.TryGetValue(table, out var tokens) && tokens.Contains(column); + + /// Something names this column, but nothing says which table it meant. + public bool Unattributed(string column) => _unattributed.Contains(column); + } +} diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/DroppedColumn.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/DroppedColumn.cs new file mode 100644 index 00000000..145603f8 --- /dev/null +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/AppScope/DroppedColumn.cs @@ -0,0 +1,21 @@ +namespace TALXIS.CLI.Features.Data.DataModelConverter.AppScope; + +/// Why a column was left out, so a reader can tell a judgement from an absence. +public enum DropReason +{ + /// Nothing belonging to the column's own table referred to it. + NoReferenceFound = 0, + + /// Platform plumbing a reader never needs, even where something refers + /// to it. + PlatformPlumbing = 1 +} + +/// One column left out of the conversion. +/// Logical name of the table the column was declared on. +/// The column's own name. +/// Why it was left out. +public sealed record DroppedColumn(string Table, string Column, DropReason Reason) +{ + public override string ToString() => $"{Table}.{Column}"; +} diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs index 8f3fbad9..b40a9d82 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/DataModelConverterService.cs @@ -55,6 +55,14 @@ public static void ConvertModel(List inputPaths, string targetFormat, st /// a repository root rather than a declarations folder. /// public static void ConvertModel(List inputPaths, string targetFormat, string outputFilePath, string? appUniqueName, List? appSearchRoots) + => ConvertModel(inputPaths, targetFormat, outputFilePath, appUniqueName, appSearchRoots, DetailLevel.Full); + + /// + /// of narrows each table to + /// the columns its own artefacts refer to, and drops the platform plumbing a reader + /// never needs. + /// + public static IReadOnlyList ConvertModel(List inputPaths, string targetFormat, string outputFilePath, string? appUniqueName, List? appSearchRoots, DetailLevel detail) { if (!SupportedFormats.Contains(targetFormat.ToLower())) throw new ArgumentException($"Unsupported target format '{targetFormat}'. Supported formats are: {string.Join(", ", SupportedFormats)}."); @@ -85,7 +93,15 @@ public static void ConvertModel(List inputPaths, string targetFormat, st ResolvedAppScope? appScope = null; if (!string.IsNullOrWhiteSpace(appUniqueName)) { - appScope = AppScopeResolver.Resolve(appSearchRoots is { Count: > 0 } ? appSearchRoots : inputPaths, appUniqueName); + var roots = appSearchRoots is { Count: > 0 } ? appSearchRoots : inputPaths; + appScope = AppScopeResolver.Resolve(roots, appUniqueName); + appScope.Detail = detail; + appScope.SearchRoots = [.. roots]; + + foreach (var prefix in modules.Select(m => m.CustomizationPrefix).Where(p => !string.IsNullOrWhiteSpace(p))) + { + appScope.AuthorPrefixes.Add(prefix!); + } } var parsedModel = ParseModules(modules, appScope); @@ -99,8 +115,12 @@ public static void ConvertModel(List inputPaths, string targetFormat, st _ => ConvertToDBML(parsedModel) }; - using var writer = new StreamWriter(outputFilePath); - writer.Write(resultString); + using (var writer = new StreamWriter(outputFilePath)) + { + writer.Write(resultString); + } + + return appScope?.DroppedColumns ?? []; } /// @@ -341,7 +361,11 @@ public static List DiscoverDeclarationFolders(string root) private static Module ParseFolderIntoModule(string folderPath) { - Module module = new() { ModuleName = ModuleNameFor(folderPath) }; + Module module = new() + { + ModuleName = ModuleNameFor(folderPath), + CustomizationPrefix = PrefixFromFolder(folderPath) + }; // Get files named Entity.xml in subfolders // Ordered: Directory.GetFiles gives no ordering guarantee, so without this the @@ -421,9 +445,37 @@ private static Module ParseZipIntoModule(string base64solution) throw new FileNotFoundException("The solution archive does not contain the required customizations.xml or solution.xml files."); } + var manifest = XDocument.Load(solutionxml.Open()); + return new Module( - XDocument.Load(solutionxml.Open()).Descendants().First(x => x.Name == "UniqueName").Value, - XDocument.Load(customizationsxml.Open())); + manifest.Descendants().First(x => x.Name == "UniqueName").Value, + XDocument.Load(customizationsxml.Open())) + { + CustomizationPrefix = Module.PrefixFrom(manifest) + }; + } + + /// + /// A declarations folder keeps its manifest at Other/Solution.xml, beside the + /// relationships this converter already reads. + /// + private static string? PrefixFromFolder(string folderPath) + { + var manifestPath = Path.Combine(folderPath, "Other", "Solution.xml"); + if (!File.Exists(manifestPath)) + { + return null; + } + + try + { + return Module.PrefixFrom(XDocument.Load(manifestPath)); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Could not read the publisher prefix from {File}", manifestPath); + return null; + } } public static ParsedModel ParseModel(string? base64solution) @@ -489,6 +541,13 @@ row.RowType is (RowType.Picklist or RowType.Multiselectoptionset or RowType.Stat List EntityRelationships = ParseRelationships(modules, EntityTables, appScope); + if (appScope is { Detail: DetailLevel.Minimal }) + { + // After relationships: the set of columns an edge depends on is only knowable + // once they exist. + AttributeReferenceFilter.Apply(EntityTables, EntityRelationships, appScope); + } + if (appScope != null) { // Option sets belonging to tables the scope removed would otherwise still be @@ -514,12 +573,30 @@ row.RowType is (RowType.Picklist or RowType.Multiselectoptionset or RowType.Stat public static List ParseRelationships(List modules, List
EntityTables) => ParseRelationships(modules, EntityTables, null); + /// + /// Which kind of stub stands in for a table an edge points at. One the inputs do declare + /// was removed by app scoping; without app scoping every stub is genuinely absent. + /// + private static TableType StubTypeFor(ResolvedAppScope? appScope, string logicalName) + => appScope?.AllDeclaredTableLogicalNames.Contains(logicalName) == true + ? TableType.NotInApp + : TableType.NotInSolution; + private static bool IsInAppScope(XElement relationship, ResolvedAppScope appScope) { if (relationship.Element("EntityRelationshipType")?.Value == "ManyToMany") { - return appScope.TableLogicalNames.Contains(relationship.Element("FirstEntityName")?.Value ?? string.Empty) - || appScope.TableLogicalNames.Contains(relationship.Element("SecondEntityName")?.Value ?? string.Empty); + var firstInScope = appScope.TableLogicalNames.Contains(relationship.Element("FirstEntityName")?.Value ?? string.Empty); + var secondInScope = appScope.TableLogicalNames.Contains(relationship.Element("SecondEntityName")?.Value ?? string.Empty); + + // An N:N is part of an app's own design only when both its tables are. Admitting + // it on one side drags a shared table's whole association network in: systemuser + // alone contributed nine intersects and four far-side stubs to one real app. + // This gate is read before the branch that builds those tables, so tightening it + // withholds the tables and the edge together rather than leaving a dangling end. + return appScope.Detail == DetailLevel.Minimal + ? firstInScope && secondInScope + : firstInScope || secondInScope; } return appScope.TableLogicalNames.Contains(relationship.Element("ReferencingEntityName")?.Value ?? string.Empty); @@ -550,14 +627,14 @@ public static List ParseRelationships(List modules, List ParseRelationships(List modules, List ParseRelationships(List modules, List ParseRelationships(List modules, List ParseRelationships(List modules, ListHow much of a solution's metadata the conversion emits. +public enum DetailLevel +{ + /// Everything the inputs declare. + Full = 0, + + /// + /// Only what shows how the app was built: tables the app is built on, and the + /// columns something belonging to those tables refers to. Not a schema export. + /// + Minimal = 1 +} diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs index c69992ab..3ed4cb17 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Module.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Security.Cryptography; using System.Text; using System.Threading.Tasks; using System.Xml.Linq; @@ -21,9 +22,19 @@ public Module(string module, XDocument xml) optionsets = XmlDoc.Descendants().Where(x => x.Name == "optionset").ToList(); } + /// Reads the publisher prefix out of a solution manifest, from either a folder's + /// Other/Solution.xml or an archive's solution.xml. + public static string? PrefixFrom(XDocument solutionManifest) => + solutionManifest.Descendants().FirstOrDefault(x => x.Name == "CustomizationPrefix")?.Value; + public string ModuleName { get; set; } = ""; public XDocument XmlDoc { get; set; } = new XDocument(); + /// The publisher prefix this module's own columns carry, from its Solution.xml. + /// Ground truth for telling an author's column from a platform one, which the + /// per-attribute metadata alone gets wrong on primary keys and name fields. + public string? CustomizationPrefix { get; set; } + public List entities = []; public List relationships = []; public List optionsets = []; @@ -37,16 +48,8 @@ public Module(string module, XDocument xml) /// Derives the module colour from its name so the same input always converts to the /// same bytes. A random colour made every conversion a spurious diff, which meant a /// generated diagram could not be committed or compared across a model change. - /// FNV-1a rather than string.GetHashCode, which is randomised per process on .NET Core. + /// Not string.GetHashCode, which is randomised per process on .NET Core. /// private static string ColourFor(string moduleName) - { - uint hash = 2166136261; - foreach (var c in moduleName ?? string.Empty) - { - hash ^= c; - hash *= 16777619; - } - return string.Format("#{0:X6}", hash & 0xFFFFFF); - } + => "#" + Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(moduleName ?? string.Empty)))[..6]; } diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Table.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Table.cs index 7241b512..337d0034 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Table.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/Table.cs @@ -1,4 +1,4 @@ -using TALXIS.CLI.Logging; +using TALXIS.CLI.Logging; using Microsoft.Extensions.Logging; using DocumentFormat.OpenXml.Vml.Office; using System.Text.Json.Serialization; @@ -17,7 +17,14 @@ public enum TableType { InSolution = 0, NotInSolution = 1, - ConnectionTable = 2 + ConnectionTable = 2, + + /// + /// A stub for a table an input does declare, which is only a stub because app scoping + /// dropped it. Without this a diagram marks most of its stubs as missing from the + /// solution, which is untrue: 13 of 14 in one real app were declared in the same inputs. + /// + NotInApp = 3 } public class Table @@ -69,8 +76,14 @@ public void ParseMultipleRowsFromXml(List xElements) if (existing == null) { Rows.Add(row); + continue; } - else if (existing.RowType != row.RowType) + + // First non-null wins rather than first input: export styles differ in whether + // they emit this at all, so a later, more complete declaration still counts. + existing.IsLogical ??= row.IsLogical; + + if (existing.RowType != row.RowType) { // Several modules extending one shared table is the normal case for a // layered product, so a divergent declaration warns rather than aborting. diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/TableRow.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/TableRow.cs index 70aa4f3a..f3007bde 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/TableRow.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/Model/TableRow.cs @@ -25,6 +25,11 @@ public TableRow(string name, RowType rowType) public string OptionSetName { get; set; } public RowType RowType { get; set; } + /// Whether the platform computes this column rather than storing it, from the + /// declaration's own IsLogical. OwningUser and OwningTeam are the + /// common cases. + public bool? IsLogical { get; set; } + internal static TableRow? ParseXElement(XElement attribute) { string optionsetName = string.Empty; @@ -115,7 +120,8 @@ public TableRow(string name, RowType rowType) return new TableRow(attribute.Attribute("PhysicalName").Value.ToLower(), rowType) { MaxLenght = maxLength, - OptionSetName = optionsetName + OptionSetName = optionsetName, + IsLogical = attribute.Element("IsLogical") is { } isLogical ? isLogical.Value == "1" : null }; } diff --git a/src/TALXIS.CLI.Features.Data/DataModelConverter/Translators/DBDiagramTranslator.cs b/src/TALXIS.CLI.Features.Data/DataModelConverter/Translators/DBDiagramTranslator.cs index 8b7f4382..2340b966 100644 --- a/src/TALXIS.CLI.Features.Data/DataModelConverter/Translators/DBDiagramTranslator.cs +++ b/src/TALXIS.CLI.Features.Data/DataModelConverter/Translators/DBDiagramTranslator.cs @@ -27,6 +27,11 @@ public static string ToDbDiagramNotation(this Table table) case TableType.NotInSolution: result += "[headercolor: #c0392b] "; break; + case TableType.NotInApp: + // Grey, not the red of a table nothing declares: this one is in the + // solution, just not in this app, which is a different fact for the reader. + result += "[headercolor: #7f8c8d] //declared outside this app \n"; + break; case TableType.ConnectionTable: result += "[headercolor: #27ae60] "; break; diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/AppScopeTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/AppScopeTests.cs index cd32891d..80ef199e 100644 --- a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/AppScopeTests.cs +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/AppScopeTests.cs @@ -194,7 +194,10 @@ public void ALookupOutOfTheApp_StillTerminates_SoTheEdgeIsNotLost() var model = DataModelConverterService.ParseModules([module], scope); Assert.Contains(model.relationships, r => r.LeftSideTable?.LogicalName == "contoso_inapp"); - Assert.Contains(model.tables, t => t.LogicalName == "contoso_outside" && t.Type == Model.TableType.NotInSolution); + + // NotInApp rather than NotInSolution: this input does declare the table, so it is + // outside the app rather than missing from the solution. + Assert.Contains(model.tables, t => t.LogicalName == "contoso_outside" && t.Type == Model.TableType.NotInApp); } [Fact] diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/ColumnScopeTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/ColumnScopeTests.cs new file mode 100644 index 00000000..9871096c --- /dev/null +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/ColumnScopeTests.cs @@ -0,0 +1,178 @@ +using System; +using System.IO; +using System.Linq; +using System.Xml.Linq; +using TALXIS.CLI.Features.Data.DataModelConverter; +using TALXIS.CLI.Features.Data.DataModelConverter.AppScope; +using Model = TALXIS.CLI.Features.Data.DataModelConverter.Model; +using Xunit; + +namespace TALXIS.CLI.Tests.Data.DataModelConverter; + +/// +/// Narrowing an app's tables to the columns something in it refers to. The rule that must +/// never break: a column an edge depends on stays, because the SQL and EDMX translators +/// read a relationship's endpoints without a null check — dropping one turns a narrower +/// diagram into a crash. +/// +public class ColumnScopeTests : IDisposable +{ + private readonly string _root = Path.Combine(Path.GetTempPath(), "txc-cols-" + Path.GetRandomFileName()); + + public void Dispose() + { + if (Directory.Exists(_root)) Directory.Delete(_root, recursive: true); + GC.SuppressFinalize(this); + } + + private void WriteForm(string contents) + { + var dir = Path.Combine(_root, "module", "Declarations", "Entities", "contoso_thing", "FormXml", "main"); + Directory.CreateDirectory(dir); + File.WriteAllText(Path.Combine(dir, "form.xml"), contents); + } + + private void WriteCode(string fileName, string contents) + { + var dir = Path.Combine(_root, "module", "Plugins"); + Directory.CreateDirectory(dir); + File.WriteAllText(Path.Combine(dir, fileName), contents); + } + + private static XElement Entity(string logicalName, params string[] columns) => + XElement.Parse($""" + + {logicalName} + + primarykey + {string.Join("", columns.Select(c => $"""nvarchar50"""))} + + + """); + + private ResolvedAppScope ScopeFor(params string[] tables) + { + var scope = new ResolvedAppScope { UniqueName = "contoso_app", Detail = DetailLevel.Minimal }; + scope.SearchRoots.Add(_root); + foreach (var t in tables) scope.TableLogicalNames.Add(t); + return scope; + } + + private static Model.Table TableIn(Model.ParsedModel m, string name) => + m.tables.Single(t => t.LogicalName == name); + + [Fact] + public void AColumnAFormRefersTo_IsKept_AndOneNothingRefersTo_IsDroppedAndReported() + { + WriteForm("""
"""); + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", "contoso_onaform", "contoso_nowhere")); + + var scope = ScopeFor("contoso_thing"); + var model = DataModelConverterService.ParseModules([module], scope); + + var columns = TableIn(model, "contoso_thing").Rows.Select(r => r.Name).ToList(); + Assert.Contains("contoso_onaform", columns); + Assert.DoesNotContain("contoso_nowhere", columns); + Assert.Contains(scope.DroppedColumns, c => + c.Table == "contoso_thing" && c.Column == "contoso_nowhere" && c.Reason == DropReason.NoReferenceFound); + } + + [Fact] + public void ThePrimaryKeyIsNeverDropped_EvenWhenNothingRefersToIt() + { + WriteForm("
"); + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", "contoso_nowhere")); + + var model = DataModelConverterService.ParseModules([module], ScopeFor("contoso_thing")); + + Assert.Contains(TableIn(model, "contoso_thing").Rows, r => r.RowType == Model.RowType.Primarykey); + } + + [Fact] + public void AColumnAnEdgeDependsOn_SurvivesAndTheSqlAndEdmxTargetsStillRender() + { + // The regression that matters: the translators dereference a relationship's + // endpoints with no null check, so dropping one crashes rather than shrinks. + WriteForm(""); + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_child", "contoso_lookupid")); + module.entities.Add(Entity("contoso_parent")); + module.relationships.Add(XElement.Parse(""" + + OneToMany + contoso_child + contoso_parent + contoso_lookupid + + """)); + + var model = DataModelConverterService.ParseModules( + [module], ScopeFor("contoso_child", "contoso_parent")); + + Assert.Contains(TableIn(model, "contoso_child").Rows, r => r.Name == "contoso_lookupid"); + Assert.All(model.relationships, r => + { + Assert.NotNull(r.LeftSideRow); + Assert.NotNull(r.RighSideRow); + }); + + Assert.Null(Record.Exception(() => DataModelConverterService.ConvertToSQL(model))); + Assert.Null(Record.Exception(() => DataModelConverterService.ConvertToEDSSQL(model))); + Assert.Null(Record.Exception(() => DataModelConverterService.ConvertToEDMX(model))); + } + + [Fact] + public void AColumnOnlyAPluginMentions_IsKept() + { + // Plug-in and script sources sit outside the declarations, so a column used only + // from one is invisible unless they are read too. + WriteForm(""); + WriteCode("Handler.cs", """var v = entity.GetAttributeValue("contoso_onlyincode");"""); + + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", "contoso_onlyincode")); + + var model = DataModelConverterService.ParseModules([module], ScopeFor("contoso_thing")); + + Assert.Contains(TableIn(model, "contoso_thing").Rows, r => r.Name == "contoso_onlyincode"); + } + + [Fact] + public void AnEntityDeclarationDoesNotCountAsAReferenceToItsOwnColumns() + { + // Scanning Entity.xml would report every column as referenced by its own + // declaration, which would make the filter a no-op that looks like it works. The + // form gives the table an artefact of its own, so the reference rule applies rather + // than the classification a table with no artefacts falls back to. + WriteForm(""""""); + var declarations = Path.Combine(_root, "module", "Declarations", "Entities", "contoso_thing"); + Directory.CreateDirectory(declarations); + File.WriteAllText(Path.Combine(declarations, "Entity.xml"), + Entity("contoso_thing", "contoso_nowhere").ToString()); + + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", "contoso_onaform", "contoso_nowhere")); + + var model = DataModelConverterService.ParseModules([module], ScopeFor("contoso_thing")); + + Assert.DoesNotContain(TableIn(model, "contoso_thing").Rows, r => r.Name == "contoso_nowhere"); + } + + [Fact] + public void AtFullDetail_ColumnsAreLeftAlone() + { + WriteForm("
"); + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", "contoso_nowhere")); + + var scope = new ResolvedAppScope { UniqueName = "contoso_app" }; + scope.TableLogicalNames.Add("contoso_thing"); + + var model = DataModelConverterService.ParseModules([module], scope); + + Assert.Contains(TableIn(model, "contoso_thing").Rows, r => r.Name == "contoso_nowhere"); + Assert.Empty(scope.DroppedColumns); + } +} diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MinimalDetailTableTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MinimalDetailTableTests.cs new file mode 100644 index 00000000..3d40153c --- /dev/null +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/MinimalDetailTableTests.cs @@ -0,0 +1,188 @@ +using System; +using System.IO; +using System.Linq; +using System.Xml.Linq; +using TALXIS.CLI.Features.Data.DataModelConverter; +using TALXIS.CLI.Features.Data.DataModelConverter.AppScope; +using TALXIS.CLI.Features.Data.DataModelConverter.Translators; +using Model = TALXIS.CLI.Features.Data.DataModelConverter.Model; +using Xunit; + +namespace TALXIS.CLI.Tests.Data.DataModelConverter; + +/// +/// The table side of a design view. An N:N belongs to an app's design only when both of its +/// tables do — admitting one on a single side dragged nine systemuser intersects and four +/// far-side stubs into one real app — and a stub for a table the inputs do declare must not +/// be coloured as missing from the solution, which was true of 13 of 14 stubs there. +/// +public class MinimalDetailTableTests : IDisposable +{ + private readonly string _root = Path.Combine(Path.GetTempPath(), "txc-detail-" + Path.GetRandomFileName()); + + public void Dispose() + { + if (Directory.Exists(_root)) Directory.Delete(_root, recursive: true); + GC.SuppressFinalize(this); + } + + private static XElement Entity(string logicalName, params string[] columns) => + XElement.Parse($""" + + {logicalName} + + primarykey + {string.Join("", columns.Select(c => $"""lookup1"""))} + + + """); + + private static XElement ManyToMany(string name, string first, string second) => + XElement.Parse($""" + + ManyToMany + {first} + {second} + {name} + + """); + + private static XElement OneToMany(string name, string referencing, string referenced, string attribute) => + XElement.Parse($""" + + OneToMany + {referencing} + {referenced} + {attribute} + + """); + + private ResolvedAppScope ScopeFor(DetailLevel detail, params string[] tables) + { + var scope = new ResolvedAppScope { UniqueName = "contoso_app", Detail = detail }; + scope.SearchRoots.Add(_root); + foreach (var table in tables) scope.TableLogicalNames.Add(table); + return scope; + } + + private static bool HasTable(Model.ParsedModel model, string name) => + model.tables.Any(t => string.Equals(t.LogicalName, name, StringComparison.OrdinalIgnoreCase)); + + [Fact] + public void AnNToNWithOnlyOneSideInTheApp_IsDroppedInDesign_AndKeptAtFullDetail() + { + // The one gate this change turns. Full detail admits the intersect on either side, + // which is deliberate there; design asks whether the association is the app's own. + Model.ParsedModel Convert(DetailLevel detail) + { + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_inapp")); + module.entities.Add(Entity("contoso_stranger")); + module.relationships.Add(ManyToMany("contoso_inapp_stranger", "contoso_inapp", "contoso_stranger")); + return DataModelConverterService.ParseModules([module], ScopeFor(detail, "contoso_inapp")); + } + + Assert.False(HasTable(Convert(DetailLevel.Minimal), "contoso_inapp_stranger")); + Assert.True(HasTable(Convert(DetailLevel.Full), "contoso_inapp_stranger")); + } + + [Fact] + public void AnNToNWithBothSidesInTheApp_Survives() + { + // The rule must not cost an association the app genuinely owns. + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_left")); + module.entities.Add(Entity("contoso_right")); + module.relationships.Add(ManyToMany("contoso_left_right", "contoso_left", "contoso_right")); + + var model = DataModelConverterService.ParseModules([module], ScopeFor(DetailLevel.Minimal, "contoso_left", "contoso_right")); + + Assert.True(HasTable(model, "contoso_left_right")); + } + + [Fact] + public void DroppingAnNToN_DoesNotRemoveAStubAnOrdinaryLookupStillNeeds() + { + // Suppression is per relationship, not "erase every table a dropped edge touched" -- + // Account and one contract table survived exactly this way in a real app. + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_inapp", "contoso_strangerid")); + module.entities.Add(Entity("contoso_stranger")); + module.relationships.Add(ManyToMany("contoso_inapp_stranger", "contoso_inapp", "contoso_stranger")); + module.relationships.Add(OneToMany("contoso_lookup", "contoso_inapp", "contoso_stranger", "contoso_strangerid")); + + var model = DataModelConverterService.ParseModules([module], ScopeFor(DetailLevel.Minimal, "contoso_inapp")); + + Assert.False(HasTable(model, "contoso_inapp_stranger")); + Assert.True(HasTable(model, "contoso_stranger")); + } + + [Fact] + public void AStubForATableAnInputDeclares_IsMarkedAsOutsideTheApp_AndColouredDifferently() + { + // 13 of 14 stubs in a real app were declared as full entities in the same inputs, so + // the red "not in the solution" was untrue for almost all of them. + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_inapp", "contoso_declaredid")); + module.entities.Add(Entity("contoso_declared")); + module.relationships.Add(OneToMany("contoso_lookup", "contoso_inapp", "contoso_declared", "contoso_declaredid")); + + var model = DataModelConverterService.ParseModules([module], ScopeFor(DetailLevel.Minimal, "contoso_inapp")); + var stub = model.tables.Single(t => t.LogicalName == "contoso_declared"); + + Assert.Equal(Model.TableType.NotInApp, stub.Type); + Assert.Contains("#7f8c8d", stub.ToDbDiagramNotation()); + } + + [Fact] + public void AStubForATableNoInputDeclares_KeepsTheColourThatSaysSo() + { + // The platform's own tables really are absent from the inputs, and a reader needs to + // keep being told that. + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_inapp", "contoso_absentid")); + module.relationships.Add(OneToMany("contoso_lookup", "contoso_inapp", "contoso_absent", "contoso_absentid")); + + var model = DataModelConverterService.ParseModules([module], ScopeFor(DetailLevel.Minimal, "contoso_inapp")); + var stub = model.tables.Single(t => t.LogicalName == "contoso_absent"); + + Assert.Equal(Model.TableType.NotInSolution, stub.Type); + Assert.Contains("#c0392b", stub.ToDbDiagramNotation()); + } + + [Fact] + public void WithoutAnAppScope_NoStubIsEverMarkedAsOutsideOne() + { + // "Outside the app" is only meaningful when an app was named; converting a whole + // solution must keep saying what it says today. + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", "contoso_absentid")); + module.relationships.Add(OneToMany("contoso_lookup", "contoso_thing", "contoso_absent", "contoso_absentid")); + + var model = DataModelConverterService.ParseModules([module], null); + + Assert.DoesNotContain(model.tables, t => t.Type == Model.TableType.NotInApp); + } + + [Fact] + public void EveryTargetStillRendersAfterTablesAndColumnsAreDropped() + { + // The crash class this change risks: the SQL and EDMX translators read a + // relationship's endpoint tables and rows with no null check, so a table may never + // go while an edge still points at it. + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_inapp", "contoso_declaredid")); + module.entities.Add(Entity("contoso_declared")); + module.entities.Add(Entity("contoso_stranger")); + module.relationships.Add(OneToMany("contoso_lookup", "contoso_inapp", "contoso_declared", "contoso_declaredid")); + module.relationships.Add(ManyToMany("contoso_inapp_stranger", "contoso_inapp", "contoso_stranger")); + + var model = DataModelConverterService.ParseModules([module], ScopeFor(DetailLevel.Minimal, "contoso_inapp")); + + Assert.Null(Record.Exception(() => DataModelConverterService.ConvertToDBML(model))); + Assert.Null(Record.Exception(() => DataModelConverterService.ConvertToSQL(model))); + Assert.Null(Record.Exception(() => DataModelConverterService.ConvertToEDSSQL(model))); + Assert.Null(Record.Exception(() => DataModelConverterService.ConvertToEDMX(model))); + Assert.Null(Record.Exception(() => DataModelConverterService.ConvertToRibbonDiff(model))); + } +} diff --git a/tests/TALXIS.CLI.Tests/Data/DataModelConverter/PerTableColumnScopeTests.cs b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/PerTableColumnScopeTests.cs new file mode 100644 index 00000000..8d13d2c1 --- /dev/null +++ b/tests/TALXIS.CLI.Tests/Data/DataModelConverter/PerTableColumnScopeTests.cs @@ -0,0 +1,222 @@ +using System; +using System.IO; +using System.Linq; +using System.Xml.Linq; +using TALXIS.CLI.Features.Data.DataModelConverter; +using TALXIS.CLI.Features.Data.DataModelConverter.AppScope; +using Model = TALXIS.CLI.Features.Data.DataModelConverter.Model; +using Xunit; + +namespace TALXIS.CLI.Tests.Data.DataModelConverter; + +/// +/// A reference belongs to the table whose artefact made it. Matching on the name alone keeps +/// a column on every table declaring that name, which is why one view showing createdon kept +/// it on nineteen tables of a real app. +/// +public class PerTableColumnScopeTests : IDisposable +{ + private readonly string _root = Path.Combine(Path.GetTempPath(), "txc-pertable-" + Path.GetRandomFileName()); + + public void Dispose() + { + if (Directory.Exists(_root)) Directory.Delete(_root, recursive: true); + GC.SuppressFinalize(this); + } + + private void WriteArtefact(string entity, string folder, string fileName, string contents) + { + var dir = Path.Combine(_root, "module", "Declarations", "Entities", entity, folder); + Directory.CreateDirectory(dir); + File.WriteAllText(Path.Combine(dir, fileName), contents); + } + + private void WriteSitemap(string contents) + { + var dir = Path.Combine(_root, "module", "Declarations", "AppModuleSiteMaps", "contoso_app"); + Directory.CreateDirectory(dir); + File.WriteAllText(Path.Combine(dir, "AppModuleSiteMap.xml"), contents); + } + + /// An attribute with whatever metadata the case under test needs. + private static string Attribute(string name, string type = "nvarchar", bool? isLogical = null) + { + var flags = string.Empty; + if (isLogical != null) flags += $"{(isLogical.Value ? 1 : 0)}"; + var length = type == "nvarchar" ? "50" : string.Empty; + return $"""{type}{length}{flags}"""; + } + + private static XElement Entity(string logicalName, params string[] attributes) => + XElement.Parse($""" + + {logicalName} + + {Attribute(logicalName + "id", "primarykey")} + {string.Join("", attributes)} + + + """); + + private ResolvedAppScope ScopeFor(DetailLevel detail, string[] tables, params string[] authorPrefixes) + { + var scope = new ResolvedAppScope { UniqueName = "contoso_app", Detail = detail }; + scope.SearchRoots.Add(_root); + foreach (var table in tables) scope.TableLogicalNames.Add(table); + foreach (var prefix in authorPrefixes) scope.AuthorPrefixes.Add(prefix); + return scope; + } + + private static Model.Table TableIn(Model.ParsedModel model, string name) => + model.tables.Single(t => t.LogicalName == name); + + private static bool Has(Model.ParsedModel model, string table, string column) => + TableIn(model, table).Rows.Any(r => string.Equals(r.Name, column, StringComparison.OrdinalIgnoreCase)); + + [Fact] + public void AColumnOnlyOneTablesFormRefersTo_IsKeptThere_AndDroppedOnTheOther() + { + // The whole point of the change. Both tables declare createdon; only one shows it. + WriteArtefact("contoso_shown", "FormXml", "form.xml", + """"""); + WriteArtefact("contoso_hidden", "FormXml", "form.xml", + """
"""); + + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_shown", Attribute("createdon", "datetime"))); + module.entities.Add(Entity("contoso_hidden", Attribute("createdon", "datetime"), + Attribute("contoso_other"))); + + var model = DataModelConverterService.ParseModules([module], ScopeFor(DetailLevel.Minimal, ["contoso_shown", "contoso_hidden"], "contoso")); + + Assert.True(Has(model, "contoso_shown", "createdon")); + Assert.False(Has(model, "contoso_hidden", "createdon")); + } + + [Fact] + public void AnUnattributedReference_CannotRescueAPlatformColumn_ButDoesRescueAnAuthorsOne() + { + // A sitemap belongs to no single table, so it can only be matched by name -- and a + // platform column's name is the same on every table in the org. Letting one rescue + // createdon puts it straight back on every table, which is the defect being fixed. + WriteArtefact("contoso_thing", "FormXml", "form.xml", "
"); + WriteSitemap("""createdon contoso_authored"""); + + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", + Attribute("createdon", "datetime"), + Attribute("contoso_authored"))); + + var model = DataModelConverterService.ParseModules([module], ScopeFor(DetailLevel.Minimal, ["contoso_thing"], "contoso")); + + Assert.False(Has(model, "contoso_thing", "createdon")); + Assert.True(Has(model, "contoso_thing", "contoso_authored")); + } + + [Fact] + public void WithNoPublisherPrefixAvailable_AnUnattributedReferenceStillKeepsAColumn() + { + // No solution manifest, so no publisher prefix to check a name against. Calling a + // column the platform's on that basis would narrow the output on no evidence, so + // the sitemap's reference counts for a name that would otherwise look like the + // platform's. + WriteArtefact("contoso_thing", "FormXml", "form.xml", ""); + WriteSitemap("""mystery_column"""); + + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", Attribute("mystery_column"))); + + var model = DataModelConverterService.ParseModules([module], ScopeFor(DetailLevel.Minimal, ["contoso_thing"])); + + Assert.True(Has(model, "contoso_thing", "mystery_column")); + } + + [Fact] + public void StateAndStatus_SurviveWithNothingReferringToThem() + { + // A state model describes the table whatever shows it, and is the one exception the + // owner named to dropping the platform's own columns. + WriteArtefact("contoso_thing", "FormXml", "form.xml", ""); + + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", + Attribute("statecode", "state"), + Attribute("statuscode", "status"))); + + var model = DataModelConverterService.ParseModules([module], ScopeFor(DetailLevel.Minimal, ["contoso_thing"], "contoso")); + + Assert.True(Has(model, "contoso_thing", "statecode")); + Assert.True(Has(model, "contoso_thing", "statuscode")); + } + + [Fact] + public void DesignPlumbing_IsDroppedEvenWhereAFormRefersToIt() + { + // Logical columns are computed rather than stored, and process-flow bookkeeping is + // not design. Both stay out whatever mentions them, and are reported as such rather + // than as unreferenced. + WriteArtefact("contoso_thing", "FormXml", "form.xml", """ + + + + + """); + + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", + Attribute("owninguser", "lookup", isLogical: true), + Attribute("stageid", "uniqueidentifier"))); + + var scope = ScopeFor(DetailLevel.Minimal, ["contoso_thing"], "contoso"); + var model = DataModelConverterService.ParseModules([module], scope); + + Assert.False(Has(model, "contoso_thing", "owninguser")); + Assert.False(Has(model, "contoso_thing", "stageid")); + Assert.All(scope.DroppedColumns, c => Assert.Equal(DropReason.PlatformPlumbing, c.Reason)); + } + + [Fact] + public void TheBaseCurrencyTwinIsDropped_AndTheColumnItShadowsIsKept() + { + // Both halves are marked as an author's, so nothing but the name pairing separates + // the shadow the platform maintains from the column it shadows. + WriteArtefact("contoso_thing", "FormXml", "form.xml", """ +
+ + + + """); + + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", + Attribute("contoso_cost", "money"), + Attribute("contoso_cost_base", "money"))); + + var model = DataModelConverterService.ParseModules([module], ScopeFor(DetailLevel.Minimal, ["contoso_thing"], "contoso")); + + Assert.True(Has(model, "contoso_thing", "contoso_cost")); + Assert.False(Has(model, "contoso_thing", "contoso_cost_base")); + } + + [Fact] + public void AtFullDetail_EveryDesignOnlyRuleIsInert() + { + // The default must stay exactly what it converts today. + var module = new Model.Module { ModuleName = "m" }; + module.entities.Add(Entity("contoso_thing", + Attribute("createdon", "datetime"), + Attribute("owninguser", "lookup", isLogical: true), + Attribute("stageid", "uniqueidentifier"), + Attribute("contoso_cost", "money"), + Attribute("contoso_cost_base", "money"))); + + var scope = ScopeFor(DetailLevel.Full, ["contoso_thing"], "contoso"); + var model = DataModelConverterService.ParseModules([module], scope); + + Assert.True(Has(model, "contoso_thing", "createdon")); + Assert.True(Has(model, "contoso_thing", "owninguser")); + Assert.True(Has(model, "contoso_thing", "stageid")); + Assert.True(Has(model, "contoso_thing", "contoso_cost_base")); + Assert.Empty(scope.DroppedColumns); + } +}