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

Content query with document containing RTE with Macro's throws ArgumentNullException #6141

Closed
Swimburger opened this issue Aug 18, 2019 · 11 comments

Comments

@Swimburger
Copy link
Contributor

Reproduction

Bug summary

I performed an upgrade from 7.15 to 8.1.2 following the upgrade blog post provided by Umbraco. After refactoring the code to the new API everything works except this one issue.

I have a document containing an RTE field with Macro's.
Instead of having the Macro render using my Macro code, it's simply returned as part of the HTML without calling the macro code.

After publishing the document an ArgumentNullException is thrown by Umbraco.Web.Macros.PublishedContentHashtableConverter..ctor(IPublishedContent doc) .

I noticed my macro was using Umbraco.TextboxMultiple instead of the new alias Umbraco.TextArea. After changing the Macro Parameter Editor to Umbraco.TextArea, nothing changed unfortunately, but it may be related... not sure.

Specifics

Pre-upgrade umbraco version: 7.15
Post-upgrade umbraco version: 8.1.2

Code snippet:

private IPublishedContent GetBlogPostContent(UmbracoContext umbracoContext, string slug)
{
    var content = umbracoContext.Content.GetByXPath("//blogPostPage[@urlName=$slug]", new XPathVariable("slug", slug)) # Exception is thrown on this line
        .ToList()
        .FirstOrDefault();
    return content;
}

Stacktrace:

