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

View rendering with unpublished datasource item resulting in error #163

Closed
hiraldesai opened this Issue Aug 3, 2015 · 16 comments

Comments

Projects
None yet
6 participants
@hiraldesai
Copy link

hiraldesai commented Aug 3, 2015

I'm using interfaces as my viewmodels for a hybrid WebForms and MVC Sitecore project. All my view renderings have their "Model" field left empty with views having @inherits GlassView<IModel> in them to dynamically bind data to them. This approach works fine except for an edge case scenario where the datasource item defined for the view rendering is not published resulting in this "The model item passed into the dictionary is of type 'Sitecore.Mvc.Presentation.RenderingModel', but this dictionary requires a model item of type IModel" error.

This error happens because both GetModel and GetModelFromView result in null passing the control back to Sitecore which then passes an item of type RenderingModel to the view.

To get around this - I tried setting "Model" field on the view rendering to the type of the model (e.g. NameSpace.IModel, Website - but that results in "Constructor on type Namespace.IModel not found" error.

Can someone please advise if 1) I'm doing something wrong here 2) there is any way to get around this problem?

@hiraldesai

This comment has been minimized.

Copy link
Author

hiraldesai commented Aug 3, 2015

Update: I can get around the issue by replacing GetModel in the pipeline with my custom implementation (subclassing Glass Mapper's GetModel) that aborts the pipeline regardless of model being null in the Process method. But that will mean it won't execute any Sitecore pipelines for views that don't use glass. So it's not really going to solve my problem.

@hiraldesai

This comment has been minimized.

Copy link
Author

hiraldesai commented Aug 4, 2015

After debugging through the code - I think the issue lies in this method:

        protected virtual object GetFromField(Rendering rendering, GetModelArgs args)
        {
            Item obj = rendering.RenderingItem.ValueOrDefault(i => i.InnerItem);
            if (obj == null)
                return null;
            return rendering.Item == null
                ? null
                : GetObject(obj[ModelField], rendering.Item.Database, rendering);
        }

Basically rendering.Item is null but rendering.DataSource is set to an item id that only exists in the master database (unpublished item). Ideally, I'd like an empty model of the specified type returned here instead of Sitecore's getModel pipelines picking it up and resulting in errors.

@mikeedwards83

This comment has been minimized.

Copy link
Owner

mikeedwards83 commented Aug 4, 2015

For me I think the SC logic is incorrect.

Within the GetModel pipeline you have the processor:

Sitecore.Mvc.Pipelines.Response.GetModel.CreateDefaultRenderingModel, Sitecore.Mvc

Which forces the standard RenderingModel without any checks. This to me is incorrect, if the item / datasource is null then the model should be null.

I would try removing this entry to see if that solves your problem.

I will get some opinions of other community members
@cardinal252 @csteeg @aquasonic @SaintSkeeta can you comment on this? What do you think?

@aquasonic

This comment has been minimized.

Copy link
Contributor

aquasonic commented Aug 4, 2015

I think the idea of Sitecore is here similar to Database.GetItem(), which also doesn't return null when an item version does not exist. But I share the thoughts from @mikeedwards83 that the model should be null if the item / datasource is null.

I don't think this is something Glass should handle. If I needed to solve this issue I would probably override the CreateDefaultModel() method in the CreateDefaultRenderingModel processor and return null if the desired model is a Glass model (or just remove this pipeline processor as Mike proposed).

@hiraldesai

This comment has been minimized.

Copy link
Author

hiraldesai commented Aug 5, 2015

Thank you for your feedback guys - I will try these options out. From what I've tried so far - just removing CreateDefaultRenderingModel won't help because Sitecore will fail later in the pipeline on args.ModelLocator.GetModel(obj["Model"], true) of GetFromRenderingItem pipeline because it doesn't create proxies like Glass does.

[MissingMethodException: Constructor on type 'NameSpace.IModel' not found.]
   System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture, Object[] activationAttributes, StackCrawlMark& stackMark) +1528
   System.Activator.CreateInstance(Type type, BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture, Object[] activationAttributes) +191
   System.Activator.CreateInstance(Type type, BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture) +27
   Sitecore.Mvc.Presentation.ModelLocator.GetModelFromTypeName(String typeName, String model, Boolean throwOnTypeCreationError) +101
   Sitecore.Mvc.Pipelines.Response.GetModel.GetFromRenderingItem.Process(GetModelArgs args) +38

@aquasonic - can you please advise what's the best way to check "if the desired model is a Glass model"?

@hiraldesai

This comment has been minimized.

Copy link
Author

hiraldesai commented Aug 5, 2015

