Fix algorithm recast child map copy

[Abhi190702]

Summary

Fixes #451

This fixes a shallow-copy bug in the Algorithm(IAlgorithm, asKind) recast constructor.

The constructor previously reused the source algorithm's mutable children map, which meant the original algorithm node and the recast algorithm node shared the same child state.

Root Cause

The recast constructor assigned the original map reference directly:

this.children = algorithm.getChildren();

Because of this, mutating children on the recast node could also mutate the original algorithm node.

Changes

• Copy the children map in Algorithm(IAlgorithm, asKind) using new HashMap<>(algorithm.getChildren())
• Add an AES-GCM regression test covering the recast path through AESEnricher
• Verify that mutating the returned/recast AES node does not remove children from the original AES node

Validation

Ran the targeted regression test:

mvn -pl enricher -am -Dtest=AESEnricherTest -Dsurefire.failIfNoSpecifiedTests=false -Dexec.skip=true test

Result:

Tests run: 4, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

[n1ckl0sk0rtge]

Thanks for the fix! The change in Algorithm.java is correct and the regression test reliably catches the bug (verified locally: test passes with the fix, fails when the fix is reverted).

Before merging, please extend the scope — the identical shallow-copy bug exists in two sibling recast constructors:

mapper/src/main/java/com/ibm/mapper/model/Protocol.java:36

public Protocol(@Nonnull Protocol protocol, @Nonnull final Class<? extends Protocol> asKind) {
    this.type = protocol.type;
    this.children = protocol.getChildren();   // same bug
    ...
}

mapper/src/main/java/com/ibm/mapper/model/Key.java:48

protected Key(
        @Nonnull Key key,
        @Nonnull DetectionLocation detectionLocation,
        @Nonnull final Class<? extends Key> asKind) {
    this.name = key.name;
    this.children = key.getChildren();   // same bug
    ...
}

Both follow the exact pattern fixed in Algorithm and have the same failure mode: any consumer that recasts a Protocol or Key and mutates its children will silently mutate the source node. Linked issue #451 already calls out that "every algorithm subclass that calls the shared recast constructor" is at risk — the same logic applies to Protocol/Key. HashMap is already imported in both files, so the fix is the same one-liner: new HashMap<>(source.getChildren()).

Please also add regression tests for these two paths — a direct unit test on the constructor would be cleaner than going through enrichers (and would let you cover Algorithm more tightly too, decoupled from AESEnricher).

Other notes

• DCO check failing — please amend with git commit --amend --signoff and force-push to add the Signed-off-by: trailer.
• Informational (not blocking): the fix is a map-level shallow copy — child INode references are still shared. That's correct for the reported bug (#451 is about put/remove mutations) and matches the existing semantics; deepCopy() already exists for full independence. Worth keeping in mind if future consumers mutate child state in place.

Otherwise looks good — fix is minimal, focused, and well-tested. Just want to close the same defect class in one PR rather than leaving the same bug live in two sibling classes.

[Abhi190702]

Hey @n1ckl0sk0rtge !!!

Thanks for the detailed review!

I updated the sibling recast constructors in Protocol and Key to copy the children map with new HashMap<>(...), matching the existing Algorithm fix.

I also added direct regression tests for the Algorithm, Protocol, and Key recast constructor paths. Each test verifies that the recast node receives a separate children map instance and that mutating the recast map does not affect the original node's children map.

I amended the commit with --signoff and force-pushed to address the DCO failure.

Validation:

mvn -pl mapper -Dexec.skip=true test → 46 tests, 0 failures
mvn -pl enricher -am -Dtest=AESEnricherTest -Dsurefire.failIfNoSpecifiedTests=false -Dexec.skip=true test → 4 tests, 0 failures