Copy events modify properties of Copy/Original objects as if they were the same instance

[tomwidel]

Using Umbraco v8.0.2

Copied/Copying events modify properties of Copy/Original objects as if they were the same instance.

The issue occurs during handling of ContentService.Copied and/or ContentService.Copying events.

Assigning a value to either e.Copy or e.Original object via SetValue() method updates values of both objects as if they were the same instance.

I believe the issue is caused by the Umbraco.Core.Models.ContentBase.Properties property being marked with DoNotCloneAttribute.

During DeepCloneWithResetIdentities() method called internally by Copy the same Umbraco.Core.Models.ContentBase.Properties instance is shared by both e.Copy and e.Original.

Reproduction

Steps to reproduce

public class CopyEventsComponent : IComponent
{

    public void Initialize()
    {
        ContentService.Copied += ContentService_Copied;
        ContentService.Copying += ContentService_Copying;
    }

    public void Terminate()
    {
        ContentService.Copied -= ContentService_Copied;
        ContentService.Copying -= ContentService_Copying;
    }

    private void ContentService_Copied(IContentService sender, CopyEventArgs<IContent> e)
    {
        e.Copy.SetValue("PROPERTY_NAME", "1");
        // e.Copy.GetValue<string>("PROPERTY_NAME") and
        // e.Original.GetValue<string>("PROPERTY_NAME") are both equal to "1"

        e.Original.SetValue("PROPERTY_NAME", "2");
        // e.Copy.GetValue<string>("PROPERTY_NAME") and
        // e.Original.GetValue<string>("PROPERTY_NAME") are both equal to "2"
    }

    private void ContentService_Copying(IContentService sender, CopyEventArgs<IContent> e)
    {
        e.Copy.SetValue("PROPERTY_NAME", "1");
        // e.Copy.GetValue<string>("PROPERTY_NAME") and
        // e.Original.GetValue<string>("PROPERTY_NAME") are both equal to "1"

        e.Original.SetValue("PROPERTY_NAME", "2");
        // e.Copy.GetValue<string>("PROPERTY_NAME") and
        // e.Original.GetValue<string>("PROPERTY_NAME") are both equal to "2"
    }

}

Expected result

e.Copy.GetValue("PROPERTY_NAME") should be "1"

Actual result

e.Copy.GetValue("PROPERTY_NAME") is "2"

[nul800sebastiaan]

Thanks for reporting @tomwidel - I'll need to discuss with some people at HQ to see what the expected behavior is and why, it sounds like a bug indeed but maybe I'm missing something. Have you looked into the sender object as well to see if that contains a usable copy instead? Note: I'm not familiar with the code, so I might be saying silly things now.. ;-)

I'll let you know when I get some answers from my colleagues!

[atsimourtos]

@nul800sebastiaan Hello, is there any update on this? In version 8.5.2 this bug still exists. As an alternative to make it work someone can re-fetch the node by using Content Service.

var copiedNode = e.Copy;

var isSaved = sender.Save(copiedNode, -1, false);

var newNode = sender.GetById(copiedNode.Id);
// change property value here by referencing newNode
// save newNode

[Shazwazza]

The [DoNotClone] is meant to be there, there's a note in the code comments:

Marked DoNotClone since we'll manually clone the underlying field to deal with the event handling

The actual cloning of the properties manually takes place in the override of PerformDeepClone

That said there's definitely an issue here. I've created a unit test to work with. Although we could 'fix' this by re-fetching the node it's just covering up another issue which I think is actually to do with the cloning of property values within the property collection. I'll see what i can find.

[Shazwazza]

Fixed in this PR #7795

Please see notes there... this is actually quite a large bug that could have unknown larger consequences.

[nul800sebastiaan]

Thanks Shannon for digging in. This is fixed for v8.7.0.