Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Orphan language is left in YAML after deleting all versions (and having no unversioned fields) #168

Closed
kamsar opened this issue Aug 23, 2016 · 11 comments

Comments

@kamsar
Copy link
Collaborator

kamsar commented Aug 23, 2016

Reported via @cassidydotdk on Slack.

Reproduction:

  • Create an item with versions.
  • Remove all versions in that language.
  • Review YAML.

It will contain:

- Language: en
  Versions:

This causes confusion on deserialization.

@kamsar
Copy link
Collaborator Author

kamsar commented Aug 23, 2016

This is caused by all languages being ensured to be present when saving (a requisite for sitecore packages and their wacky way to creating items).

Since it's not truly an error to specify a language but have no fields or versions for it, I modified the YAML formatter to simply not emit a language if nothing of value exists underneath it.

@kamsar
Copy link
Collaborator Author

kamsar commented Aug 23, 2016

Ref: SitecoreUnicorn/Rainbow@399bd65

@cassidydotdk
Copy link
Member

Hi Kam. Delayed - life got in the way - I have found a way to provoke the issue we've been chatting about. If you run this - not at all pretty - snippet of code on a clean 8.2 using RC4, you end up with corrupted YAML.

protected void Page_Load(object sender, EventArgs e)
{
    var db = Factory.GetDatabase("master");
    var root = db.GetItem("/sitecore/content/home");
    var sampleItemTemplateId = new TemplateID(new ID("{76036F5E-CBCE-46D1-AF0A-4143F9B557AA}"));
    int breakpoint = 0;

    using (new BulkUpdateContext())
    using (new EventDisabler())
    {
        for (var i = 0; i < 10; i++)
        {
            var addedItem = root.Add($"Test{i}", sampleItemTemplateId);

            var languageItem = addedItem.Database.GetItem(addedItem.ID, Language.Parse("da"));
            var addedVersion = languageItem.Versions.AddVersion();
            addedVersion.Editing.BeginEdit();
            addedVersion["Title"] = "da Title";
            addedVersion["Text"] = "da Text";
            addedVersion.Editing.EndEdit();
        }

        breakpoint = 1;

        foreach (Item child in root.GetChildren(ChildListOptions.None))
        {
            var languageChild = child.Versions.GetLatestVersion(Language.Parse("en"));
            languageChild.Versions.RemoveVersion();
        }

        breakpoint = 2;
    }

    litOut.Text = breakpoint.ToString();
}

The resulting YAML for one of the child items, looks like this:

---
ID: "e77c5eb1-2ac3-4b1a-9b1c-60ba03ed6141"
Parent: "110d559f-dea5-42ea-9c1c-8a5df7e70ef9"
Template: "76036f5e-cbce-46d1-af0a-4143f9b557aa"
Path: /sitecore/content/Home/Test0
DB: master
Languages:
- Language: da
  Versions:
  - Version: 1

I've narrowed it down some. If I remove the EventDisabler(); the YAML comes out correct like this:

---
ID: "97b45cf9-a19c-40ca-abb0-273d09a88aa6"
Parent: "110d559f-dea5-42ea-9c1c-8a5df7e70ef9"
Template: "76036f5e-cbce-46d1-af0a-4143f9b557aa"
Path: /sitecore/content/Home/Test0
DB: master
Languages:
- Language: da
  Versions:
  - Version: 1
    Fields:
    - ID: "25bed78c-4957-4165-998a-ca1b52f67497"
      Hint: __Created
      Value: 20160927T081148Z
    - ID: "5dd74568-4d4b-44c1-b513-0af5f4cda34f"
      Hint: __Created by
      Value: |
        sitecore\admin
    - ID: "75577384-3c97-45da-a847-81b00500e250"
      Hint: Title
      Value: da Title
    - ID: "a60acd61-a6db-4182-8329-c957982cec74"
      Hint: Text
      Value: da Text

Which is odd; since Unicorn/Rainbow should not be relying on event code.

It's worth noting; even with the EventDisabler(); the items do get created correctly in Sitecore - they just end up broken on disk.

