-
Notifications
You must be signed in to change notification settings - Fork 189
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
Discussion for better URLS: NOT MERGE #3261
Conversation
Signed-off-by: Christian Richter <crichter@owncloud.com@>
Co-Authored-By: Christian Richter <crichter@owncloud.com@>
* update reva to include decomposedfs nodes-per-space Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * check create space with own constraint Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * update reva Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * unexpected passed Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * update reva Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * remove unused variable Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * revert reva commit id Co-authored-by: Florian Schade <f.schade@icloud.com>
Co-Authored-By: Christian Richter <crichter@owncloud.com> Co-Authored-By: Willy Kloucek <wkloucek@owncloud.com>
This copies the validation code from the accounts service, also fixing a bug in the regex that allowed adding mail addresses with whitespace and other problematic characters to the domain part of the mail address. Partial fix for: owncloud#3247
Similar to what the accounts service is doing, all new users get the User role assigned now. Otherwise creating the user's personal space upon login is not working.
* bump web rc.1 * Use web v5.2.0-rc.2 Co-authored-by: Pascal Wengerter <pascal@wengerter.info>
Bumps [github.com/grpc-ecosystem/grpc-gateway/v2](https://github.com/grpc-ecosystem/grpc-gateway) from 2.7.3 to 2.8.0. - [Release notes](https://github.com/grpc-ecosystem/grpc-gateway/releases) - [Changelog](https://github.com/grpc-ecosystem/grpc-gateway/blob/master/.goreleaser.yml) - [Commits](grpc-ecosystem/grpc-gateway@v2.7.3...v2.8.0) --- updated-dependencies: - dependency-name: github.com/grpc-ecosystem/grpc-gateway/v2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
* bring back web master for drone env
| https://cernbox.cern.ch/external/files-public-files/eoshome-d!2639/Microsoft%20Office?contextRouteParams.item=sstLvrDFEpxwwxL&public-token=sstLvrDFEpxwwxL | https://cernbox.cern.ch/external/public/sstLvrDFEpxwwxL/New%20file.docx?app=microsoft-office&previous=/files/public/sstLvrDFEpxwwxL | | ||
|
||
|
||
* We strip off `personal/home` as it does not bring any value to the URL and makes the URL relative to the current user, making it impossible to just copy and paste the URL to be reused by another user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. The current implementation is still WIP. personal
is used to determine which view to render, eg personal or projects. The plan is to rely on the GraphAPI /drives
endpoint to distinguish spaces by driveType
(personal
, project
).
home
is legacy (resolves to the old webdav URL) and will be dropped when switching to aliases.
https://cloud.owncloud.com/files/spaces/personal/einstein/relative/path/to/file.txt
.
Here, personal/einstein
is the alias for Albert Einsteins personal space.
Example
A technical URL would look like
https://cernbox.cern.ch/files/spaces/95b4c6f6-d8f2-4976-b0a3-1b016bdfcb7e/d/dalvesde/relative/path/to/file.txt
.
To make the URL human-readable we replace the UUID with an alias.
https://cernbox.cern.ch/files/spaces/eos/users/d/dalvesde/relative/path/to/file.txt
The WebUI discovers the spaces using the /drives
Endpoint. For CERNBox it can return a space with the alias eos/users
and the id:95b4c6f6-d8f2-4976-b0a3-1b016bdfcb7e
. This aliasing allows the WebUI to use human-readable URLs and translate the alias back into a UUID which is then used to make the actual WebDAV Requests.
The problem with aliases is, that they are not unique, can be customized and might clash. Therefore the UUID is always appended as an URL Parameter ?id=95b4c6f6-d8f2-4976-b0a3-1b016bdfcb7e
Future CERNBox URL
https://cernbox.cern.ch/files/spaces/eos/users/d/dalvesde/relative/path/to/file.txt?id=95b4c6f6-d8f2-4976-b0a3-1b016bdfcb7e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important regarding timeline: the ticket owncloud/web#6448 which is part of the current sprint (to be done by 18th of March) is a requirement for making the points above possible, because one part of the ticket is to switch over from hardcoded dav endpoints to fully relying on the GraphAPI /drives
endpoint for discovering dav endpoints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API Endpoint https://cernbox.cern.ch/graph/v1/me/drives
can be implemented for CERNBox using a static list of existing storages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The beauty of it is the forward compatibility to a EOS driver which implements spaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact you will be stuck with the cernbox web fork after owncloud/web#6448 is done and you don't provide the static list of existing storages that @micbar mentioned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments
|
||
|
||
* We strip off `personal/home` as it does not bring any value to the URL and makes the URL relative to the current user, making it impossible to just copy and paste the URL to be reused by another user. | ||
* The files app works with slashes (files/spaces/personal/home/) and the application works with dashes (files-spaces-personal-home). We find no sense to this distinction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having the origin route name in the path of app routes is only an intermediate solution. We plan to move the origin route name to a query parameter. For apps in the web monorepo this is already very easy and straightforward to use.
The origin route name is needed to go back to where the user came from when closing an app (e.g. when closing the mediaviewer, going back to a public link or the all files page respectively).
* We strip off `personal/home` as it does not bring any value to the URL and makes the URL relative to the current user, making it impossible to just copy and paste the URL to be reused by another user. | ||
* The files app works with slashes (files/spaces/personal/home/) and the application works with dashes (files-spaces-personal-home). We find no sense to this distinction. | ||
Applications should not behave differently that the files app (another application). | ||
* We find the usage of query parameters `contextRouteParams.*` confusing and we prefer to opt for a more natural value (the previous url) with the `previous` query parameter. Note that this query parameter should only be used for applications rendered inside the Web (not for external applications opening in a tab). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to give it a different name. I'd opt for origin
, as previous
implies a sequence.
In general, urls should be of the type: | ||
|
||
* `<app-name>/</namespaced/alias></relative/path/to/resource>` or | ||
* `<app-name>/public/<token></relative/path/to/resource>` for public links (allowing to distinguish if they need to be authenticated or not) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already discussed dedicated public
application routes with you in the past and opted for the solution of providing the origin route name via URL. It was a conscious design decision for oCIS to not make a difference between public and private links anymore. Actively distinguishing that in the path feels wrong to us.
Also, regarding developer experience, it seems to be an unnecessary effort for developers to define routes twice (once for authenticated, once for public context). When we have the origin route name in the query parameters it will be as simple as using a small set of composables in your self developed app to get the authentication, file retrieval and back-navigation basically for free. At the moment this is only possible for apps in the web monorepo.
As far as I can tell, this is completely outdated as we implemented most of the ideas here already or chose different approaches in the CERN Web Merge project. If there's anything still relevant, please open a new PR :) |
No description provided.