We have moved to GitHub Issues
Created by Alessandro Calzavara 07 Jul 2017, 13:36:25 Updated by Stephan 28 Jul 2017, 07:44:06
Installed umbraco 7.6.3 with fanoe.
I tried to print the Key of the current node on a view, but it's always empty
@Model.Content.GetKey() // this prints all zeroes, 00000000-0000-0000-0000-000000000000
var withKey = Model.Content as IPublishedContentWithKey; // withKey is null
Same behavior with nodes from Umbraco.TypedContent(idPage)
The only way to get the correct key was to hit the db via ContentService
Thanks, '''unwrapping it before calling GetKey() worked'''. Is there any documentation (apart source code itself) or post on what is this wrap/unwrap about?
@sebastiaan This should be treated as a bug, IMO. If you call the GetKey() method on IPublishedContent you should get a key, not an empty Guid. The fact there is some obtuse and undocumented workaround really isn't the point.
If Umbraco is to move to using Guids then it needs this to be as painless as using Ids. Key should just be a property, like Id.
For anyone else stumbling on this, I found this three extension methods make working with GUIDs and UDI much less painful. I'd hope something similar would find their way into core?
if (withKey != null) return withKey; var wrapped = content as PublishedContentWrapped; if (wrapped != null) return GetContentWithKey(wrapped.Unwrap()); return null; }
The following method should guarantee to return a GUID key from
The following extension method should return a UDI from
''Is there any documentation (apart source code itself) or post on what is this wrap/unwrap about?''
The content cache internally produces IPublishedContent instances. Then we decided to add the Key property to published content, but due to backward compatibility we could not modify IPublishedContent and had to create the IPublishedContentWithKey interface.
Then, models (created with models builder) come on top of the IPublishedContent returned by the content cache. They ''wrap'' the original IPublishedContent and add some sugar (strongly typed properties, etc). The PublishedContentWrapped (and -WithKey) class is used as a base class to wrap, ie as a base class to models.
Because of all this, getting the Key of an IPublishedContent is tricky. We need to see if the IPublishedContent is, in reality, an IPublishedContentWithKey. But we also need to unwrap down to the very original IPublishedContent coming from the content cache (before models were created) to get the key.
And GetKey wasn't doing it properly. Should be fixed with the following PR:
Review: PublishedContentTests.GetKey() should pass
@zpqrtbnk Now that makes sense, thanks for the explanation!
Thanks @DanDiplo for keeping me sharp, should indeed not have closed this! Should all work in 7.6.5!
@sebastiaan No worries - I'm using it all in anger now, so there'll probably be more :p Thanks for fixing so fast!
"Then we decided to add the Key property to published content, but due to backward compatibility we could not modify IPublishedContent and had to create the IPublishedContentWithKey interface."
I've been thinking about this. You obviously are privy to a lot more knowledge than this, so forgive my probably uneducated comments, but it feels like this was a mistake. It seems better to bite the bullet now and extend the interface now rather than forever be stuck with two similar implementations.
Adding a Key property to
IPublishedContent would kept everything working as people currently expect. I'm not sure what backward compatibility would have broken - I'm pretty sure that in 99% of cases the only code that directly references the interface is your own code. For the small fraction of developers who do implement their own classes on IPublishedContent directly (rather than a derived class) then I'm sure they could add a new property (even if it returned an empty GUID) and recompile.
Ah, you are entirely forgiven, for a rather sensible comment. The decision was not an obvious one, and I certainly would not argue that the current situation is "the best". Backward compatibility is a pain.
Backwards Compatible: True
Affected versions: 7.6.3
Due in version: 7.6.5
Sprint: Sprint 63
Story Points: 1