[ArgumentNullException: Value cannot be null.
Parameter name: doc]
   Umbraco.Web.Macros.PublishedContentHashtableConverter..ctor(IPublishedContent doc) in D:\a\1\s\src\Umbraco.Web\Macros\PublishedContentHashtableConverter.cs:57
   Umbraco.Web.Macros.MacroRenderer.Render(String macroAlias, IPublishedContent content, IDictionary`2 macroParams) in D:\a\1\s\src\Umbraco.Web\Macros\MacroRenderer.cs:200
   Umbraco.Web.PropertyEditors.ValueConverters.<>c__DisplayClass4_1.<RenderRteMacros>b__1(String macroAlias, Dictionary`2 macroAttributes) in D:\a\1\s\src\Umbraco.Web\PropertyEditors\ValueConverters\RteMacroRenderingValueConverter.cs:56
   Umbraco.Web.Macros.MacroTagParser.ParseMacros(String text, Action`1 textFoundCallback, Action`2 macroFoundCallback) in D:\a\1\s\src\Umbraco.Web\Macros\MacroTagParser.cs:194
   Umbraco.Web.PropertyEditors.ValueConverters.RteMacroRenderingValueConverter.RenderRteMacros(String source, Boolean preview) in D:\a\1\s\src\Umbraco.Web\PropertyEditors\ValueConverters\RteMacroRenderingValueConverter.cs:51
   Umbraco.Web.PropertyEditors.ValueConverters.RteMacroRenderingValueConverter.ConvertSourceToIntermediate(IPublishedElement owner, IPublishedPropertyType propertyType, Object source, Boolean preview) in D:\a\1\s\src\Umbraco.Web\PropertyEditors\ValueConverters\RteMacroRenderingValueConverter.cs:81
   Umbraco.Core.Models.PublishedContent.PublishedPropertyType.ConvertSourceToInter(IPublishedElement owner, Object source, Boolean preview) in D:\a\1\s\src\Umbraco.Core\Models\PublishedContent\PublishedPropertyType.cs:208
   Umbraco.Web.PublishedCache.NuCache.Property.GetInterValue(String culture, String segment) in D:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\Property.cs:192
   Umbraco.Web.PublishedCache.NuCache.Property.GetXPathValue(String culture, String segment) in D:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\Property.cs:232
   Umbraco.Web.PublishedCache.NuCache.Navigable.NavigableContent.Value(Int32 index) in D:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\Navigable\NavigableContent.cs:74
   Umbraco.Core.Xml.XPath.NavigableNavigator.MoveToFirstChildProperty() in D:\a\1\s\src\Umbraco.Core\Xml\XPath\NavigableNavigator.cs:602
   Umbraco.Core.Xml.XPath.NavigableNavigator.MoveToFirstChild() in D:\a\1\s\src\Umbraco.Core\Xml\XPath\NavigableNavigator.cs:562
   MS.Internal.Xml.XPath.XPathDescendantIterator.MoveNext() +60
   MS.Internal.Xml.XPath.DescendantQuery.Advance() +185
   MS.Internal.Xml.XPath.FilterQuery.Advance() +39
   MS.Internal.Xml.XPath.XPathSelectionIterator.MoveNext() +17
   Umbraco.Web.PublishedCache.NuCache.<GetByXPath>d__31.MoveNext() in D:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\ContentCache.cs:358
   System.Collections.Generic.List`1..ctor(IEnumerable`1 collection) +246
   System.Linq.Enumerable.ToList(IEnumerable`1 source) +54
   Blog.Core.BlogRouting.BlogContentFinder.GetBlogPostContent(UmbracoContext umbracoContext, String slug, String category) in C:\Users\niels\source\repos\Blog\Blog.Core\BlogRouting\BlogPostRouteHandler.cs:60
   Blog.Core.BlogRouting.BlogContentFinder.TryFindContent(PublishedRequest request) in C:\Users\niels\source\repos\Blog\Blog.Core\BlogRouting\BlogPostRouteHandler.cs:45
   Umbraco.Web.Routing.<>c__DisplayClass19_0.<FindPublishedContent>b__0(IContentFinder finder) in D:\a\1\s\src\Umbraco.Web\Routing\PublishedRouter.cs:427
   System.Linq.Enumerable.Any(IEnumerable`1 source, Func`2 predicate) +140
   Umbraco.Web.Routing.PublishedRouter.FindPublishedContent(PublishedRequest request) in D:\a\1\s\src\Umbraco.Web\Routing\PublishedRouter.cs:424
   Umbraco.Web.Routing.PublishedRouter.FindPublishedContentAndTemplate(PublishedRequest request) in D:\a\1\s\src\Umbraco.Web\Routing\PublishedRouter.cs:386
   Umbraco.Web.Routing.PublishedRouter.PrepareRequest(PublishedRequest request) in D:\a\1\s\src\Umbraco.Web\Routing\PublishedRouter.cs:131
   Umbraco.Web.UmbracoInjectedModule.ProcessRequest(HttpContextBase httpContext) in D:\a\1\s\src\Umbraco.Web\UmbracoInjectedModule.cs:155
   Umbraco.Web.UmbracoInjectedModule.<Init>b__23_4(Object sender, EventArgs e) in D:\a\1\s\src\Umbraco.Web\UmbracoInjectedModule.cs:483
   System.Web.SyncEventExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute() +200
   System.Web.<>c__DisplayClass285_0.<ExecuteStepImpl>b__0() +24
   System.Web.StepInvoker.Invoke(Action executionStep) +100
   System.Web.<>c__DisplayClass4_0.<Invoke>b__0() +17
   Microsoft.AspNet.TelemetryCorrelation.TelemetryCorrelationHttpModule.OnExecuteRequestStep(HttpContextBase context, Action step) +64
   System.Web.<>c__DisplayClass284_0.<OnExecuteRequestStep>b__0(Action nextStepAction) +54
   System.Web.StepInvoker.Invoke(Action executionStep) +84
   System.Web.HttpApplication.ExecuteStepImpl(IExecutionStep step) +100
   System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously) +73

Steps to reproduce

Not sure if reproducible with a clean Umbraco 8 instance (instead of upgrading).
At minimum, I expect you'll need RTE with a Macro. The macro needs a parameter.

Expected result

I expect the macro to be rendered using my Razor macro code as it did before the upgrade.

Actual result

Stack trace above.

@Swimburger
Copy link
Contributor Author

Swimburger commented Aug 18, 2019

Same issue has been described on this Our Umbraco post by Damien.

@nul800sebastiaan
Copy link
Member

Does it help if you remove and re-insert the macro in the RTE? It seems like your current macro might be storing some unexpected data.

It would be good to see what data is stored for the macro currently and then try to update by removing and re-inserting the macro.

@nul800sebastiaan nul800sebastiaan added the state/needs-more-info We don't have enough information to give a good reply label Aug 19, 2019
@stevemegson
Copy link
Contributor

I think the problem is where the macro rendering is happening, rather than the macro itself.

RteMacroRenderingValueConverter is setting the current page to umbracoContext.PublishedRequest?.PublishedContent. If we're not currently rendering a page, this may be null (in this case we haven't finished routing the request, so we haven't yet identified the current page).

Even when it's not null, we'll be rendering the macro with Model.Content set to the page that's currently rendering rather than the page the macro is embedded in. We could pass in the right IPublishedContent if the RTE property is directly on a document, but if the RTE is in nested content then we're stuck.

However, I'm not sure that we should be rendering macros at all in GetXPathValue. V7's XML cache returns the stored RTE content without additional processing, shouldn't we do the same here?

@Swimburger
Copy link
Contributor Author

@nul800sebastiaan thanks for the suggestion.
I created a new document where there's no Macro in the RTE which works fine.
The moment I add a new fresh Macro in the RTE in the new document, the same issue occurs.

@stevemegson that sounds like it would apply to my case. My XPath query is happening in an IContentFinder.
When I Query the same document as mentioned above (without xpath, using Model.Children...) from a razor view, no exception is thrown, but the Macro isn't rendered.
Instead of the macro being rendered, the macro parameter value is printed.

@nul800sebastiaan
Copy link
Member

Ah okay, I understand now, the macro is failing because the code in your macro is no longer working. I though the migration might have forgotten about some macro configuration somewhere.

Alright, it seems like you might need to update the way you get your content for now.

Does something like this work for you?

umbracoContext.Content.GetByRoute(slug);

GetByRoute is properly cached so I don't expect any performance issues.

@Swimburger
Copy link
Contributor Author

So the Macro itself works fine when I add it to an RTE. The preview even renders properly and I can see it execute my Razor code. Only after publishing the issue occurs in my code.

Also will try your suggestion when I get home.

@Swimburger
Copy link
Contributor Author

Swimburger commented Aug 19, 2019

@nul800sebastiaan
When using GetByRoute, the exception is not thrown and works as expected.
Though, we should be able to still use the XPath queries, correct?

I noticed that the macros are being calculated at site startup and then cached.
This is different behavior than v7, isn't it? This seems like a large breaking change for anyone relying on macros being executed at HTTP request time instead of site startup.
This is what @stevemegson alluded to as well I believe.

@nul800sebastiaan
Copy link
Member

Great to hear!

There's loads of large breaking changes in v8 :-)

If this works for you then roll with it. We don't have an XML cache in v8 and getting by XPATH is not as performant as it was before.

If someone wants to fix this issue then feel free to help out if you can.

@nul800sebastiaan nul800sebastiaan added community/up-for-grabs help wanted and removed state/needs-more-info We don't have enough information to give a good reply labels Aug 20, 2019
@umbrabot
Copy link

Hi @Swimburger,

We're writing to let you know that we've added the Up For Grabs label to your issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly PR team bot :-)

@Swimburger
Copy link
Contributor Author

@nul800sebastiaan What's the high performance alternative for complex querying?

@nul800sebastiaan
Copy link
Member

Fixed in #6010
Cherry picked to 8.1.4 703092c

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

4 participants