umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
Other
4.49k stars 2.69k forks source link

Exception while trying to access property when it has "Vary by culture" set to true after upgrade. #14689

Closed bartoszc9 closed 11 months ago

bartoszc9 commented 1 year ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

Umbraco version 12.1.0

Bug summary

For some reason I have ArgumentNullException while trying to access one of properties on object generated by Umbraco. It only happens, when "Vary by culture" is set to true, when I set it to false, then exception is not occurring. Everything was working before I've updated version from 8.18.8.

Do you have any ideas what is wrong?

image image

Specifics

stack trace: at Umbraco.Cms.Core.Collections.CompositeStringStringKey..ctor(String key1, String key2) at Umbraco.Cms.Infrastructure.PublishedCache.Property.GetSourceValue(String culture, String segment) at Umbraco.Cms.Infrastructure.PublishedCache.Property.HasValue(String culture, String segment) at Umbraco.Extensions.PublishedContentExtensions.Value[T](IPublishedContent content, IPublishedValueFallback publishedValueFallback, String alias, String culture, String segment, Fallback fallback, T defaultValue) at Umbraco.Web.PublishedModels.PressReleaseCategory.get_Category() in C:\Users\cloudnine\source\repos\Permobil-Global-Backend\src\Permobil.Core\Models\Umbraco\PressReleaseCategory.generated.cs:line 58 at Permobil.Content.PressReleases.PressReleaseCategoryIdService.GetPressReleaseCategory(PressReleaseCategory pressReleaseCategory) in C:\Users\cloudnine\source\repos\Permobil-Global-Backend\src\Permobil.Content\PressReleases\PressReleaseCategoryIdService.cs:line 27 at System.Linq.Enumerable.SelectEnumerableIterator2.MoveNext() at System.Linq.Enumerable.WhereEnumerableIterator1.MoveNext() at System.String.Join(String separator, IEnumerable1 values) at Permobil.Content.Components.Search.PressReleaseSearchIndexComponent.SavePressReleaseFields(Object sender, IndexingItemEventArgs e) in C:\Users\cloudnine\source\repos\Permobil-Global-Backend\src\Permobil.Content\Components\Search\PressReleaseSearchIndexComponent.cs:line 77 at Umbraco.Cms.Infrastructure.Examine.UmbracoExamineIndex.OnTransformingIndexValues(IndexingItemEventArgs e) at Examine.Lucene.Providers.LuceneIndex.ProcessIndexQueueItem(IndexOperation op) at Examine.Lucene.Providers.LuceneIndex.ProcessQueueItem(IndexOperation item) at Examine.Lucene.Providers.LuceneIndex.PerformIndexItemsInternal(IEnumerable1 valueSets, CancellationToken cancellationToken)

Steps to reproduce

-

Expected result / actual result

No response

github-actions[bot] commented 1 year ago

Hi there @bartoszc9!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

elit0451 commented 1 year ago

Hi @bartoszc9 👋

Since you have upgraded from v8 perhaps something went wrong with your custom implementation - it is a long way to v12, so perhaps you missed doing some changes. I would recommend visiting our https://our.umbraco.com/forum/ and initiating a discussion there. Perhaps someone from our friendly community has seen similar behaviour and can help you to figure out what went wrong.

And if you figure out that there is an actual bug within the CMS, list the Steps to reproduce in a GitHub issue like this one and we will have a look 🙂

benmurphycb commented 1 year ago

We are also getting this error - I have managed to track it back to an issue with the segment.

Even though segment is an optional parameter (string? segment = null) - it will throw an exception if you do not set it to "" an empty string.

@elit0451 please can we reopen this?

I have managed to track this back in the Umbraco code: The issue is with the Umbraco.Cms.Infrastructure.PublishedCache.GetSourceValue(string? culture = null, string? segment = null) method - the return statement is:

return _sourceValues.TryGetValue(
    new CompositeStringStringKey(culture, segment),
    out SourceInterValue? sourceValue)
    ? sourceValue.SourceValue
    : null;

The issue is that when newing up the CompositeStringStringKey, the segment is still null and not an empty string. The CompositeStringStringKey is expecting a string, not a nullable string.

From what I can see - the segment should be set by this line: _content.VariationContextAccessor.ContextualizeVariation(_variations, _content.Id, ref culture, ref segment);

But this method (Umbraco.Extensions.VariationContextAccessorExtensions.ContextualizeVariation()) will not always assign this to an empty string if it cannot get the segment.

The null coalescing operator in this code:

segment = contentId == null
    ? publishedVariationContext?.Segment
    : publishedVariationContext?.GetSegment(contentId.Value);

means that it can still end up being null, which then causes the exception to be thrown with the CompositeStringStringKey initialization mentioned above.

Is this an issue with how the site is set up, or should the code be:

segment = contentId == null
    ? publishedVariationContext?.Segment ?? ""
    : publishedVariationContext?.GetSegment(contentId.Value) ?? "";

?

bartoszc9 commented 1 year ago

@benmurphycb In my case as a workaround for this issue I had to set Variation context with specific culture (_variationContextAccessor.VariationContext = new(Cultures.RootCulture);) before accessing property.

smarshallsay commented 1 year ago

I think this may be the same issue I am getting on one of our websites that has many cultures. If I upgrade past 11.2.2 then I get null reference exceptions when I try to access certain values whether or not they have a value set. In my case the first erroring value is a bool (true/false) set on a page inherited from a composition

image

image

although the values are returned in the non-public members _content

benmurphycb commented 1 year ago

@smarshallsay I think this is different to the original issue. I created a new issue with more in depth details: https://github.com/umbraco/Umbraco-CMS/issues/14954

smarshallsay commented 1 year ago

Ok, thanks. I will make a new issue

netcamo commented 1 year ago

Hi @benmurphycb!

Thank you very much for informative explanation on the problem!

While I see the problem that is caused I would like to reproduce it before putting appropriate labels on it. I would be very happy if you could give me steps and instructions to reproduce the error.

I am looking forward to your reply and I wish you a wonderful week ahead! 😄

benmurphycb commented 1 year ago

Hi @netcamo

No problem, I have added as much details as I can to this issue

Thank you, you too! :)

Migaroez commented 11 months ago

Hey @bartoszc9

After inspecting your stacktrace, I think you might be using certain parts of the framework in a way that it might not be meant for.

I can see that one of the early entrypoints is examine Examine.UmbracoExamineIndex.OnTransformingIndexValues, this is part of the Content authoring part of the framework which uses IContent to move data around. This type of data is not optimized for rendering, holds history data, all available cultures, and so on. It is what is used in the backoffice as that is the place where you author your content.

You seem to have hooked into examine with PressReleaseSearchIndexComponent which calls a custom service PressReleaseCategoryIdService which then returns a modelsbuilder model (I think, looking at the name and the image you provided). A modelsbuilder model is a wrapper around IPulblishedContent, which is an optimized version of content for Rendering (not authoring). To be able to construct this model from an IContent, the pipeline needs to have information about the culture to be able to know which variant to supply as an IPublishedContent. Generally we do this for you when you request content trough the UmbracoContext as part of the request pipeline, which in this case you are not doing as this is not part of a request.

So it makes sense that if you set the VariationContext, it all suddenly works again.

But it is generally a bad idea to mix IpublishedContent derived data with IContent derived data. Using the IContentService for whatever you are doing is probably the better way to go.

Since this explain the issue you were having and all other reactions to this issue have been turned into their own issues, i will be closing this one. If something doesn't sound right, feel free to reopen it with a comment.

Have a wonderful day!