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

IDataPortalCache interface won't work as intended (and is uncompilable in an actual async context) #3635

Closed
ossendorf-at-hoelscher opened this issue Dec 27, 2023 · 6 comments · Fixed by #3636
Assignees
Labels

Comments

@ossendorf-at-hoelscher
Copy link

ossendorf-at-hoelscher commented Dec 27, 2023

Describe the bug
The current implementation of IDataPortalCache won't work in an actual async scenario.

The problem is:
Task Foo(p1, p2, out string p3) will compile just fine. But as soon as you try to implement it with async Task the compiler won't compile because out (and in or ref) parameters are illegal in an async declared method( c.f. https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/async#return-types first paragraph)

Further more: Is it intended that the cache will actually sequentialize all access to the server when objects are cacheable? (Given we are in one DI scope typically Blazor or WPF)

@rockfordlhotka
Copy link
Member

Good catch, thank you for finding that issue!

In our initial usage, the cache is actually synchronous, so there is no use of await. The intent is that await should work, so the API needs to be rethought.

Further more: Is it intended that the cache will actually sequentialize all access to the server when objects are cacheable? (Given we are in one DI scope typically Blazor or WPF)

The cache will be asked if it wants to deal with all root types as they flow through the data portal. The intent is that someone implementing the cache will be smart about how they choose the types to cache.

Specifically, I would expect that types containing reference data (immutable, versioned data) will be cached. Other types of data are typically mutable, and caching object graphs with mutable data is something that incurs great risk.

The cache is intentionally per-scope (per-user) because caching even a read-only CSLA object across users requires extra work. This is because the CSLA rules engine relies on the current user identity, which flows from ApplicationContext, which is dependent on the underlying scoped DI provider.

It is possible to have a server-wide cache (or use REDIS) if you know what you are doing. In this case, you'd need to serialize the object graphs and store the byte stream in the cache, then deserialize on request. The deserialization process would run in the requesting code's context, and so would get the correct DI provider, ApplicationContext manager, and current user.

This scenario is, in fact, the reason that an async API is necessary, because I don't know how someone might implement a shared cache of this type.

@ossendorf-at-hoelscher
Copy link
Author

ossendorf-at-hoelscher commented Dec 28, 2023

You're welcome :)

As it happens we have implemented this month a client side cache through an IDataPortal decorator. Inseatd of a Tester-Doer Pattern we opted (inspired by IMemoryCache) for a GetOrCreate() Method. So all of our fetches are routed through the decorator and based on (Type,Criteria) either it forwards the call to an actual caching implementation or just to the decorated IDataPortal. With that we don't have any problems with sequentializing access of different cacheable Types.
It can happen that cacheable stuff is retrieved more than once due to concurrency here but we prefer that over sequentializing all access to avoid that.

I've never worked with REDIS so I don't know if the described approach works with that. And I assumed (and expected) the cache scope per user. Otherwise you would be down into a rabbit hole with plentoras of problems 😄.

Yes only immutable or stuff that changes really rare are cached. If you like (and with permission from my superior) I can share our current "simple" implementation.

Btw do you have any mechanism for cache invalidation in mind? Currently the cache does not provide any method to remove an item from the cache.

@ossendorf-at-hoelscher
Copy link
Author

@rockfordlhotka I got permission to share our client side cache. Would you be interested?

@rockfordlhotka
Copy link
Member

@ossendorf-at-hoelscher yes, I am interested. Please either share in a Discussion thread, or if you want it private let me know and we can use email or discord.

@rockfordlhotka
Copy link
Member

Btw do you have any mechanism for cache invalidation in mind? Currently the cache does not provide any method to remove an item from the cache.

The interface is for use by the data portal, and the data portal has no way of knowing when or how you might invalidate cached items.

Nothing stops you from having another interface on your cache implementation (or just other methods) that are used ty invalidate items in the cache.

In the original issue (#3623) for implementing the cache concept, there is a simple code example for a cache implementation. That implementation invalidates cache items each time the cache is accessed.

@StefanOssendorf
Copy link
Contributor

@ossendorf-at-hoelscher yes, I am interested. Please either share in a Discussion thread, or if you want it private let me know and we can use email or discord.

I'll share it tomorrow (it's 10pm for me here) when I get back to work.

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

Successfully merging a pull request may close this issue.

3 participants