SolidOS / solidos

The operating system for Solid
https://solidos.solidcommunity.net/
MIT License
127 stars 19 forks source link

Provide user-override for fetch method #72

Closed jeff-zucker closed 2 years ago

jeff-zucker commented 3 years ago

In rdflib, when a fetcher is defined, there is the possibility for users to define their own fetch method. This can happen with an option when creating the fetcher e.g. fetcher = $rdf.fetcher( store , { fetch : fetch_method }) OR it can be overridden by the user/app by setting a global or window variable in the app to define a different fetch method. This is key for using rdflib outside a browser because we need to define a file-based fetch method but is also needed for example if someone writes a fetch method for Dropbox or other storage.

Currently the global variable to override fetch is named window.solidFetcher. This is wrong - fetcher is the object, fetch is the method and this variable overrides the method, not the object. So that needs changing in rdflib to look for window.solidFetch instead of window.solidFetcher. I guess, for legacy we should support both?

And, we need to allow this user override every place we manually define a new fetcher rather than calling rdflib's method of creating a fetcher. This happens in solid-logic where a fetcher is created using either a hard-coded solid-authn-js fetch method or the window.fetch method. It needs to be modified to accept a user-defined window.solidFetch (OR global.solidFetch in out-of-browser) method. In addition, solid-logic also uses "fetcher" when it means "fetch" so it needs to be changed there too.

bourgeoa commented 3 years ago

I suppose you meant solidFetcher and solidFetch with a small s

jeff-zucker commented 3 years ago

Yes, I corrected my post, thanks.

bourgeoa commented 3 years ago

can you review https://github.com/linkeddata/rdflib.js/pull/530

bourgeoa commented 3 years ago

This is what I propose for solid-ui/src/logic.ts. To build it locally not having a latest rdflib update I temporarily added the declare global.

root@DESKTOP-PIRJOCS:/mnt/d/github/solid/solidos/workspaces/solid-ui# git diff
diff --git a/src/logic.ts b/src/logic.ts
index 3e189cb8..7178fad9 100644
--- a/src/logic.ts
+++ b/src/logic.ts
@@ -5,11 +5,25 @@ import * as debug from './debug'
 import authSession from './authn/authSession'
 import { SolidLogic } from 'solid-logic'

+declare global {
+  interface Window {
+    solidFetch?: any
+  }
+  var solidFetch: Function
+}
+
 const fetcher = async (url, requestInit) => {
   if (authSession.info.webId) {
     return authSession.fetch(url, requestInit)
   } else {
-    return window.fetch(url, requestInit)
+    // solidFetcher is deprecated
+    const _fetch = (typeof global !== 'undefined' && (global.solidFetch || global.solidFetcher)) ||
+      (typeof window !== 'undefined' && (window.solidFetch || window.solidFetcher)) ||
+      window.fetch
+    if (!_fetch) {
+      throw new Error('No _fetch function available for Fetcher')
+    }
+    return _fetch(url, requestInit)
   }
 }

(END)
bourgeoa commented 3 years ago

I have not seen anything to do in solid-logic. fetcher is used as fetcher.fetch and there is no creation of any fetch function, like fetch = window.fetch or rdf.fetcher

jeff-zucker commented 3 years ago

No, I do not believe that code belongs in solid-logic. All of the creation of the liveStore and the fetcher should happen in rdflib, not in solid-logic. See https://github.com/linkeddata/rdflib.js/issues/528

jeff-zucker commented 3 years ago

there is no creation of any fetch function, like fetch = window.fetch or rdf.fetcher

Right, and that's the problem - there is no way for a user or app to declare a different fetch method.

bourgeoa commented 3 years ago

This is what I propose for solid-ui/src/logic.ts.

Have you look at this 4th comment above

Right, and that's the problem - there is no way for a user or app to declare a different fetch method.

Not sure to understand the argument. Why can't you call new SolidLogic({ fetch: _fetch }, authSession) from solid-logic. With_fetch being any fetch method you want. This is creating the liveStore.

If there are reasons to create liveStore without SolidLogic then it is a different issue.

jeff-zucker commented 3 years ago

Not sure to understand the argument.

Supporting redefinition of fetch in the solid-logic constructor is not adequate. Apps which make use of SolidOS as a whole need a way to redefine fetch from outside of SolidOS. For example, where would an app using solid-file-client call that constructor? But this is a moot point since Tim has said that this whole part of solid-logic should be moved back into rdflib.

TallTed commented 3 years ago

Have you look at this 4th comment above

@bourgeoa -- I don't know about anyone else, but I am not at all clear which comment you meant by the above string. Please note that the timestamp displayed on each comment is an <a> element with a direct and specific href which you can copy and paste to provide a specific link to the comment you're referencing -- as I'm doing here, to your comment containing the above string.

bourgeoa commented 3 years ago

@TallTed You are right I should not have done that with the github phone app where this function is apparently missing.

bourgeoa commented 3 years ago

@jeff-zucker link to my proposal for solid-ui https://github.com/solid/solidos/issues/72#issuecomment-976813963

jeff-zucker commented 3 years ago

@bourgeoa - Tim has very clearly said that he wants the LiveStore stuff removed from solid-logic and put into rdflib (see https://github.com/linkeddata/rdflib.js/issues/528). I do not understand why you continue to propose changes to solid-logic,

jeff-zucker commented 3 years ago

Also, solid-ui can not work on the command line so using global (which only relates to the command-line) here rather than in rdflib where it belongs will not accomplish anything.

jeff-zucker commented 2 years ago

The method @bourgeoa proposes is not the way it should be done. This only overrides the fetcher when the user isn't logged in We need the ability to always override the fetcher. This will be critical for cloud storage and local-first neither of which will use Sold authenticated fetches.

timea-solid commented 2 years ago

I took a look at the code and structure. https://github.com/linkeddata/rdflib.js/issues/528 only talks about moving the types to rdflib, we still need to decide about solidLogicSingleton (see: https://github.com/solid/solid-ui/blob/main/src/logic.ts) which causes the fetcher problem mentioned above. I do not recommend moving solidLogicSingleton to rdflib because that makes rdflib depend on solid-ui (cyclic dependency because solid-ui depends on rdflib).

Then there is the whole matter of singleton: I am not sure singleton makes sense for SolidOS. As long as we have different flavors of SolidOS (web app, data kitchen..) which use differently the fetcher I need to further understand:

bourgeoa commented 2 years ago

linkeddata/rdflib.js#528 only talks about moving the types to rdflib,

Not what I understood. The types where added to solid-logic because rdflib did not produce the d.ts types files. This has been resolved with rdflib@2.2.12

bourgeoa commented 2 years ago

solid-file-client (outside of SolidOS as mentioned above) will need a different kind of solidLogicSingleton or rather a different fetcher (the crossFetch maybe? see: https://github.com/linkeddata/rdflib.js/blob/ce17c86d8f857d183ed31e7e6641aa7952f245ca/src/fetcher.ts#L770) Is this correct?

Not solid-file-client but solid-rest. And will use the global.fetch

bourgeoa commented 2 years ago

My understanding was that livestore that is store.updater() be added to rdflib https://github.com/solid/solid-logic/blob/233102db95e5086a31e3d4d7363238962a54606a/src/index.ts#L52

jeff-zucker commented 2 years ago

@theRealImy I can not say very much about moving the singleton - I am afraid I do not understand that level of the architecture. But let me try to clear up the situation with the alternate fetch methods. If we insist on hard-coding the fetch method for SolidOS, it will never support fetches to anything but a Solid Server - no file://, no cloud storage, no in-browser because neither Inrupt's client nor cross-fetch support any of those. There is also no possibility of using SolidOS for local-first because to do local-first you need an alternate fetch method that writes locally and syncs with the remote. Among other uses for running SolidOS on these other storages is the possibiity of an in-browser pod - basically one could experiment with SolidOS and a Pod without signing up for anything. It's possible we could even run chat from an in-browser pod. If we keep SolidOS agnostic about the storage backend, we can leverage the same SolidOS code on multiple backends.

To support alternate storage backends, we need an alternate fetch method, not an alternate fetcher object. We want the fetcher object to behave as it always does - loading, and updating data but we want the last step - the actual putting the data into storage or getting the data from storage to use a different method to account for different storage backends.

For a fetch to be supported by rdflib and (eventually) SolidOS, it needs to follow the Solid spec for requests and responses - to handle PUT, POST, PATCH, etc. the same way a Solid server does except for the authorization checking. That's what Solid Rest does - it provides a Solid REST API for any backend.

Currently with rdflib, providing alternate fetches works very well. Because we can supply an alternate fetch method, we can use all the rdflib methods in the browser, cloud, or local file system simply by using alternate schemes (file:///,browser:///,etc.) and specifying an alternate fetch method. This means that any software (Solid-File-Client and Solid-Shell are two examples) can use either Inrupt's fetch or Solid-Rest's file fetch via rdflib. This means the apps can use the exact same methods to access data in the browser or file system or Dropbox as it does in a pod so the same code that works in one place, works in the others.

Data-Kitchen does not redefine the fetch, it only provides a cut-out so that authentication is not checked for local fetches. That is done by the authn-checking code, not by the fetch method. So whatever happens with the fetch method, that has nothing to do with and should have no impact on Data-Kitchen.

So this all works as it should in rdflib. But, as you point out, panes insists on hard-coding Inrupt's fetch making panes and therefore the databrowser broken for all alternate storage backends. YES, we need to override the fetch in panes and everywhere else. Is there a downside? The upside seems quite large. The auth-upgrade was such a huge undertaking partially because of previous decisions to tie the SolidOS stack to a specific authenticated fetch method.

The code in rdflib that handles this is one of the first bits in the Fetcher object constructor in fetcher.ts.

this._fetch = options.fetch
               || (typeof global !== 'undefined' && (global.solidFetcher || global.solidFetch))
               || (typeof window !== 'undefined' && (window.solidFetcher || window.solidFetch))
               || crossFetch
    if (!this._fetch) {
      throw new Error('No _fetch function available for Fetcher')
    }

This basically says, if the user passes in a fetch method on Fetcher creation, use it, otherwise look for a global/window variable setting the fetch method and use it, otherwise use cross-fetch. This _fetch method becomes the method for all Fetcher operations including load, putBack, and also updater.update when using the same store as the Fetcher. Setting the method on Fetcher creation is okay for some uses but we need to override the fetch in all fetchers to get the SolidOS stack to respect the alternate backends.

Probably the easiest place to see this working is in the SolidRestBrowser rdflib example. That creates a standard fetcher object but with a fetch that sends browser:// URIs to the in-browser HTML5 Native File system. The authn example creates a hybrid fetch - it uses the Inrupt client for htpp(s):// fetches and SolidRest for browser:// fetches. Ping me if you have questions.

jeff-zucker commented 2 years ago

@bourgeoa - my understanding is that a store, by default, has no fetcher or updater - they must be created manually with fetcher = $rdf.fetcher(store) and updater = new $rdf.UpdateManager(store). That all happens in rdflib. AFAIK, what you end up with those manual steps is a a livestore which is the same thing but the fetcher and updater are created automatically.

bourgeoa commented 2 years ago

@jeff-zucker I fully agree but my understanding is that @timbl created the livestore so that calling UI.store as a liveStore makes it the highest level rdflib object tying together store, fetch, fetcher and updater.

I'm not sure this is compatible with your objective to be able to replace the fetch().

jeff-zucker commented 2 years ago

I don't see why it would beincompatible. This redefines UI.store's fetch method. We just need a cleaner way to do that.

<script src="https://solidcommunity.net/mashlib.js"></script>                
<script>                                                                     
UI.store.fetcher._fetch = (uri,options)=>{alert(uri)};                       
UI.store.fetcher.load('foo:').then({});                                      
</script>                                                                    
timea-solid commented 2 years ago

Thank you @jeff-zucker! After gathering the info, my proposal is to change solid-ui/logic:

  1. move it to solid-logic
  2. use directly new LiveStore(options.fetch) when creating the store for solidOS on line 16 (see @timbl's PR)

The question remaining is: where to delegate the options.fetch choise? Maybe make it a server configuration? Or a SolisOS configuration in a property file or something like that?

jeff-zucker commented 2 years ago

@theRealImy wrote "As long as we have different flavors of SolidOS (web app, data kitchen..) which use differently the fetcher I need to further understand"

This is wrong. Neither the web app nor data kitchen use alternate fetches, they both use the standard fetcher and fetch method with the supplied Singleton and neither of them has any problem with fetches or needs any change to the Singleton or uses any fetch or fetcher different from anything else in SolidOS. Those apps require a different authentication and there is code to make sure we authenticate differently with Data-Kitchen and the webapp but neither data-kitchen nor the web-app make any use of alternate fetches - when they read or write to a Pod, they do it the same way as the standard SolidOS. Data-Kitchen and the webapp only write to Pods - either the localhost or remote, but they do NOT write to non-pods and therefore have nothing at all to do with the question of alternate fetch methods.

The problem comes when writing to non-pods - there the problem is the fetch method and it is a problem for all of SolidOS, not just DK and the webapp. Pods supply all kinds of REST headers - wac-allow, sparql-update, etc. and can request content-negotiation in a request header (e.g. get JSON-LD from a Turtle file). A standard fetch can not get those headers from a file system or Dropbox or in-browser storage because they do not send those headers. Yes, we can write code to do e.g. localStorage.setItem() but that would not allow us to use the storage from within SolidOS. We want SolidOS to write to localStorage using the same logic as writing to a pod. Otherwise, we need to rewrite all of SolidOS. Instead we just re-write the part that does the reading an writing to non-pods.

So the only way to access them is with an alternate fetch which supplies the needed headers. This can be seen in operation with Solid-File-Client and Solid-Shell and Gitter-Solid and other apps which operate in the file-system. They use a standard Inrupt fetch to fetch from pods and an alternate SolidRest fetch to fetch from the file-system. The same thing is possible for in-browser and cloud storage. These kinds of non-pod storages are going to be critical in the development of local-first apps.

Also your use of the phrase "a different fetcher" is wrong - we are always using rdflib's fetcher object, we only change its fetch method. And we only change it when we are fetching from non-pods. We do not change anything else about the fetcher object or the way the fetch method works within it. We want everything to operate as it always does except the actual reading/writing bit.

jeff-zucker commented 2 years ago

@theRealImy wrote - "The question remaining is: where to delegate the options.fetch choise? Maybe make it a server configuration? Or a SolisOS configuration in a property file or something like that?"

This has to be something the user or app can override.

jeff-zucker commented 2 years ago

Summary :

Redefining the fetch method in rdflib and SolidOS

An app may wish to redefine the method rdflib uses to read and write data. One reason is to handle protocols other than https:, e.g. file: or browser:. Another reason would be local-first replicating fetches. Once rdflib's fetch is redefined, the new fetch will be used by all fetcher methods (load,webOperation,update,etc).

An app using rdflib

Can override the fetch globally

     window.solidFetch = myFetchMethod  // in browser 
     global.solidFetch = myFetchMethod  // in nodejs

Or modify a specific fetcher

    myFetcher = $rdf.fetcher(kb,{fetch:myFetchMethod})

These work fine and require no changes.

An app using mashlib or panes

Can override the fetch globally

   panes.UI.store.fetcher._fetch = myFetchMethod

This works fine but is rather ugly. We could add some syntactic sugar to make it look better. Since only people with fairly advanced knowledge of SolidOS are ever likely to use this, I don't think even the syntactic sugar is worthwhile. We should, however, document it. @theRealImy, perhaps you can point me to a place in the documentation this should go.

timea-solid commented 2 years ago

@jeff-zucker I think we can continue on adding stuff in the documentation here: https://github.com/solid/solidos/tree/main/documentation What I would also like to see mentioned is: this ticket and a link to a code example.