Update: as expected it failed at a few places along the pipeline. These are the places where I ended up patching the Sitecore pipelines.

  1. Can I get your opinion on whether my checks for returning null results in these methods are correct?
  2. I'm quite surprised nobody has encountered this problem before - I might get Sitecore support to look and see if they have anything to say about this
    <!--<processor type="Sitecore.Mvc.Pipelines.Response.GetModel.GetFromRenderingItem, Sitecore.Mvc"/>-->
    public class CreateDefaultRenderingModelIfValid : CreateDefaultRenderingModel
    {
        protected override object CreateDefaultModel(Rendering rendering, GetModelArgs args)
        {
            // Not quite sure about this condition - get community opinion - maybe only abort if the view has @inherits GlassView<IModel>
            return rendering.Item == null || rendering.RenderingItem.ValueOrDefault(i => i.InnerItem) == null
                ? null
                : base.CreateDefaultModel(rendering, args);
        }
    }

    <!--<processor type="Sitecore.Mvc.Pipelines.Response.GetModel.CreateDefaultRenderingModel, Sitecore.Mvc"/>-->
    public class GetFromRenderingItemIfValid : GetFromRenderingItem
    {
        protected override object GetFromField(Rendering rendering, GetModelArgs args)
        {
            // Not quite sure about this condition - get community opinion - maybe only abort if the view has @inherits GlassView<IModel>
            return rendering.Item == null || rendering.RenderingItem.ValueOrDefault(i => i.InnerItem) == null
                ? null
                : base.GetFromField(rendering, args);
        }
    }

    <!--<processor type="Sitecore.Mvc.Pipelines.Response.GetRenderer.GetViewRenderer, Sitecore.Mvc"/>-->
    public class GetViewRendererIfValid : GetViewRenderer
    {
        protected override string GetViewPath(Rendering rendering, GetRendererArgs args)
        {             
                // Not quite sure about this condition - get community opinion - maybe only abort if the view has @inherits GlassView<IModel>
                return rendering.Item == null || rendering.RenderingItem.ValueOrDefault(i => i.InnerItem) == null
                ? null
                : base.GetViewPath(rendering, args);
        }
    }
@aquasonic

This comment has been minimized.

Copy link
Contributor

aquasonic commented Aug 7, 2015

@hiraldesai you could create a base class/interface IItem or similar and check if the model in the view inherits/implements of this base class/interface. If yes, it's a Glass model :)

@mikeedwards83

This comment has been minimized.

Copy link
Owner

mikeedwards83 commented Aug 11, 2015

I have just tried this on an SC8 instance and don't get the problem.

Which SC version are you using?

@mikeedwards83

This comment has been minimized.

Copy link
Owner

mikeedwards83 commented Aug 11, 2015

Ok this only relates to scenarios where the Model is derived from the CSHTML file using GetModelFromView

@hiraldesai

This comment has been minimized.

Copy link
Author

hiraldesai commented Aug 12, 2015

@aquasonic - Thanks. @mikeedwards83 - I'm using 7.2.

Instead of trying to patch at a few places like above, I was trying to find one place to patch this (or patch in as few places as possible). I went down the path of creating a custom model locator and a few other things but in the end it just turned out to be as simple as this:

<configuration xmlns:patch="http://www.sitecore.net/xmlconfig/">
  <sitecore>
    <pipelines>
      <mvc.getRenderer>
        <processor type="Namespace.To.Sitecore.Pipelines.Response.GetRenderer.GetViewRendererWithItemValidation, Library"
patch:instead="processor[@type='Sitecore.Mvc.Pipelines.Response.GetRenderer.GetViewRenderer, Sitecore.Mvc']" />
      </mvc.getRenderer>       
    </pipelines>
  </sitecore>
</configuration>
    public class GetViewRendererWithItemValidation : GetViewRenderer
    {        
        protected override Renderer GetRenderer(Rendering rendering, GetRendererArgs args)
        {           
            var viewRenderer = base.GetRenderer(rendering, args) as ViewRenderer;
            if (viewRenderer == null)
                return null;

            // Ignore item check when in page editor
            if (Context.PageMode.IsPageEditor || Context.PageMode.IsPageEditorEditing)
                return viewRenderer;

            // Override renderer to null when there is an unpublished item refererenced by underlying view
            return viewRenderer.Rendering.Item != null && viewRenderer.Rendering.RenderingItem.ValueOrDefault(i => i.InnerItem) != null
                ? viewRenderer
                : null;
        }
    }

Initial tests show that it's not breaking anything - I will keep testing and share this with Sitecore team too later.

Edit: Made a slight adjustment to stop page editor from throwing error as per #144

@mikeedwards83

This comment has been minimized.

Copy link
Owner

mikeedwards83 commented Aug 24, 2015

Hi

Do you mind if I add this to the Glass.Mapper site as a blog post/documentation but giving you the credit?

Mike

@hiraldesai

This comment has been minimized.

Copy link
Author

hiraldesai commented Aug 24, 2015

No problems Mike!

On Mon, 24 Aug 2015 at 4:45 pm Mike Edwards notifications@github.com
wrote:

Hi

Do you mind if I add this to the Glass.Mapper site as a blog
post/documentation but giving you the credit?

Mike


Reply to this email directly or view it on GitHub
#163 (comment)
.

@fluxdigital

This comment has been minimized.

Copy link

fluxdigital commented Aug 10, 2016

@mikeedwards83 This is great, it fixed problems with our content editors removing components and leaving broken links to data sources. However Ideally what I would like to do is render out a message in place of the component when in editing mode to say something like 'incorrect data source configured, please edit'. I looked at doing this by extending glassview, e.g: http://www.chrissulham.com/protect-your-sitecore-renderings-from-bad-datasources/. However it seems if the rendering is returned as null then It will never hit my baseglassview as it isn't even trying to render the component anymore. Any ideas how to do this in GetViewRendererWithItemValidation instead?

@fluxdigital

This comment has been minimized.

Copy link

fluxdigital commented Aug 11, 2016

@mikeedwards83 I did something like this in the end to handle errors and invalid datasources: http://www.hhogdev.com/blog/2015/june/mvc-rendering-exception-handler.aspx

@Swimburger

This comment has been minimized.

Copy link

Swimburger commented Jan 9, 2017

@mikeedwards83 I am having the same issue because my DataSource isn't published when using a ViewRendering. I tried using z.Glass.Mapper.Sc.ViewRender.config, but now I get a NullReferenceException.

[NullReferenceException: Object reference not set to an instance of an object.]
   Sitecore.Mvc.Pipelines.Response.RenderPlaceholder.PerformRendering.CreateCyclePreventer(String placeholderName, Rendering rendering) +39
   Sitecore.Mvc.Pipelines.Response.RenderPlaceholder.PerformRendering.Render(String placeholderName, TextWriter writer, RenderPlaceholderArgs args) +154
   (Object , Object[] ) +73
   Sitecore.Pipelines.CorePipeline.Run(PipelineArgs args) +483
   Sitecore.Pipelines.DefaultCorePipelineManager.Run(String pipelineName, PipelineArgs args, String pipelineDomain) +21
   Sitecore.Mvc.Pipelines.PipelineService.RunPipeline(String pipelineName, TArgs args) +192
   Sitecore.Mvc.Helpers.SitecoreHelper.Placeholder(String placeholderName) +258
   XXX.Sitecore.DynamicPlaceholder.Mvc.SitecoreHelper.DynamicPlaceholder(SitecoreHelper helper, String key) in c:\Projects\Packages\XXX.Sitecore.DynamicPlaceholder\Mvc\SitecoreHelper.cs:17
   ASP._Page_Areas_Main_Views_Grid_2ColumnGrid6_6_cshtml.Execute() in C:\projects\XXX\website\Areas\Main\Views\Grid\2ColumnGrid6_6.cshtml:3
   System.Web.WebPages.WebPageBase.ExecutePageHierarchy() +252
   System.Web.Mvc.WebViewPage.ExecutePageHierarchy() +147
   System.Web.WebPages.WebPageBase.ExecutePageHierarchy(WebPageContext pageContext, TextWriter writer, WebPageRenderingBase startPage) +121
   System.Web.Mvc.Html.PartialExtensions.Partial(HtmlHelper htmlHelper, String partialViewName, Object model, ViewDataDictionary viewData) +136
   Sitecore.Mvc.Presentation.ViewRenderer.Render(TextWriter writer) +342

At the top of the file it tells me to "update the code in the GlassMapperScCustom", though I have no idea what to add/update inside this class.
I didn't really find any documentation, can you point me to documentation on how to use the GetViewRendererWithItemValidation?

@andreicvasniuc

This comment has been minimized.

Copy link

andreicvasniuc commented Feb 2, 2018

Hello,
It seems I have the same bug or a similar one:

Inner Exception: The model item passed into the dictionary is of type 'Sitecore.Mvc.Presentation.RenderingModel', but this dictionary requires a model item of type 'Feature.Model'.

I am using Sitecore 8.2.170728 and GlassMapper 4.4.0.199. When a Datasource for View rendering is empty, Sitecore returns the Context.Item (page item) instead. I would like to get just null in normal and edit modes. So I have decided to override the Process method in GetModelFromView and to create an instance of NullModel and to assign to model for my case. I also added a new DeleteNulModel processor to the end of getModel pipeline in order to find my case and to assign null to the model. After doing some debugging until the line of code when a render renders it in ExecuteRenderer, the model is still null. But when I let it go to render it I get the error above. I don't get when Sitecore re-assigns it. The last getModel processor set it to null for sure.

Can anyone help me please to figure out what is going on?

Thanks
Andrei

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment