-
Notifications
You must be signed in to change notification settings - Fork 43
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
(Binary) serialization of Taxonomies not supported #192
Comments
Tried to work around this issue using BSON data format but no go on that front as well. Attempted new serialization/deserialization routines:
New Error:
|
hi @xantari. I'm looking into it right now. |
Thanks so much! Let me know if you want any more of our code. |
Hi @xantari ,
This effectively prevents serialization and deserialization of Taxonomies. Let me please further understand your scenario so that we can come up with a good fix:
|
IDistributedCache interface which is what nearly all of the distributed cache providers all have Set() and SetAsync() methods that require raw binary data:
IDistributedCache requires binary data (byte[] as shown in Set methods in link above).
I just took your current CacheDeliveryClient from here: Then I changed the CacheManager to use the DistributedCache. Which means doing binary serialization of all the objects there. The CachingDeliveryClient.cs at the above URL is serializing the entire DeliveryTaxonomyResponse. |
Whats interesting is I just noticed the extension methods here: https://docs.microsoft.com/en-us/dotnet/api/microsoft.extensions.caching.distributed.idistributedcache?view=dotnet-plat-ext-3.1 Which show a bunch of strings. So perhaps I can try serializing all of that data as regular JSON strings instead. Will try that and update this issue request when I finish. |
Yup, that'd be great. There still might be a problem deserializing the taxonomies but a potential fix might be simpler than in the case of binary serialization. Thanks for the effort! |
Couldn't get it working with String's either. Get this error when trying to de-serialize DeliveryTaxonomyResponse.
It is erroring on this line: return JsonConvert.DeserializeObject(Encoding.UTF8.GetString(data));
Attached is my full DistributedCachingDeliveryClient.cs. |
Yes, I suspected that would happen. As I said earlier:
We know for sure that we'll need to fix the taxonomies. But I'm more curious whether the other objects/methods work fine - like If possible, I'd like to avoid using the Thank you! |
Couldn't get GetItemsAsync to work either. Tried this:
Then got the following error:
It seems the generated models are not compatible with serialization either. This was the command we used to generate our models:
If fails when it tries to deserialize the JSON data that is stored in the distributed cache:
Attached is what was in the JSON data that got returned from the distributed cache. |
I've been trying to figure out a way around this issue and started down the path of making the delivery client use it's own HttpClient that pulls it's json data from the distributed cache. My issue is that I don't fully understand how the cache eviction works with the current sample code and those SemaphoreSlims to know how to correlate it to some JSON data that is in the distributed cache based off of the unique URL that is used as the Cache Key in the distributed cache. I was inspecting the data within the CacheManager and it appears there is an ApiUrl property in the response. This would correlate exactly to what the cache key is that is being used in the HttpRequestCachingHandler shown below. So perhaps this might be a way to solve this problem, but I don't fully understand all the internals here to complete it.
The KenticoHttpClient:
The startup code:
|
@xantari thank you very much for all the information. Implementing IDistributedCache is much more complex than IMemoryCache (mostly due to the serialization). We now need to decide what level of support we want to incorporate in the SDK. Questions we are asking ourselves are:
Caching HTTP responses
Since Polly also supports caching, it could be a way to go for caching HTTP responses:
Disadvantages:
This can already be tested if you use the services.AddHttpClient("FactoryClient", c => { /* Do whatever else you wish here... */ })
.AddHttpMessageHandler<LoggingHandler>()
.AddTypedClient(c => DeliveryClientBuilder.WithOptions(...).WithHttpClient(c).Build())
.AddPolicyHandlerFromRegistry(/* see https://nodogmablog.bryanhogan.net/2018/11/caching-in-polly-6-and-the-httpclientfactory/ */); Caching strongly-typed models &
So the question now is: Do we want to support object-level caching as for IMemoryCache or are we ok to support caching of JSON HTTP responses for IDistributedCache? If you have any input on this @xantari please let us know or we can schedule a call if you'd be up for it. |
Our main goal is as follows:
The caching of HTTP JSON data using Polly would work fine as long as there was a way to remove the entire cache when a change occurred so that it could start building up the cache again. However you need to know all the cache keys Webserver Node 1, Webserver Node 2, and Webserver Node 3 put into the cache for instance. (example of a 3 node web server). So when the webhook came in, how would you know how to clear the cache? Even keeping track of keys on each node is hard, because Node 1 might only have 15 of the keys, Node 2 might have 30 of the keys, and Node 3 has 12 of the cache keys. If there is a way to simply clear the entire distributed cache then we could perhaps easily solve this with Polly and when an incoming webhook came in, we just clear the entire cache so that all nodes start to re-fetch content again. That is why I was trying to get it to work with what you already have working with IMemoryCache. Because it has all the dependency stuff figured out already and seems to figure out the depedency keys to evict. It would have provided the best solution as It would have just been a matter of swapping out IMemoryCache for IDistributedCache if it weren't for the serialization issue. |
thanks for the video call, the motivation is now clear and let's treat this as a more generic issue of supporting distributed caching in #196 |
Brief bug description
I am attempting to cache the objects using a variant of your Webhooks version of the CachingDeliveryClient that I have modified to store into a distributed cache. But it seems the objects are not marked as serializable :(
I get this error when attempting to store my object into our distributed cache:
Repro steps
Try serialize a DeliveryTaxonomyResponse to a distributed cache.
We use this code to take care of the serialization/deserialization:
Expected behavior
All Kentico Kontent models should be serializable.
Test environment
The text was updated successfully, but these errors were encountered: