msgraph-sdk-javascript icon indicating copy to clipboard operation
msgraph-sdk-javascript copied to clipboard

Add support for caching in the SDK

Open waldekmastykarz opened this issue 3 years ago • 9 comments

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

waldekmastykarz avatar Apr 08 '21 07:04 waldekmastykarz

Links in the issue are 404'ing.

andrewconnell avatar Apr 08 '21 09:04 andrewconnell

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

waldekmastykarz avatar Apr 08 '21 09:04 waldekmastykarz

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.

darrelmiller avatar Apr 08 '21 12:04 darrelmiller

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

waldekmastykarz avatar Apr 08 '21 13:04 waldekmastykarz

pontificating... 🤔

  • How would that work when using the JS SDK server-side (Node.js) when window === undefined?
  • While I get this discussion is happening in the JS SDK, any consideration to others? Or are we seeing a divergence of SDK capabilities? 🤢
  • Is this more of a pattern the SDK(s) will offer developers to implement if they choose so?

andrewconnell avatar Apr 08 '21 14:04 andrewconnell

  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.

baywet avatar Apr 08 '21 14:04 baywet

@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.

darrelmiller avatar Apr 10 '21 00:04 darrelmiller

@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 avatar Jul 23 '21 15:07 baywet

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

baywet avatar Jul 23 '21 17:07 baywet