27-09-2016 10-14-25

I'm not even sure where to start when it comes to debugging this issue; and my local dev environment is also a bit... sporadic right now (new PC) - but I might give it a go once I have all my projects set up again.

@cassidydotdk
Copy link
Member

Note; it might not be the exact same issue as above, as the YAML is not emitting a blank version like your initial comment above. I will try and mess with the Sample Item template a bit, see if I can provoke the exact same response.

@cassidydotdk
Copy link
Member

cassidydotdk commented Sep 27, 2016

Addendum: Actually. The items do not get created correctly. The "da" versioned fields have no content in them when the EventDisabler() is on. Which puts the issue in Sitecore's court I think.

edit

No such luck. If I disable the config file (and therefore disable Unicorn operating on these child items), the items get created correctly - EventDisabler() or not. Therefore it must be Unicorn that interferes in some way.

@cassidydotdk
Copy link
Member

cassidydotdk commented Sep 27, 2016

Ok, fairly confident we're dealing with the same issue here. I expanded my code to also remove the "da" versions - and Sitecore ended up as expected with versionless items. YML however, looks faulty.

Code:

breakpoint = 1;

foreach (Item child in root.GetChildren(ChildListOptions.None))
{
    var languageChild = child.Versions.GetLatestVersion(Language.Parse("en"));
    languageChild.Versions.RemoveVersion();
}

breakpoint = 2;

foreach (Item child in root.GetChildren(ChildListOptions.None))
{
    var languageChild = child.Versions.GetLatestVersion(Language.Parse("da"));
    languageChild.Versions.RemoveVersion();
}

breakpoint = 3;

YML:

---
ID: "0a380878-3604-468d-80fc-523ff15cbd37"
Parent: "110d559f-dea5-42ea-9c1c-8a5df7e70ef9"
Template: "76036f5e-cbce-46d1-af0a-4143f9b557aa"
Path: /sitecore/content/Home/Test0
DB: master
Languages:

And current version:

27-09-2016 11-24-51

Enabling or disabling EventDisabler() makes no difference in this case. It just breaks. YML output with EventDisabler() commented out produces same output as above.

---
ID: "15084c0f-69f2-4337-9098-8a397381db3b"
Parent: "110d559f-dea5-42ea-9c1c-8a5df7e70ef9"
Template: "76036f5e-cbce-46d1-af0a-4143f9b557aa"
Path: /sitecore/content/Home/Test0
DB: master
Languages:

@kamsar
Copy link
Collaborator Author

kamsar commented Sep 29, 2016

I am able to reproduce the same behavior with 8.2 RTM running a harness off an MVC action (see below). I extended the samples above to run both with and without EventDisabler.

You're correct that Unicorn should not be affected by event disabling. However we all know how much Sitecore likes its caches, and it's all too possible that some indirect dependency on cache invalidation, say, which does depend on events being on, is preventing data from getting where it needs to go.

Of course it's also possible that my code sucks ;)

  public ActionResult Provocation()
    {
        var db = Factory.GetDatabase("master");
        var root = db.GetItem("/sitecore/content/prototype/home");

        var sampleItemTemplateId = new TemplateID(new ID("{76036F5E-CBCE-46D1-AF0A-4143F9B557AA}"));

        // CREATE A ROOT
        var testRootA = root.Children["TestA"];
        if (testRootA == null) testRootA = root.Add("TestA", sampleItemTemplateId);
        testRootA.RecycleChildren();

        // CREATE B ROOT
        var testRootB = root.Children["TestB"];
        if (testRootB == null) testRootB = root.Add("TestB", sampleItemTemplateId);
        testRootB.RecycleChildren();

        int breakpoint = 0;

        using (new BulkUpdateContext())
        using (new EventDisabler())
        {
            RunReproTests(testRootA, sampleItemTemplateId);
        }

        using (new BulkUpdateContext())
        {
            RunReproTests(testRootB, sampleItemTemplateId);
        }

        return Content(breakpoint.ToString());
    }

    private static void RunReproTests(Item root, TemplateID sampleItemTemplateId)
    {
        int breakpoint = 0;

        for (var i = 0; i < 10; i++)
        {
            var addedItem = root.Add($"Test{i}", sampleItemTemplateId);

            var languageItem = addedItem.Database.GetItem(addedItem.ID, Language.Parse("da"));
            var addedVersion = languageItem.Versions.AddVersion();
            addedVersion.Editing.BeginEdit();
            addedVersion["Title"] = "da Title";
            addedVersion["Text"] = "da Text";
            addedVersion.Editing.EndEdit();
        }

        breakpoint = 1;

        foreach (Item child in root.GetChildren(ChildListOptions.None))
        {
            var languageChild = child.Versions.GetLatestVersion(Language.Parse("en"));
            languageChild.Versions.RemoveVersion();
        }

        breakpoint = 2;
        foreach (Item child in root.GetChildren(ChildListOptions.None))
        {
            var languageChild = child.Versions.GetLatestVersion(Language.Parse("da"));
            languageChild.Versions.RemoveVersion();
        }

        breakpoint = 3;
    }

@kamsar
Copy link
Collaborator Author

kamsar commented Oct 4, 2016

The apparent root cause is quite interesting; it looks like due to the use of the event disabler we run into a situation where:

  • The item being saved is loaded using DatabaseCacheDisabler and proxied into an ItemData
  • ItemData then lazily loads its Versions property outside the DatabaseCacheDisabler and for reasons unknown the cache is returning empty field values for all the things during this event disabled time.

Example from immediate window:

 var dis = new DatabaseCacheDisabler();
 Expression has been evaluated and has no value
 item.Database.GetItem(item.ID, Language.Parse("da"))["Title"]
 "da Title"
 dis.Dispose();
 Expression has been evaluated and has no value
 item.Database.GetItem(item.ID, Language.Parse("da"))["Title"]
 ""

@kamsar
Copy link
Collaborator Author

kamsar commented Oct 4, 2016

By my estimation to provoke this issue you'd need:

  1. Multilingual site
  2. Doing item ops in an event disabler
  3. Specifically be removing versions

Geeze @cassidydotdk you know how to find the bugs 😀

@kamsar
Copy link
Collaborator Author

kamsar commented Oct 4, 2016

There's also a secondary issue of cache clearing as also noticed. I still am able to reproduce this with the Unicorn data provider disabled.

To reproduce that:

  • Grab the repro code above until breakpoint = 2
  • Run it
  • The items under Test0 (with EventDisabler) will have blank field values in da (in content editor)
  • The items under Test1 (without EventDisabler) will have correct field values in da (in content editor)
  • Nuking caches with /sitecore/admin/cache.aspx will restore the disabled values to their correct glory

As mentioned above I get the same result whether I have the Unicorn data provider active in this case or not. This is because the EventDisabler is blocking the item cache updates - so if you're doing bulk ops in an EventDisabler you should blow all caches when you're done.

kamsar added a commit that referenced this issue Oct 4, 2016
See #168

Previously the code to get a source item would respect database cache settings _only for the original language_ retrieved. This would cause wacky results when an item with more than one language had some operations performed on it, such as removing a version. The secondary versions would be null in the cache and the disabler setting would be ignored due to lazy loading thus resulting in field data loss.
kamsar added a commit that referenced this issue Oct 14, 2016
…ntom "v2" versions to be added to newly created items

This was caused by the database cache not being used when getting the item's versions to determine the serialized version number to add. When adding a new item, this would result in incorrectly selecting version 2, because the version 1 record had been already created in the database (Sitecore's data provider goes first). Thus, getting the versions for the item without data cache said it already possessed version 1, and we'd then say great we're adding v2.

This fixes the problem by simply serializing the already-updated item we got without cache - which already has the version data we need. The existing versioning code is still needed for Transparent Sync items however, where no data provider has gone ahead and updated the DB already. So that is still there for cases where the source item did not come from the database.
@kamsar
Copy link
Collaborator Author

kamsar commented Oct 27, 2016

Released in 3.3.1

@kamsar kamsar closed this as completed Oct 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants