Skip to content

[Bug]: UpdateValueAndRemoveDeletedReferenceProperties throws on every removal — the Assembler deletion path is dead #349

Description

@antoineatstariongroup

What type of issue is this?

  • Bug report
  • Feature request

Description

UpdateValueAndRemoveDeletedReferenceProperties throws
InvalidOperationException: Collection was modified on every removal, so the deletion path of
Assembler.Synchronize is dead — any Synchronize whose DTOs drop an element from an enumerable
reference property aborts mid-assembly.

Source: SysML2.NET.CodeGenerator/Templates/Uml/core-dal-poco-uml-extensions.hbs:78-90, replicated
into 231 call sites across 167 files under SysML2.NET.Dal/Core/AutoGenPocoExtension/. Affects
OwnedRelationship, OwnedRelatedElement, and Dependency.Client / Supplier.

The emitted shape, e.g. SysML2.NET.Dal/Core/AutoGenPocoExtension/ForkNodeExtensions.cs:105-112:

var ownedRelationshipToDelete = poco.OwnedRelationship.Select(x => x.Id).Except(dto.OwnedRelationship);

foreach (var identifier in ownedRelationshipToDelete)
{
    ((IContainedElement)poco).OwnedRelationship.Remove(poco.OwnedRelationship.Single(x => x.Id == identifier));
}

identifiersOfObjectsToDelete.AddRange(ownedRelationshipToDelete);
  • Throws. Except is deferred over the live ContainerList; the loop body removes from the
    collection it is streaming, invalidating the enumerator on the first removal.
  • Silently loses the deletions. Line 90 re-runs the deferred query after the removals, so
    identifiersOfObjectsToDelete returns empty and the removed POCOs leak in the cache. Fixing only
    the throw leaves this in place.
  • Quadratic. Single(x => x.Id == identifier) is the scan-per-element [Tech Debt]: Assembler.Synchronize is O(n²) — 89 s to assemble a 50 000-element model #347 removed from
    Assembler.Synchronize.

Steps to Reproduce

var assembler = new Assembler();
assembler.Synchronize([owner, membership, package]);   // owner.OwnedRelationship = [membership.Id]

owner.OwnedRelationship = [];

assembler.Synchronize([owner, membership, package]);   // InvalidOperationException

Expected: membership and its owned package are removed from the cache.

Proposed fix

In the template — do not hand-edit generated files:

var ownedRelationshipRetained = new HashSet<Guid>(dto.OwnedRelationship);
var ownedRelationshipToDelete = poco.OwnedRelationship.Where(x => !ownedRelationshipRetained.Contains(x.Id)).ToList();

foreach (var relationship in ownedRelationshipToDelete)
{
    ((IContainedElement)poco).OwnedRelationship.Remove(relationship);
}

identifiersOfObjectsToDelete.AddRange(ownedRelationshipToDelete.Select(x => x.Id));

Tasks

  • Fix the template and regenerate SysML2.NET.Dal/Core/AutoGenPocoExtension/.
  • Add removal coverage to AssemblerTestFixture — none exists today, which is why this was missed.
  • Verify cascading deletion now that identifiersOfObjectsToDelete is populated for the first
    time; the Assembler deletion loop has never executed.

System Configuration

Metadata

Metadata

Labels

CodeGenerationa Code Generation taskbugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions