U4-509 - Unpublish, then Publish - Node loses sort order in XML Cache

Created by Sebastiaan Janssen 19 Aug 2012, 14:53:39 Updated by Tom Fulton 22 Apr 2013, 17:00:15

If you unpublish a document, then Publish it, the document is added back to the XML Cache without regard to it's sort order - it's added to the end of the current level.

Ex:

  • Home -- News -- About -- Contact

  • Visit News and click the Unpublish button on the Properties tab

  • News is removed from the XML cache

  • Visit News again and click Save/Publish

  • News is added back to the XML, but it's added after the Contact node, so your navigation/etc appears as:

-- About -- Contact -- News

Even though the content is sorted properly in the back-end.

A workaround is after you publish an unpublished item, right-click it's parent and click Sort, then click the Sort button. This updates the XML cache with the correct sort order.

This is tested on a fresh 4.7.1 install

Screencast: http://www.screenr.com/PU6s

''Originally created on CodePlex by [tomnf|http://www.codeplex.com/site/users/view/tomnf]'' on 12/15/2011 4:53:52 PM [Codeplex ID: 30643 - Codeplex Votes: 4]

Imported comments

''Comment by [tomnf|http://www.codeplex.com/site/users/view/tomnf] on 12/15/2011 5:56:32 PM:'' Tim pointed out that the sortOrder attribute still reflects the correct sort order, so you could also work around this by sorting by that in your XSLT. I don't think we should have to worry about that though :)

I'm thinking the Publish doesn't take into account the sortOrder - why would it need to - new documents are added at the bottom, only time sorting changes is on the Sort action. So maybe a Publish should also check/update the sort order, or perhaps we can "fix" it with an event handler

Comments

Sebastiaan Janssen 29 Mar 2013, 19:14:29

In v6 this is now fixed.


Tom Fulton 16 Apr 2013, 05:28:07

Looks like this issue has re-appeared in 6.1.0 beta. Doesn't appear to be a problem in 6.0.3. Did it get lost in a merge maybe?

Screencast: http://screencast.com/t/9c6OlGS9rFS


Sebastiaan Janssen 17 Apr 2013, 11:17:53

Just to verify this is working in 6.0.3 yes, so something must've happened..


Shannon Deminick 17 Apr 2013, 19:18:14

@Tom - I've just tested this and it's not because the real sort order has changed, its mainly because the control that is rendering that menu doesn't sort by the sort order, it just renders based on the order in the XML file. If you look at your xml file after you publish again, you'll see that the sortOrder value is correct, but the node has been appended to the parent, not inserted. From the looks of the code I don't see how this has changed in 6.1, I'm going to test this in 6.0.4 to see if it's any different.

Also note that I'd like to refactor how the content class builds the xml structure. I recently did some profiling on a large site and the method to Sort nodes takes up nearly 50% of the CPU time during a publish request!! yikes.


Tom Fulton 17 Apr 2013, 19:24:25

Yep, that's correct - the @sortOrder attribute is correct but the actual order its sorted in the XML Cache isn't.

So the workaround is to make sure all your XSLT/Razor/etc macros explicitly sort by @sortOrder. But I don't think we should have to worry about that - we should expect loops to return in the same order as shown in the backoffice.

This is an old bug that was fixed somewhere in v6 I think, it works fine in v6.0.3, but not 6.1.0 beta. Haven't tested 6.0.4 yet. If you need a hand I can try to find where it broke by testing older changesets :)


Shannon Deminick 17 Apr 2013, 19:28:45

@Tom, gonna start testing now but if you have time atm would be awesome if you can help me determine where it's changed. From what I can tell the sorting stuff and appending a node into the xml hasn't changed. That is done in umbraco.content.AppendDocumentXml and umbraco.content.SortNodes


Shannon Deminick 17 Apr 2013, 19:32:00

I've just confirmed that this is an issue in 6.0.4 also.


Tom Fulton 17 Apr 2013, 19:32:27

Sure, I'll start digging right now to see if I can find the offending changeset. I did this yesterday for the inheritance issue, a bit tedious but helps narrow things down :)


Shannon Deminick 17 Apr 2013, 19:41:25

@Tom, I've just tested with 6.0.3 and this issue still remains.


Shannon Deminick 17 Apr 2013, 19:43:03

Unfortunately there is not reference in the repository for this issue U4-509 being fixed and this issue does not reference a changeset so not sure if/when this was actually fixed.


Tom Fulton 17 Apr 2013, 19:47:28

Yeah - it may be related to U4-888 but that also doesn't have an associated changeset. Give me just a few and I'll see if I can locate.


Shannon Deminick 17 Apr 2013, 19:49:00

Well, I know where to fix it but can't see how this has ever been fixed.


Tom Fulton 17 Apr 2013, 20:22:47

Just spent some time going through the 6.0.4 chain, but it turns out I can't reproduce it there. Did you say that you were?

I'll start again on the 6.1 branch


Shannon Deminick 17 Apr 2013, 20:26:19

On the 6.0.4 branch and the 6.0.3 branch when you unpublish the first node and look at the xml, it is removed. When you re-publish the xml node and look at your xml it will be appended to the bottom of the parent's list. This is the whole issue and it happens in 6.0.4 and 6.0.3. Unless there's some post sorting taking place when we pass the xml to the front end ?


Tom Fulton 17 Apr 2013, 20:32:59

In 6.0.4 & 6.0.3 it seems to be working fine for me - ie when you re-publish the xml node, it's not appended at the bottom but it exists in the placement where it should be. Maybe it's just in 6.1 - I'll start checking through those.


Shannon Deminick 17 Apr 2013, 20:34:45

That's strange because I get the same behavior in 6.0.4 and 6.0.3 !


Tom Fulton 17 Apr 2013, 21:44:22

Hmm, not sure why that's happening to you - are you using the same DB in both? You might need to Republish entire site before you test (I had a similar issue, was always at bottom until I did that)

Anyway, I'm having trouble narrowing down to a specific commit, as I keep running into unrelated problems on these older changesets, like unpublishing doesn't even remove from the XML cache.

But I'm pretty sure the issue is between rev 4098 - 4061 somewhere, as it seems to work correctly in 4098. Seems to be a lot with caching going on in that area, so might make sense. I'm done for the day but if you're still having problems I can keep trying to narrow down.


Shannon Deminick 17 Apr 2013, 23:47:40

Well, not sure how this actually works in any revision. The underlying problem is in content.AppendDocumentXml where it checks the 'currentSortOrder' against the 'siblingSortOrder'. The check is flawed because the siblingSortOrder will always be equal to the currentSortOrder so the Xml is never physically sorted.

I've also rewritten the algorithm for content.SortNodes since this was a performance bottleneck. Have written unit tests for this as well and benchmark tests. The results show (this is ONLY for this one method, not the process of sorting nodes in Umbraco back office):

current algorithm (x 10000 sorting 28 child nodes) ~ 7000ms new algorithm (x 10000 sorting 28 child nodes) ~ 600ms

this is an 92% improvement.

All of this has been fixed in 4.11.7 in revision: 416fd972d1f5


Tom Fulton 18 Apr 2013, 00:21:39

Nice! Is that going to be in 6.1 also? Let me know and I'll be glad to pull down and test.


Shannon Deminick 18 Apr 2013, 13:51:03

Yup, the change will be all merge up into higher branches. Will let you know when it is done.


Shannon Deminick 18 Apr 2013, 16:03:52

This has been fixed in all branches in revision 188b6d003876

I'm still unsure how this was ever working in any version but it definitely works in all versions now.


Tom Fulton 22 Apr 2013, 17:00:15

Looks like it works now - thanks once again :)


Priority: Normal

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 4.8.0, 4.9.0, 4.10.0, 4.11.0, 6.0.0, 6.1.0, 4.9.1, 4.11.1, 4.11.2, 4.11.3, 4.11.4, 6.0.1, 4.11.5, 6.0.2, 4.11.6, 6.0.3

Due in version: 6.0.4, 4.11.7

Sprint:

Story Points:

Cycle: