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

Added Solid support #1319

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Conversation

yasharpm
Copy link

Hello, I have implemented the Solid backend. These changes are compatible with the widget. I am creating another pull request for the widget as well.

The test code and documentation updates are also there...

Thank you :-)

@yasharpm
Copy link
Author

@raucao
Copy link
Member

raucao commented Aug 29, 2024

Great to see a PR for another open protocol!

Feel free to also notify people on the community forums about these PRs: https://community.remotestorage.io/t/adding-solid-as-a-backend/828

Maybe you could also add some information for people new to SOLID (like myself e.g.) about how to test this end-to-end (i.e. which server implementation and.or popular provider/hoster to use).

Copy link
Contributor

@DougReeder DougReeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wonderful; thank you!

A big change like this will take us a while to review and may take a couple rounds of changes (hopefully small); please be patient.

1. Have you had a chance to run the API test suite against this? rs-backup tweaked for your server is the easiest way to generate a token, that I've found.

  1. Do you know if the Solid library is using fetch or XHR? We've been supporting environments with either one; if Solid only uses fetch we'll need to document that.

  2. I see you've written tests using Jaribu. Unfortunately, that has been deprecated and doesn't run under recent versions of Node.js. New tests are being written using Mocha. I know it's a pain, but could you please add Mocha tests for Solid?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add some comment lines, explaining how this file fits into the architecture?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this interface is only used for Solid code, then you can move it to the Solid module and export it from there.

@@ -368,7 +370,7 @@ export class RemoteStorage {
if (typeof cfg === 'object') { extend(config, cfg); }

this.addEvents([
'ready', 'authing', 'connecting', 'connected', 'disconnected',
'ready', 'authing', 'connecting', 'pod-not-selected', 'connected', 'disconnected',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can imagine some reasons for 'pod-not-selected' to be separate from 'error', but please detail them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... because no existing app will respond to the 'pod-not-selected' event, but they will respond to other events.

Copy link
Member

@raucao raucao Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be the state of the Solid connection process for the widget or someone implementing custom UI. So it should be part of a documentation page for adding Solid to an app (maybe another section on the GDrive/Dropbox page, and renaming the page and menu item).

@raucao
Copy link
Member

raucao commented Aug 29, 2024

  1. Have you had a chance to run the API test suite against this?

The API test suite is irrelevant here, as it tests the compliance of RS servers with the RS specification. However, this PR is for clients connecting to Solid servers, as an alternative to RS servers.

@raucao
Copy link
Member

raucao commented Aug 29, 2024

Another note for tests: there is a new Mocha suite for the RemoteStorage class in this open PR: #1318

@DougReeder
Copy link
Contributor

  1. Have you had a chance to run the API test suite against this?

The API test suite is irrelevant here, as it tests the compliance of RS servers with the RS specification. However, this PR is for clients connecting to Solid servers, as an alternative to RS servers.

Right you are! I've been wrestling with Armadietto for half a year. :-S

@DougReeder
Copy link
Contributor

When testing with a real app, does calling setAuthURL() eliminate the need for the updated Widget? And what object has that method? dropbox-and-googledrive.md is somewhat vague.

More generally, when I pull this version of remotestoragejs into a real app, what do I need to do to connect to a pod? (I have a pod on Inrupt)

@yasharpm
Copy link
Author

  1. Do you know if the Solid library is using fetch or XHR? We've been supporting environments with either one; if Solid only uses fetch we'll need to document that.

I am almost certain that it only uses fetch. I can do a through inspection and report back if necessary.

  1. I see you've written tests using Jaribu. Unfortunately, that has been deprecated and doesn't run under recent versions of Node.js. New tests are being written using Mocha. I know it's a pain, but could you please add Mocha tests for Solid?

I've tried to follow the pattern of the rest of the code. But I've missed the recent updates. I'll try to match and let you know.

@yasharpm
Copy link
Author

Let me elaborate on the pod-not-selected state. Each Solid account can have multiple storages/pods connected to it or even none, by the protocol. But in practice there is always only one storage connected to each account.

After a user logs in into their Solid account, their gonna have to select which pod they want to use. So pod-not-selected is not essentially an error state but just another step in the connection process. The the logic for the widget automatically passes this event if there is only one pod. If there is none, throws an error. And if there is multiple, asks the user to choose one from the list.

My line of logic is that, if the developer is using the widget, they don't need to worry about anything. The Solid option does not even show up if they don't put it in the widget's configuration. They need to worry about it only if they are bypassing the widget and intent to add support for Solid. It doesn't affect the existing apps.

@yasharpm
Copy link
Author

yasharpm commented Aug 30, 2024

As for the setAuthURL() function, we have a similar situation. Solid does not have a specific backend as it is only a protocol. So it needs to be selected. But on the other hand we don't need an API key or other configurations like Google drive and Dropbox do.

So before asking the Solid backend to connect, it needs to be told what to connect to. The widget shows some options and also an input. If bypassing the widget, then we assume that the developer already has one in mind or asked the user via some other process.

We call it "authURL" on rs. Solid calls it "Identity provider".

@yasharpm
Copy link
Author

Btw thank you guys for attending to this PR so quickly. This is very refreshing and welcoming 😊

@yasharpm
Copy link
Author

yasharpm commented Aug 30, 2024

Maybe I need to add a full documentation on how to directly incorporate the Solid backend without using the widget?

When testing with a real app, does calling setAuthURL() eliminate the need for the updated Widget? And what object has that method?

It does. The object that has the method is Solid. Accessible via remoteStorage.solid.setAuthURL(). But we would still have the pod-not-selected step which needs to be called if the developer is not using the widget.

dropbox-and-googledrive.md is somewhat vague.

I'll fix it. I believe I renamed the file to dropbox-googledrive-solid.md.

More generally, when I pull this version of remotestoragejs into a real app, what do I need to do to connect to a pod? (I have a pod on Inrupt)

const connectTask = setTimeout(() => {
  remoteStorage.solid.setAuthURL('your-solid-identity-provider'); // i.e. https://login.inrupt.com
  remoteStorage.solid.connect();
  // Calling 'connect()' will immediately redirect to your identity provider website.
}, 1000);
remoteStorage.on('pod-not-selected', () => {
  clearTimeout(connectTask);
  const podURLs = remoteStorage.solid.getPodURLs();
  // Choose one. Maybe there is even 0?
  remoteStorage.solid.setPodURL(podURLs[0]);
  // That's it. 'connected' is fired immediately.
};
remoteStorage.on('connected', () => {
  // We are connected.
  clearTimeout(connectTask);
  // We arrive here either through calling 'setPodURL' on the `pod-not-selected` event or
  // on page getting loaded. Because: Inrupt Solid library prefers to always redirect to the
  // identity provider to get the session tokens. I spent a lot of time on this. If we want to
  // change it, we'll have to make fundamental changes to Inrupt's library and make a PR.
});

Calling connect() redirects the page. And we get back to our homepage with a payload containing the session data. So whenever our home page is loaded, we are either not connected at all or in the process i.e. we get a session but have not selected the pod yet, or a pod was already selected and we immediately jump to the connected state.

@DougReeder
Copy link
Contributor

My line of logic is that, if the developer is using the widget, they don't need to worry about anything. The Solid option does not even show up if they don't put it in the widget's configuration. They need to worry about it only if they are bypassing the widget and intent to add support for Solid. It doesn't affect the existing apps.

Great! Please add a comment in the code and update the event documentation, to that effect.

@DougReeder
Copy link
Contributor

If you have an app that uses this, it would help if we could look at it.

@DougReeder
Copy link
Contributor

I am almost certain that it only uses fetch. I can do a through inspection and report back if necessary.

I believe the vast majority of browsers in use support fetch, but the API didn't fully stabilize in Node.js until v21. Security support for v20 ends on 30 Apr 2026 (in 1 year and 8 months) If I understand correctly, the implementation in v18 and v20 doesn't support streams fully.

So, we'll need to fully test this under Node 18 and 20, to see if this works properly with them. If it doesn't, I'm okay with documenting that this version of remotestorage.js when used in Node.js, should only be used with v21 and later.

@raucao
Copy link
Member

raucao commented Aug 30, 2024

According to our docs, fetch is available from node.js 18 onwards, and we already ask to add a polyfill for when it's not. So the usage of fetch in a new back-end shouldn't change anything for node.js users or in our docs.

https://remotestorage.io/rs.js/docs/nodejs.html#caveats

@yasharpm
Copy link
Author

yasharpm commented Sep 2, 2024

I've updated the documentation and added mocha tests. This should be it as far as I followed the conversation. What do you guys think?

@DougReeder
Copy link
Contributor

I haven't been able to use this in a real app yet — the app I've been testing with is throwing up issues that don't appear to be caused by this. I'll have to try with a different app.

@NoelDeMartin
Copy link

Hi there, I don't know much about remoteStorage; but I've been working on Solid Apps and something crucial to Solid is using RDF for interoperability. Seeing this PR, I understand that this adds support for a Solid backend as a "file storage", right?

I wonder how would anyone use this library to make a Solid application using RDF. Would the developers need to write RDF manually? What about updates to documents?

Anyways, I'm just mentioning this in case you weren't aware. If that's already the intention, using Solid as a file storage rather than a store of RDF documents, that's fine. But I think using a Solid POD like that is missing a lot of the potential. I guess it's better than nothing though, if it adds "Solid support" to some of the existing apps implemented using remoteStorage. But I don't think I'd recommend anyone making a Solid App from scratch to follow this approach.

By the way, in case it's useful for anyone, some years ago we had a discussion comparing remoteStorage, Fission, and Solid. And this was one of the points that came up: https://vimeo.com/779640162

@DougReeder
Copy link
Contributor

Anyways, I'm just mentioning this in case you weren't aware. If that's already the intention, using Solid as a file storage rather than a store of RDF documents, that's fine. But I think using a Solid POD like that is missing a lot of the potential. I guess it's better than nothing though, if it adds "Solid support" to some of the existing apps implemented using remoteStorage. But I don't think I'd recommend anyone making a Solid App from scratch to follow this approach.

Yes, you won't get many of the benefits of a native Solid app. But this is an important step in moving people to storage independent of apps. With this, someone who has a Solid pod can also use the same storage for Apps written for remoteStorage.

@raucao
Copy link
Member

raucao commented Sep 12, 2024

Wouldn't that be something that could be supported in the data modules for this library?

@yasharpm
Copy link
Author

Wouldn't that be something that could be supported in the data modules for this library?

This was the idea when we were considering the task. The data module receives the Client instance which probably exposes the RemoteStorage hence the Solid backend which then you can get the raw Solid Session from it.

So a data module can implement a specific behavior for dealing with Solid the way it is recommended. The downside is that in order to be Solid friendly is that the different data modules need to have a check for the back-end type and do different thins.

@DougReeder
Copy link
Contributor

I've tried adding this and the updated widget to a simpler app, but no option for Solid storage appears in the list. Is there an example of this running in a real app?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants