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

Using LazyItemEnumerable being used even with presence of Cachable=true & IsLazy = false #299

Closed
JamieG opened this issue May 12, 2017 · 10 comments
Assignees

Comments

@JamieG
Copy link

JamieG commented May 12, 2017

I'm seeing errors related to a disposed SitecoreContext trying to be used, this would make sense if I were trying to access a cached item that has lazily loaded properties but I've made every attempt to disable lazy loading in the solution. I've appended [SitecoreType(Cachable = true, AutoMap = true)] to every model in the project and [SitecoreChildren(IsLazy = false, InferType = true)] to every IEnumerable property and yet I can see in the errors that Glass.Mapper.Sc.LazyItemEnumerable`1.ProcessItems() is still being used, which then is cached resulting in the SitecoreContext going out of scope.

I've tried to follow all the instructions on the GlassMapper website to disable LazyLoading, could you please advise how I can prevent the solution using LazyLoading?

@mikeedwards83
Copy link
Owner

Hum, this is an interesting error. Are you using the SitecoreContext via and IOC container? Can the IOC container be disposing the SitecoreContext? Can you supply and example of the model with the SitecoreChildren property that is causing the problem?

@JamieG
Copy link
Author

JamieG commented May 15, 2017

Here is an example, perhaps an issue with the child collection being abstracted behind a generic interface? Other than that I can't see anything that deviates from a typical use case. I did have a look through the LazyItemEnumerable class and I see that it would still be used even if the activation happens immediately in the constructor due to the presence of the Cachable flag or global disable of Lazy Load, this wouldn't explain the call to the disposed SitecoreContext though.

The context is coming out of Autofac on a Per Request scope, assuming the execution is happening when it should that scope should be correct.

Any idea how much the context would grow over time if I registered as Singleton? Wouldn't be happy as a long term solution but might give some time to investigate further.

Registration
builder.RegisterType<SitecoreContext>().As<ISitecoreContext>().AsSelf().InstancePerRequest();

Factory, injected from constructor
private readonly Func<ISitecoreContext> _sitecoreContextFactory;

Setup and call

ISitecoreContext sitecore = _sitecoreContextFactory();
ISiteRoot siteRoot = sitecore.GetItem<ISiteRoot>(Context.Site.RootPath);
IEnumerable<SelectListItem> jobTypes = siteRoot.Helpers().JobTypes().ToSanatisedRadioList(i => i.LookupName, i => i.LookupName, searchFilter.JobTypes).ToList();
[SitecoreType(Cachable = true, AutoMap = true)]
public interface ISiteRoot
{
    [SitecoreField(FieldId = FieldId.SiteRoot.JobTypes)]
    ILookups<IJobType> JobTypes { get; }
}
public static class SiteRootExtensions
{
    public static SiteRootHelpers Helpers(this ISiteRoot item)
    {
        return new SiteRootHelpers(item);
    }       
}
public class SiteRootHelpers
{
    protected ISiteRoot Context { get; private set; }

    public SiteRootHelpers(ISiteRoot context)
    {
        Context = context;
    }

    public IEnumerable<IJobType> JobTypes()
    {
	try
	{
		if (Context.JobTypes?.Items != null)
			return Context.JobTypes.Items.ToList();
	}
	catch (Exception exception)
	{
		Log.Error("SiteRootHelpers.JobTypes", exception, this);
	}

	return new List<IJobType>();
    }   
}
[SitecoreType(Cachable = true, AutoMap = true, TemplateId = Shared.TemplateId.Lookups)]
public interface ILookups<out T> : IStandardTemplate where T : ILookup
{
    [SitecoreChildren(InferType = true, IsLazy = false)]
    IEnumerable<T> Items { get; } 
}
[SitecoreType(Cachable = true, AutoMap = true)]
public interface IJobType : ILookup
{ }
[SitecoreType(Cachable = true, AutoMap = true, TemplateId = TemplateId.Lookup)]
public interface ILookup
{
    [SitecoreField(FieldId = FieldId.Lookup.LookupName)]
    string LookupName { get; }

    [SitecoreField(FieldId = FieldId.Lookup.LookupDisplayName)]
    string LookupDisplayName { get; }

    [SitecoreField(FieldId = FieldId.Lookup.LookupIsDefault)]
    bool IsDefault { get; }
}

@mikeedwards83
Copy link
Owner

I have written a unit test to check the behaviour and the unit test seems to work ok. Can you provide a unit tests that shows the issue?

My unit tests:
952cff1

@mikeedwards83 mikeedwards83 self-assigned this Oct 27, 2017
@DerDreschner
Copy link

DerDreschner commented Nov 8, 2018

Well, we see this issue in our environments (development and testing, we're not yet on production - Sitecore 8.2.7 with .NET 4.6.1 and Glass.Mapper 4.5.0.4 on Windows Server 2016), too. Did you do some more investigations or is this ticket up-to-date, @mikeedwards83?

@niclasleonbock
Copy link

niclasleonbock commented Nov 10, 2018

Hey there,

we've been able to further narrow down the specific issue.

We're able to reliably reproduce the issue with the the following test case, which is based on the test cases proposed/provided by you:

[Test]
public void SitecoreService_NotLazyLoadingChildrenOfReferencedItem_EnumerationAfterServiceDisposal() {
    //Arrange
    using (Db db = new Db()
    {
        new DbItem("Root")
        {
            new DbField("Reference")
            {
                Value = "{564F2C6A-C171-4630-8A94-7A9749C00975}"
            }
        },
        new DbItem("Target", new ID("{564F2C6A-C171-4630-8A94-7A9749C00975}"))
        {
            new DbItem("Child1"),
            new DbItem("Child2")
        }
    }) {

        var resolver = Utilities.CreateStandardResolver();
        var context = Context.Create(resolver);
        var service = new SitecoreService(db.Database, context);

        //Act
        var result = service.GetItem<IStub>("/sitecore/content/Root");

        service.Dispose();

        //Assert
        Assert.AreEqual(true, result.Reference.Children.Any());
        Assert.AreEqual(2, result.Reference.Children.Count());
    }
}

Using the following stubs:

public interface IStub
{

    IStubNotLazy Reference { get; set; }

}

public interface IStubNotLazy
{

    [SitecoreChildren(IsLazy = false)]
    IEnumerable<IStubNotLazy> Children { get; set; }

    Guid Id { get; set; }

}

The test fails with the exception under discussion:

Glass.Mapper.MapperException : Service has been disposed, cannot create object
   at Glass.Mapper.AbstractService.InstantiateObject(AbstractTypeCreationContext abstractTypeCreationContext) in C:\Dev\Glass.Mapper\Source\Glass.Mapper\AbstractService.cs:line 142
   at Glass.Mapper.Sc.SitecoreService.CreateType(Type type, Item item, Boolean isLazy, Boolean inferType, Dictionary`2 parameters, Object[] constructorParameters) in C:\Dev\Glass.Mapper\Source\Glass.Mapper.Sc\SitecoreService.cs:line 501
   at Glass.Mapper.Sc.LazyItemEnumerable`1.ProcessItems() in C:\Dev\Glass.Mapper\Source\Glass.Mapper.Sc\LazyItemEnumerable.cs:line 95
   at Glass.Mapper.Sc.LazyItemEnumerable`1.<.ctor>b__6_0() in C:\Dev\Glass.Mapper\Source\Glass.Mapper.Sc\LazyItemEnumerable.cs:line 61
   at System.Lazy`1.CreateValue()
   at System.Lazy`1.LazyInitValue()
   at Glass.Mapper.Sc.LazyItemEnumerable`1..ctor(Func`1 getItems, Boolean isLazy, Boolean inferType, ISitecoreService service) in C:\Dev\Glass.Mapper\Source\Glass.Mapper.Sc\LazyItemEnumerable.cs:line 66
   at lambda_method(Closure , Object[] )
   at Glass.Mapper.Sc.DataMappers.SitecoreChildrenMapper.MapToProperty(AbstractDataMappingContext mappingContext) in C:\Dev\Glass.Mapper\Source\Glass.Mapper.Sc\DataMappers\SitecoreChildrenMapper.cs:line 86
   at Glass.Mapper.Pipelines.ObjectConstruction.Tasks.CreateInterface.InterfacePropertyInterceptor.LoadValue(AbstractPropertyConfiguration propertyConfiguration) in C:\Dev\Glass.Mapper\Source\Glass.Mapper\Pipelines\ObjectConstruction\Tasks\CreateInterface\InterfacePropertyInterceptor.cs:line 137
   at Glass.Mapper.Pipelines.ObjectConstruction.Tasks.CreateInterface.InterfacePropertyInterceptor.Intercept(IInvocation invocation) in C:\Dev\Glass.Mapper\Source\Glass.Mapper\Pipelines\ObjectConstruction\Tasks\CreateInterface\InterfacePropertyInterceptor.cs:line 102
   at Castle.DynamicProxy.AbstractInvocation.Proceed()
   at Castle.Proxies.IStubNotLazyProxy.get_Children()
   at Glass.Mapper.Sc.FakeDb.Issues.Issue299.Issue299Fixture.SitecoreService_NotLazyLoadingChildrenOfReferencedItem_EnumerationAfterServiceDisposal() in C:\Dev\Glass.Mapper\Tests\Unit Tests\Glass.Mapper.Sc.FakeDb\Issues\Issue299\Issue299Fixture.cs:line 49

As you can see, the issue only occurs when referencing from an item. It seems to be related to using lists/enumerables contained in typed referenced items. Simply accessing referenced items after service disposal works. To proof this, we adjusted the test case accordingly:

using (Db db = new Db()
    {
        new DbItem("Root")
        {
            new DbField("Reference")
            {
                Value = "{564F2C6A-C171-4630-8A94-7A9749C00975}"
            }
        },
        new DbItem("Target", new ID("{564F2C6A-C171-4630-8A94-7A9749C00975}"))
        {
            new DbItem("Child1"),
            new DbItem("Child2")
        }
    }) {

        var resolver = Utilities.CreateStandardResolver();
        var context = Context.Create(resolver);
        var service = new SitecoreService(db.Database, context);

        //Act
        var result = service.GetItem<IStub>("/sitecore/content/Root");

        service.Dispose();

        //Assert
        Assert.NotNull(result.Reference);
        Assert.AreEqual(result.Reference.Id, new Guid("{564F2C6A-C171-4630-8A94-7A9749C00975}"));
    }
}

This results in the test being green again.

We're running the suite with Sitecore 8.2 and Glass.Mapper on tag build-4.5.0.4-beta as that's the version currently in use.

For now, we'll use Guids in the "Root" model instead of the typed referenced models and fetch and cast the item manually. However, we'll still be looking into the issue in more detail and are hoping to be able to resolve it soon.

We're happy for any help or hints. Please let us know, if you have any "bright idea" where the issue may come from or need further information from us.

Best regards

Update: IEnumerable<IStubNotLazy> Children { get; set; } may be somewhat misleading. We used this to be closer to our code base. The issue also occurs when using IEnumerable<IStubChild> Children { get; set; } as in the original test cases or even IEnumerable<Item> Children { get; set; }

Update 2: Strangely, the behavior seems to be different when using class models instead of interfaces. In this case, result.Reference.Children is always null, even if the service has not been disposed. However, unfortunately I can not dig further into this issue.

Update 3: Concluding from the two issues, I suppose Glass.Mapper was never intended to be used that way. Is this conclusion correct? If so, I suppose you don't plan on accepting any changes in this direction in ("the old") version 4, right?

@Glass-Build-Github
Copy link
Collaborator

Glass-Build-Github commented Nov 10, 2018 via email

@DerDreschner
Copy link

Hey @Glass-Build-Github, did you already had some time left to take a look at the test case from @niclasleonbock ? Thanks for the feedback and sorry for any inconvenience in advance! David

@ted-duncan
Copy link

whatever became of looking next week?

@mikeedwards83
Copy link
Owner

@ted-duncan some very very long weeks.

In v5 I changed how islazy and other options are passed through the object graph. V4 could cause some inconsistencies on how the islazy flag is passed down the graph.

Additionally with V5 changes it had become possible to cache object with lazy loading enabled (assuming you don't use SitecoreContext).

I am going to close this issue but reopen it if you have further comments.

@ted-duncan
Copy link

@mikeedwards83, owe you an apology man, i came off sounding like a jerk and i totally get the long weeks comment. thanks for the response, cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants