ContentService returns outdated result

[sniffdk]

When getting content through ContentService.GetById(guid) the returned result will be a cached version that is the first version from that app restart.
Using the normal/older ContentService.GetById(int) returns the up-to-date result

This can be reproduced by comparing the two results after performing a change on a node, which should have resulted in a new version being created.

Using the Guid version:

Using the int version:

This was tested on Umbraco v. 7.12.2

[nul800sebastiaan]

Looks like I forgot to update this issue 🙈 but a lot of discussion has been going on here:
#3299

I've marked this as "Up for grabs" so that you or someone else coming along could create a pull request for it.

[sniffdk]

That's a tad misleading when you closed the PR while waiting for @zpqrtbnk to investigate ? 😄

[nul800sebastiaan]

lol.. I missed my own last comment!

I'll discuss with @zpqrtbnk or @Shazwazza

[sniffdk]

thanks 😆

[nul800sebastiaan]

Sounds like @Shazwazza knows what to do with the CacheRefreshers - I've asked him to either do a fix or reply on how to fix!

[sniffdk]

Great, thanks a lot 👍 😃

[tormnator]

I hope this bug will get fixed soon (both Umbraco 7 and 8). It bit me badly today. Until it's fixed, here's a work around:

public static IContent GetContentById(IContentService contentService, Guid udiGuid)
{
    /* As of (at least) Umbraco 7.14, there's a bug causing GetById to sometimes load an
     * older version of the content. This method works around the bug by comparing the
     * loaded content's version to the latest version, and loading the latest version if
     * needed.
     * https://github.com/umbraco/Umbraco-CMS/issues/2997
     * https://issues.umbraco.org/issue/U4-4254
     * https://issues.umbraco.org/issue/U4-10713
     */
    IContent possiblyOlderPage = contentService.GetById(udiGuid);
    Guid latestVersion = contentService.GetVersionIds(possiblyOlderPage.Id, 1).First();
    return latestVersion == possiblyOlderPage.Version
        ? possiblyOlderPage
        : contentService.GetByVersion(latestVersion);
}

[azure-devops-sync]

This item has been added to our backlog AB#1280

[anthonydotnet]

I've looked into this, and have some

Cache at the repository layer means the two repos have their own caches which are not "synch'd". Super difficult to unravel.

Cache at the repository layer is very problematic due to stale/invalidation issues such as this. It seems that it was implemented due to performance problems with either inefficient SQL queries, and/or excessive layers of abstraction.

A better long term solution is to remove the caching from repositories. Then add the cache at a higher layer in a way that you can switch it on and off. Right now there is an arbitrary 5min timeout.

I will look at a paper over the cracks solution to this issue in the mean time.

[anthonydotnet]

I looked into this, and there is a temporary paper over the cracks fix for this. The instantiation of the default cache policy can be done from a shared static method.

Is this acceptable?

[Shazwazza]

The fix for this is in the cache refreshers, we do no need to touch anything at the repository level. I plan to look into this today but will report back soon if other changes are needed.

[Shazwazza]

PR is here #6462

[KevinJump]

Hi, really minor, as this in in a point release anyway, but I think this should be marked as a breaking change, as the fix changes the signature of the JsonPayload constructors for content and other items..

[bergmania]

@Shazwazza, if we reintroduce the ctor's of signature public JsonPayload(int id, TreeChangeTypes changeTypes). This could be non-breaking, right?

[hemraker]

@KevinJump thanks for flagging this 🙌 It's marked as breaking for now, as it is breaking in the RC, but we're looking into making it non-breaking for the final 8.4 release.

[Shazwazza]

Sure, i've put back in the orig constructor but using it will result in this bug not being fixed. The orig ctor is marked as obsolete. See commit 2ec5a81