aliostad / CacheCow

An implementation of HTTP Caching in .NET Core and 4.5.2+ for both the client and the server
MIT License
847 stars 172 forks source link

Implement ITimedETagQueryProvider using Ninject #253

Closed Afshin1980 closed 4 years ago

Afshin1980 commented 4 years ago

Hello, I'm trying to implement ITimedETagQueryProvider using Ninject in ASP.NET Web API, but I couldn't find any example. Would you please provide an example.

Thank you

aliostad commented 4 years ago

Well, implementing should not be any different. As for registering all IoC libraries have similar concepts, so for example in my samples, instead of:

Component.For<ITimedETagQueryProvider>().ImplementedBy<TimedETagQueryCarRepository>()
                    .LifestyleSingleton().IsDefault()

I guess it just becomes:

Component.Bind<ITimedETagQueryProvider>().To<TimedETagQueryCarRepository>()

No?

afshinhaftlangi commented 4 years ago

Thank you for your response. I tried that but it doesn't call the QueryAsync method. I get the following error in postman when I pass the If-None-Match.

{
    "Message": "An error has occurred.",
    "ExceptionMessage": "An item with the same key has already been added.",
    "ExceptionType": "System.ArgumentException",
    "StackTrace": "   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at CacheCow.Server.WebApi.HttpCacheAttribute.<OnActionExecutingAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Controllers.ActionFilterResult.<ExecuteAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Dispatcher.HttpControllerDispatcher.<SendAsync>d__15.MoveNext()"
}

I created a simple solution as an example here

TimedETagNinject.zip

aliostad commented 4 years ago

Sorry but there is so much not right in there, never used Ninject but I guess that HTTP module is doing something nasty. I can see the code is called twice, it does not resolve the dependencies, etc.

Bear in mind, you would normally want to include ViewModel = typeof(Car) on the attribute and also implement generic of the : ITimedETagQueryProvider<Car>.

Afshin1980 commented 4 years ago

I agree with you the issue is related to Ninject but the interesting thing is The Ninject resolve other dependencies, I tested the solution with Castle again. It works fine. Anyway, I will try to use Castle in my solution. Thank you

Afshin1980 commented 4 years ago

Another question. In your provided sample (CacheCow.Samples.WebApi.WithQueryAndIoc) if you create a new car with (C) and then use (L) to get the last item, my expectation is that the second time that we use (L) we should get the result from the cache and the method "cars/get/{id}" shouldn't be called. Is that right? but if you put a breakpoint in line 72 in HttpCacheAttribute, you will see the ETags are not matched. I don't know my expectation is right or wrong? basically what I need is a way to prevent calling my API for the second time by implementing ITimedETagQueryProvider

Thank you in advance

aliostad commented 4 years ago

Yes you are right, it is broken in Web API, it is working in MVC. Not sure how since it should have been covered by so many tests. Thanks for the tip, let me find out.

aliostad commented 4 years ago

OK problem was that for ITimedETagExtractor, default one (hashes the entire object graph) was being resolved instead of the one used by QueryProvider (which hashes just the last modified date) hence these two were generating different ETags for the same entity resulting in a non-match. Not sure when it broke but I can swear I had seen it working.

If you are supplying your QueryProvider, make sure the registration is done for both. Note how this works: unless it can resolve both, most likely you will get the default ones. In a real project with so many object types to return, you probably will register each separately and designate the type in attribute to guide IoC to resolve it correctly.

https://github.com/aliostad/CacheCow/blob/master/src/CacheCow.Server.WebApi/CachingRuntime.cs#L43

Afshin1980 commented 4 years ago

Thank you, it works now

Afshin1980 commented 4 years ago

Hi, Re to https://github.com/aliostad/CacheCow/issues/253#issuecomment-644232431 I have another issue. In your provided sample (CacheCow.Samples.WebApi.WithQueryAndIoc) if we add the ViewModelType =typeof(Car) on the method "cars/get/{id}" It prevents calling QueryAsync method in TimedETagQueryCarRepository Is there something wrong there?

aliostad commented 4 years ago

No as I explained in order for a successful resolution, both ITimedETagExtractor and ITimedETagQueryProvider for that generic needs to resolve otherwise non-generic gets resolved. If you use ViewModelType =typeof(Car) you should register generic types in the container.

Afshin1980 commented 4 years ago

Thank you for your response I tried your suggestion but still get the same result. I don't know what I'm missing. Here is my implementation in your sample (CacheCow.Samples.WebApi.WithQueryAndIoc)

Programs.cs
            container.Register(
                Component.For<CarController>().ImplementedBy<CarController>()
                    .LifestyleTransient(),
                Component.For<ITimedETagExtractor<Car>>().ImplementedBy<CarAndCollectionETagExtractor>()
                    .LifestyleSingleton().IsDefault(),
                Component.For<ITimedETagQueryProvider<Car>>().ImplementedBy<TimedETagQueryCarRepository>()
                    .LifestyleSingleton().IsDefault(),
                Component.For<ICarRepository>().Instance(InMemoryCarRepository.Instance),
                Component.For<ISerialiser>().ImplementedBy<IgnoreLoopJsonSerialiser>().IsDefault() // demonstrate how to replace

            );

CarContoller.cs

        [HttpGet]
        [HttpCache(DefaultExpirySeconds = 0,ViewModelType =typeof(Car))]
        public IHttpActionResult Get(int id)
        {
            var car = _repository.GetCar(id);
            return car == null
                ? (IHttpActionResult) new NotFoundResult(this)
                : Ok(car);
        }

TimedETagQueryCarRepository.cs


    public class TimedETagQueryCarRepository : ITimedETagQueryProvider, ITimedETagQueryProvider<Car>
    {
        private readonly ICarRepository _repository;

        public TimedETagQueryCarRepository(ICarRepository repository)
        {
            this._repository = repository;
        }

        public void Dispose()
        {
            // none
        }

        public Task<TimedEntityTagHeaderValue> QueryAsync(HttpActionContext context)
        {
            int? id = null;
            if (context.RequestContext.RouteData.Values.ContainsKey("id"))
                id = Convert.ToInt32(context.RequestContext.RouteData.Values["id"]);

            if (id.HasValue) // Get one car
            {
                var car = _repository.GetCar(id.Value);
                if (car != null)
                    return Task.FromResult(new TimedEntityTagHeaderValue(car.LastModified.ToETagString()));
                else
                    return Task.FromResult((TimedEntityTagHeaderValue)null);
            }
            else // all cars
            {
                return Task.FromResult(new TimedEntityTagHeaderValue(_repository.GetMaxLastModified().ToETagString(_repository.GetCount())));
            }
        }
    }
    public class CarAndCollectionETagExtractor : ITimedETagExtractor<Car>
    {
        public TimedEntityTagHeaderValue Extract(object viewModel)
        {
            var car = viewModel as Car;
            if(car != null)
                return new TimedEntityTagHeaderValue(car.LastModified.ToETagString());
            var cars = viewModel as IEnumerable<Car>;
            if (cars != null)
                return new TimedEntityTagHeaderValue(cars.GetMaxLastModified().ToETagString(cars.Count()));

            return null;
        }

        public TimedEntityTagHeaderValue Extract(Car viewModel)
        {
            //implementation
        }
    }
Afshin1980 commented 4 years ago

Hi @aliostad Did you have a chance to take look at my issue?

aliostad commented 4 years ago

Apologies for late reply. This is a bug! DefaultCacheDirectiveProvider<T> has a conditional compile which does not compile for .NET Framework hence the generic never gets resolved. I am releasing a fix.

aliostad commented 4 years ago

OK version 2.7.3 should have fixed it.

Afshin1980 commented 4 years ago

No worries, I re-tested your sample again with your new changes, I still have the same problem. As soon as I register the generic types in the container and add ViewModelType =typeof(Car) It prevents calling QueryAsync.

aliostad commented 4 years ago

Same problem?? Sorry I did not get, do you still have the same issue?

Afshin1980 commented 4 years ago

Yes, I have exact same issue

aliostad commented 4 years ago

OK, lemme try it

aliostad commented 4 years ago

OK, when I was fixing it last time I registered the generic DefaultCacheDirectiveProvider in the sample project hence it worked. I just had to add it to default registrations. v2.7.4 should fix it. Please check it our once you can.

Afshin1980 commented 4 years ago

Thank you for the fix! I will check it out and get back in a couple of days.

Afshin1980 commented 4 years ago

It works fine now! Thanks again for your help.

aliostad commented 4 years ago

Awesome! Apologies for it to take two fixes and so long.