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

Use a generic ICache interface for caching implementation #58

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

keshavkaul
Copy link

Motivation:

  1. I want to use redis cache using Easycaching.Redis library and while deserialising CachedResult class, I get an error when converting using Newtonsoft.Json serializer because deserialisation target type can't be found.

I've taken careful measures to not break previous library version.
Request to accept the change or provide comments for improvement.

@moozzyk
Copy link
Owner

moozzyk commented Aug 4, 2023

Thanks for your pull request. Unfortunately, this issue is lacking details that would explain why this change is needed. In addition, the ICache interface is public and is implemented by at least two caching solutions EFCache.Redis and DacheCache and this PR is modifying this interface breaking these implementations.

@keshavkaul
Copy link
Author

@moozzyk Thanks for replying. I'll share the exception I got when I used Newtonsoft.Json serializer, I don't have that at the moment.

Wrt. me changing ICache interface by removing InvalidateItem
The library was not publicly using this interface method. As the library at the moment does not have a way to invalidate specific items from cache.

I'm also handling the case where other library users can still use ICache as I'm handling here when getting the item from cache.

Also, I see that DacheCache is flagging itself as archived. EFCache.Redis I believe is not in active development and they're using Binaryformatter serializer which Microsoft doesn't recommend to use nowadays due to security reasons. See here

@keshavkaul
Copy link
Author

keshavkaul commented Aug 5, 2023

Here's the error I got when deserialising the CachedResults object.
Serialising works well. But during deserialising, the serializer needs to know the target type into which to form the object.
So now, the serializer is assuming a default Object type. And when casting to CachedResults, we get the below error.

Screenshot 2023-08-05 at 3 16 15 PM Screenshot 2023-08-05 at 3 16 36 PM

@moozzyk
Copy link
Owner

moozzyk commented Aug 5, 2023

I am not interested in breaking the interface, sorry.

Looking at the exception it seems you didn't return the object you were given.You got an object the CachedResults type but you're giving back an object of the JObject type. I think the biggest problem is that the CachedResults class is internal which may prevent deserialization.

Realistically, EF6 is a legacy product at this point and I may push a new version only in case of a severe issue. If you need your changes feel free to fork the repo and build a private package with whatever functionality you need.

@keshavkaul
Copy link
Author

@moozzyk No worries, Thanks for your time. Really appreciate all your efforts 😊

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

Successfully merging this pull request may close these issues.

2 participants