search

microsoftgraph / msgraph-sdk-javascript

Microsoft Graph client library for JavaScript
https://graph.microsoft.com
MIT License
756 stars 230 forks source link

Add support for caching in the SDK #417

Open waldekmastykarz opened 3 years ago

waldekmastykarz commented 3 years ago

Feature Request

Is your feature request related to a problem? Please describe

Say you're building an app connected to Microsoft 365 where you want to show your colleagues, upcoming meetings and recent files. All these objects have information about users on them. If for each person you wanted to show their name, photo and presence, you might end up requesting the same data from Graph multiple times. If you navigate between pages in your app, you'd keep unnecessarily downloading the same information over and over again.

Describe the solution you'd like

It would be invaluable if the SDK allowed us to cache previously retrieved data. If the data has been retrieved previously and it hasn't exceeded its expiration time, it would be loaded from the cache without calling Graph. That would make a huge difference in the app's performance and would probably help developers avoid throttling in data-heavy apps.

Similar functionality is already available in the Microsoft Graph Toolkit and it would be great if it was also available in the Graph SDK in cases developers can't use MGT.

Describe alternatives you've considered

I've experimented with building a Cache middleware for the Graph SDK which you can find here: https://gist.github.com/waldekmastykarz/654157658bc905e263ddd54c24fef7e2. The cache middleware allows you to configure cache settings on a Graph path so that you can configure different cache settings for different resources. You can also choose not to cache specific requests. The middleware supports caching JSON as well as binary data (like user's photo).

Is this something that you'd consider including in the Graph SDK? AB#8900

andrewconnell commented 3 years ago

Links in the issue are 404'ing.

waldekmastykarz commented 3 years ago

Sorry, my bad. Pointed to a private repo. Link updated.

darrelmiller commented 3 years ago

Yes, absolutely. This would be awesome, and has been on our backlog for way too long.

The default behavior should respect the request and response Cache-Control header and can fall back to heuristics for expiry times if there is no Cache-Control header. I would like the caching middleware to be in a position where services can define the default expiry times of resources and clients can override those times if they want to. I don't want to get into the position of relying on clients to set expiration policies for everything.

I have an implementation of RFC 7234 in .NET here https://github.com/tavis-software/Tavis.HttpCache/blob/master/src/HttpCache/HttpCacheHandler.cs#L22

I don't think we need fully implement the vary header, that allows for vary headers that change over time. I tried to do it in the C# implementation but it massively overcomplicates the algorithm. The standard caching rules are described here https://apisyouwonthate.com/blog/caching-is-hard-draw-me-a-picture

We will need to account for vary: accept-language but we might not need to account for vary: accept-encoding depending on how fetch handles content-encoding.

waldekmastykarz commented 3 years ago

Awesome suggestion! I haven't touched the cache-control header at all in my POC, but it would be great to have in there like you mentioned @darrelmiller

andrewconnell commented 3 years ago

pontificating... 🤔

baywet commented 3 years ago
  1. We should define an abstraction layer, and have some kind of factory that provides the session storage implementation when running in a browser or some kind of in memory hashmap for backend/native apps.
  2. I don't see any reason for the Java/Dotnet/... SDKs not to have it. Especially considering such languages can be used for native apps (android/xamarin/...), front end apps (blazor), or low connectivity scenarios (IoT).
  3. One of the very first steps would probably to specify a new middleware in the design guidelines.
darrelmiller commented 3 years ago

@baywet Creating an abstraction layer for storage is something we should do for all language implementations. Sometimes an in-memory cache will be appropriate, other times a persistent cache in some secure storage would be a better option. In the case of a web farm, you may even have a distributed cache. I implemented this in the .NET version I did but having to implement a cache that "properly" implements the vary header makes the storage stupidly hard. Most implementations don't both and I would recommend we don't bother either.

It definitely should be a goal to have this available on all platforms.

baywet commented 3 years ago

@roinochieng @darrelmiller as the next step would be to document the spec in the sdk deisgn guidelines, would you rather create an issue over there, or transfer this one?

baywet commented 3 years ago

some additional resources to put the spec together https://apisyouwonthate.com/blog/caching-is-hard-draw-me-a-picture https://github.com/tavis-software/Tavis.HttpCache https://httpwg.org/specs/rfc7234.html