Glass.Mapper icon indicating copy to clipboard operation
Glass.Mapper copied to clipboard

Issue with `SitecoreParentMapper` and caching

Open kevinbrechbuehl opened this issue 9 years ago • 8 comments
trafficstars

Hi,

I have a problem with Glass mapper caching. I try to describe the issue as good as possible. Given is the following base model:

public interface IBaseItem
{
    ...

    [SitecoreParent(IsLazy = true, InferType = true)]
    IBaseItem Parent { get; set; }

    ...
}

Where there are several content types which can be loaded into Parent (due to infertype). All of them have [SitecoreType(Cachable = true)]. To build my 3-level main navigation, I call the following code for each item:

protected bool IsAncestorOfItem(IBaseItem potentialAncestor, IBaseItem item)
{
    while (item != null)
    {
        if (item.ItemId == potentialAncestor.ItemId)
        {
            return true;
        }

        item = item.Parent;
    }

    return false;
}

With enabled Glass cache (new SitecoreContext { CacheEnabled = true };) this results in the following error:

Exception: System.NullReferenceException
Message: Object reference not set to an instance of an object.
Source: Glass.Mapper
   at Glass.Mapper.AbstractService.RunConfigurationPipeline(AbstractTypeCreationContext abstractTypeCreationContext)
   at Glass.Mapper.AbstractService.InstantiateObject(AbstractTypeCreationContext abstractTypeCreationContext)
   at Glass.Mapper.Sc.SitecoreService.CreateType(Type type, Item item, Boolean isLazy, Boolean inferType, Dictionary`2 parameters, Object[] constructorParameters)
   at Glass.Mapper.Sc.DataMappers.SitecoreParentMapper.MapToProperty(AbstractDataMappingContext mappingContext)
   at Glass.Mapper.Pipelines.ObjectConstruction.Tasks.CreateInterface.InterfacePropertyInterceptor.LoadValue(AbstractPropertyConfiguration propertyConfiguration)
   at Glass.Mapper.Pipelines.ObjectConstruction.Tasks.CreateInterface.InterfacePropertyInterceptor.Intercept(IInvocation invocation)
   at Castle.DynamicProxy.AbstractInvocation.Proceed()
   at Castle.Proxies.IOverviewPageProxy.get_Parent()
   at Feature.Navigation.Services.MainNavigationService.IsAncestorOfItem(IBaseItem potentialAncestor, IBaseItem item)
   .........

The error won't be thrown for all items. Usually the first item called after an app pool recycle works, all other calls doesn't work. If I disable Glass cache (new SitecoreContext { CacheEnabled = false };) I have no issues at all.

kevinbrechbuehl avatar Oct 03 '16 05:10 kevinbrechbuehl

Hey @aquasonic,

I've taken a quick peek at this, and I suspect this might be related to early disposal of the underlying SitecoreService. Have you by any chance registered the ISitecoreService/Context variants with a custom IoC container, and/or do you know if these would be tied to a limited lifetime scope (say, the HTTP Request)?

smithc avatar Oct 03 '16 15:10 smithc

Looking at the method then @smithc suggestion is the most likely. Normally if using a caching on a model we recommend the properties on the model aren't lazy loaded to avoid the service being disposed.

mikeedwards83 avatar Oct 03 '16 17:10 mikeedwards83

Hi guys,

Thanks for your replies.

Yes, I have registered my SitecoreContext via custom IoC container in a HTTP Request scope. But how can this be disposed before the request is finished? I mean, the call to my method is done pretty early in the request and there are other requests to the same context after the call which fails.

However, I have tried to remove all lazy loaded properties from all my models, but the error is still the same. I have a custom object construction task, could this be something which is wrong?

public class CreateViewModelTask : IObjectConstructionTask
{
    public virtual void Execute(ObjectConstructionArgs args)
    {
        // check that no other task has created an object
        // and that this is a model which should be resolved using the container
        if (args.Result != null || !typeof(IContainerModel).IsAssignableFrom(args.Configuration.Type)) return;

        // create instance using the container
        var model = Container.Resolve(args.Configuration.Type);

        // set the new object as the returned result
        args.Result = model;
    }
}

kevinbrechbuehl avatar Oct 03 '16 18:10 kevinbrechbuehl

The cache works across all requests, so it could be that a previous request has created requested a model but not the parent. This thread then finishes and disposes of the service. The next thread comes in a requests the same model but this time it needs the parent at which point you see the error.

When you say you you remove lazy loading did you set Lazy=False?

mikeedwards83 avatar Oct 03 '16 20:10 mikeedwards83

Ehm no ;-) I just completely the property, didn't see that true is the default value. And yes, of course both of you are right, when setting this explicitely to false then it works perfectly.

So the key message here is: Do not use lazy loading in properties when using Glass cache. Is there a way to globally disable lazy loading (also if this was set on a property)? Could this be disabled somewhere within the SitecoreService if cache is enabled?

Thanks for helping me out!

kevinbrechbuehl avatar Oct 04 '16 06:10 kevinbrechbuehl

I wan't to add another part to this ticket. In one place, we use a custom cache which caches Glass domain models over an infinite amount of time. The Glass cache is disabled completely in our case.

The models have multiple [SitecoreField] properties which reference other models. If these properties are not accessed within the first usage which adds them to the cache, the same issue happens with this mapper as well.

I saw that there is a setting Setting = SitecoreFieldSettings.DontLoadLazily for this mapper/attribute, but overall, the way of disable lazy loading is quite inconsistent across the different mappers. These issues started to happen after upgrading from Glass 3 to 4. We now just hold ID references in our cache instead of the whole domain model. But i would appreciate it as well, if the handling of Lazy would be optimized.

rootix avatar Nov 21 '16 09:11 rootix

Hi @rootix I am working on an implementation at the moment that means if the requested model is set to be cachable then lazy loading is disabled. This will remove the need to mark every property as lazy=false and I hope resolve the issue you have described.

mikeedwards83 avatar Nov 21 '16 10:11 mikeedwards83

@mikeedwards83 sounds very promising. But as at the moment we don't use the Glass cache because this is an upgraded project with our own caching logic, having to disable lazy trough the Cacheable property is not helping in this case i think. Will it take the property into account even if caching is disabled in glassCache site attribute? If yes, we can work around that, if not, we will have to change / remove the whole cache implementation.

rootix avatar Nov 21 '16 12:11 rootix