Skip to content

fix: honor per-type ShallowCopyForSameType overrides (#938)#974

Open
leno23 wants to merge 2 commits into
MapsterMapper:developmentfrom
leno23:fix/per-type-shallow-copy-settings-938-dev
Open

fix: honor per-type ShallowCopyForSameType overrides (#938)#974
leno23 wants to merge 2 commits into
MapsterMapper:developmentfrom
leno23:fix/per-type-shallow-copy-settings-938-dev

Conversation

@leno23
Copy link
Copy Markdown

@leno23 leno23 commented May 25, 2026

Summary

  • When deciding same-type shallow copy, use GetMergedSettings for the current source/destination pair instead of only arg.Settings and skipping whenever an explicit rule exists.
  • Allows cfg.Default.ShallowCopyForSameType(true) with cfg.NewConfig<T,T>().ShallowCopyForSameType(false) while preserving nested same-type shallow copy for types without an explicit override.

Fixes #938

Test plan

  • GlobalShallowCopy_WithPerTypeDeepCopy_ShouldRestoreViaMapToTarget
  • ParentDeepCopyOverride_ShouldNotDisableImplicitNestedShallowCopyForSameType
  • CI green on development

Use merged mapping settings when deciding same-type shallow copy so a
global default can be overridden per explicit config without disabling
nested same-type shallow copies for other types.

Co-authored-by: Cursor <cursoragent@cursor.com>
@leno23
Copy link
Copy Markdown
Author

leno23 commented May 26, 2026

Fixes #938 — uses merged settings for same-type shallow copy when RequireExplicitMapping is enabled.

GitHub Actions workflows are waiting on maintainer approval for fork PRs (action_required). Happy to address review feedback once CI can run.

public class WhenPerTypeShallowCopyOverride
{
[TestMethod]
public void GlobalShallowCopy_WithPerTypeDeepCopy_ShouldRestoreViaMapToTarget()
Copy link
Copy Markdown
Contributor

@DocSvartz DocSvartz May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is not valid and fail ❌

To obtain the desired result, the issue author used an incorrect configuration.

This is what the author wanted to achieve.

[TestMethod]
public void GlobalShallowCopy_WithPerTypeDeepCopy_ShouldRestoreViaMapToTarget()
{
    var cfg = new TypeAdapterConfig();
    cfg.RequireExplicitMapping = true;
    cfg.Default.AvoidInlineMapping(true);

    cfg.NewConfig<RandomObject2, RandomObject2>();
    cfg.NewConfig<RandomObject1, RandomObject1>()
        .Include<RandomObject2, RandomObject2>();

    cfg.NewConfig<MyFailStuff, MyFailStuff>()
       // .ShallowCopyForSameType(true)
        .UseDestinationValue(x => x.Item1)
        .UseDestinationValue(x => x.Item2);



    cfg.Compile();

    var mapper = new Mapper(cfg);


   

    var dynamicStuff = new MyFailStuff();
    dynamicStuff.Item1 = new RandomObject1();
    dynamicStuff.Item1.SampleName = "SN1";
    dynamicStuff.Item2 = new RandomObject2() { SampleNumber = 2 };
    //dynamicStuff.Item2.SampleNumber = 2;

    var str = dynamicStuff.BuildAdapter(cfg).CreateMapToTargetExpression<MyFailStuff>();



    var originalStuff = mapper.Map<MyFailStuff, MyFailStuff>(dynamicStuff);

    dynamicStuff.Item1.SampleName = "SN1CHANGED";
    (dynamicStuff.Item2 as RandomObject2).SampleNumber = 3;

    mapper.Map(originalStuff, dynamicStuff);

    dynamicStuff.Item1.SampleName.ShouldBe("SN1");
    (dynamicStuff.Item2 as RandomObject2).SampleNumber.ShouldBe(2);
    ReferenceEquals(dynamicStuff.Item1, originalStuff.Item1).ShouldBeFalse();
    ReferenceEquals(dynamicStuff.Item2, originalStuff.Item2).ShouldBeFalse();
}

  public class RandomObject1
  {
      public string? SampleName { get; set; }
  }

  public class RandomObject2 : RandomObject1
  {
      public int SampleNumber { get; set; }
  }

Comment on lines +510 to 512
if (_source.Type == destinationType && shallowCopySettings.ShallowCopyForSameType == true
&& notUsingDestinationValue)
exp = _source;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the wrong level of mapping.
This is where properties are mapped, not types.

A shallow copy creates a new top-level object but does not duplicate nested elements. Instead, it copies the memory references of nested objects and arrays. This means modifying a nested property in the copy will inadvertently change it in the original object.

If you consider this a new TopLevel, a new object must be created here in any case, and its properties must be copied by reference.
Or if this is a parsing of the properties of the original TopLevel (the one specified in arg).
Then searching for this setting is pointless, since all TopLevel properties must be copied by reference in any case.

Restore the rule == null guard alongside merged settings so explicit
type configs still map nested members on MapToTarget instead of
reusing source references.

Co-authored-by: Cursor <cursoragent@cursor.com>
@leno23
Copy link
Copy Markdown
Author

leno23 commented May 26, 2026

CI fix pushed (44b1479)

GlobalShallowCopy_WithPerTypeDeepCopy_ShouldRestoreViaMapToTarget failed because removing rule == null caused explicit configs (RandomObject1, RandomObject2) to shallow-copy nested references under the global default, so MapToTarget never restored Item1/Item2 property values.

Restored && rule == null while keeping GetMergedSettings for per-type overrides. Implicit nested same-type mappings (e.g. NestedChild in the passing test) still shallow-copy; explicit mapped types still deep-map members on MapToTarget.

Please re-run / approve CI when convenient.

@leno23
Copy link
Copy Markdown
Author

leno23 commented May 26, 2026

Friendly ping: CI fix is on 44b1479. Fork workflow runs need maintainer approval — could someone approve/re-run Build and Test when convenient? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants