feat: FileCollection uses proxy for shared drives #1604

taratatach · 2025-04-30T14:09:55Z

To make changes to files on a shared drive, clients have to call their
stack via proxied routes (e.g. /sharings/drives/:id/_changes for the
changes feed of the given shared drive).

Instead of creating a separate collection for that we enable passing
options to a collection and specifically a driveId option to the
FileCollection.
This presence of a driveId option will change the prefix of all
routes called by the FileCollection from /files to
/sharings/drives/${driveId}.

packages/cozy-stack-client/src/FileCollection.js

zatteo · 2025-04-30T15:19:30Z

I like it like this. It implies that shared drives have exactly same route except the prefix but if it is what will happen 👍

paultranvan · 2025-05-05T08:27:39Z

I think it's a good idea to avoid creating a new collection 👍
I have 2 concerns here:

How can we use this API with a client.query, rather than a direct client.collection('io.cozy.files', { driveId }) ?
The naming of driveId might be not ideal, I think I would prefer a more explicit sharedDriveId. Or maybe an even more explicit prefix? That would have the benefit of being more generic (but require the dev to know about the route path)

About the first point, maybe we could pass the prefix to the collection methods options, and do client.query(queryDef, { pathPrefix: /sharings/drives/${driveId} }) ?

taratatach · 2025-05-05T09:00:16Z

I forgot about client.query but we could pass the collection's options as argument indeed
I thought about using prefix but cozy-client abstracts away paths and I'd rather not force developers to know about them for this feature. I'm OK with using a more specific argument name like sharedDriveId though

zatteo · 2025-05-26T13:05:37Z

require the dev to know about the route path ?

I think it's better to avoid this (that's one of the main advantages of cozy-client to avoid the need to know route pathes). I would like that the user of cozy client (using a collection or a query) only has to know and pass a driveId or sharedDriveId.

So we would do something like :

for collection : const fileCollection = client.collection('io.cozy.files', { sharedDriveId: sharedDriveId })
for queries : Q(io.cozy.files).where({ trashed: true}).fromSharedDriveId(sharedDriveId)

And then maybe inside cozy-client inside the file collection, we convert the sharedDriveId to a prefix to keep the code clean and generic inside cozy-client.

About the 2. (using a query), I do not know what is best between passing it:

in query definition client.query(Q(io.cozy.files).where({ trashed: true}).fromSharedDriveId(sharedDriveId)) => we need to add it in the DSL we want to keep generic
- in query definition client.query(Q(io.cozy.files).where({ trashed: true}).sharing(sharedDriveId))
in query definition inside the where clause client.query(Q(io.cozy.files).where({ trashed: true, sharedDriveId: sharedDriveId })) => ❌ too "magic", will break Sift and Pouch
in query options client.query(Q(io.cozy.files).where({ trashed: true }), { sharedDriveId: sharedDriveId })

It makes sense for me that it is inside the query definition because it changes the data fetched and not the way we fetch data but I do not know what it implies/if it is possible.

edit:

In every case for using a query, to not break Pouch, Pouch (cozy-pouch-link) should have a notion of shared drive :

if it is already stored, to be able to get it
it it is not present, to not fail and forward the query to the stack

We need to persist data (the shared drive id) that does not exist in io.cozy.files document on stack side

zatteo · 2025-07-18T09:16:52Z

I think we are close to finish this PR 🎉

What I did in last commits :

fixed some small bugs (d63f6d6 & 2880615)
fixed CI
rebased master so that we can merge

It seems to work because when doing a Q('io.cozy.files').where({ dir_id: folderId }).setSharingId(driveId) it calls correctly POST http://bob.cozy.localhost:8080/sharings/drives/2cba5fcdea12cf00e068415cfca16a46/_find. I want to wait the stack to implement this route to be sure everything is fine.

Two questions :

In d63f6d6 I had to change DSL method from sharingId to setSharingId. What do you think about it? It will be the main exposed API.
Internally, we sometime use driveId and sometimes sharingId. Is it relevant to have this two names? I am not sure but I can't figure it out.

taratatach · 2025-07-22T07:57:48Z

* In [d63f6d6](https://github.com/cozy/cozy-client/commit/d63f6d620740e8d50efd2439080cc864736baf31) I had to change DSL method from `sharingId` to `setSharingId`. What do you think about it? It will be the main exposed API.

What about inSharing(<id>)? I think it better conveys the idea that the query will be run within the given sharing.

* Internally, we sometime use `driveId` and sometimes `sharingId`. Is it relevant to have this two names? I am not sure but I can't figure it out.

I used sharingId pretty much everywhere except in FileCollection where I preferred to used driveId as it is more precise (it's not any kind of file sharing).

paultranvan · 2025-07-22T07:57:51Z

packages/cozy-client/src/links/StackLink.js

+      sharingId,
+      ...rawOptions
+    } = query
+    let options = { ...rawOptions, driveId: undefined }


Why do you need to set the driveId here? I would prefer to make it appear only when necessary

From memory it was to satisfy the type checker.

paultranvan · 2025-07-22T08:01:46Z

packages/cozy-stack-client/src/FileCollection.js

+ * Options that can be passed to FileCollection's constructor
+ *
+ * @typedef {object} FileCollectionOptions
+ * @property {string} [driveId] - ID of the shared drive targeted by the collection


Do we have some doc about the shared drive somewhere? Typically from the stack side?

We do have documentation in cozy-stack about shared drives.

It is inside the PR for the moment cozy/cozy-stack#4511

I was wondering why I didn't find any code :)
Ok, so the stack's doc link could be added to the jsdoc eventually

packages/cozy-pouch-link/src/helpers.js

paultranvan · 2025-07-22T08:09:38Z

packages/cozy-client/src/links/StackLink.spec.js

+      expect(stackClient.collection).toHaveBeenCalledWith(
+        'io.cozy.todos',
+        expect.anything()
+      )


Since this commit actually fix tests from previous commit, it should be rebased within the commit where it breaks

I would even say squashed.

Yes, I meant squashed 👍

paultranvan · 2025-07-22T08:14:15Z

packages/cozy-client/src/queries/dsl.js

   * @returns {QueryDefinition}  The QueryDefinition object.
   */
-  sharingId(id) {
+  setSharingId(id) {


I don't really like this naming TBH. I understand that we technically set the sharingId to the QueryDefinition, but here the semantic should not be on the technical set, but rather from the querying perspective. As a developer, I would not understand doing Q('io.cozy.files').setById(id) to query a file by its id.
Likewise, I would prefer sharingById here

I suggested inSharing(id).

I'm not a big fan of the in, that sounds a bit too functional to me.
Should we settle the final decision with a trial by combat? Or maybe let @zatteo have the final word?

Oh @zatteo already called it

Sorry I answered before seeing @taratatach comment...

I think inSharing is more explicit but like you said it is more "functional", and cozy-client API and naming is not functional so maybe we should stick with sharingById.

To make changes to files on a shared drive, clients have to call their stack via proxied routes (e.g. `/sharings/drives/:id/_changes` for the changes feed of the given shared drive). Instead of creating a separate collection for that we enable passing options to a collection and specifically a `driveId` option to the `FileCollection`. This presence of a `driveId` option will change the prefix of all routes called by the `FileCollection` from `/files` to `/sharings/drives/${driveId}`. Since `uri` encodes the content of template literals placeholders, passing the prefix via a placeholder would result in an invalid path. Therefore we prepend the prefix out of routes literals.

paultranvan

9c31bdd

can be squashed, otherwise, ok for me, thanks for the work!

@paultranvan

With the contribution of @paultranvan for the PouchLink part. We add a new method to the QueryDefinition dsl so we can scope queries to documents targeted by a given sharing. This will be used for `io.cozy.files` documents in shared drives at first. e.g.: `client.query(Q('io.cozy.files').sharingId('xyz'))` Since `cozy-stack` returns the shared drive id of `io.cozy.files` documents in their `driveId` attribute, we can use it to query local documents via PouchLink. They're not saved in CouchDB though so StackLink passes the id as argument to the file collection which will target the shared drives' API.

dsl method was called sharingId and QueryDefinition class attribute was also called sharingId.

So we can chain setSharingId in whatever order we want we writing a query.

zatteo reviewed Apr 30, 2025

View reviewed changes

packages/cozy-stack-client/src/FileCollection.js Outdated Show resolved Hide resolved

zatteo reviewed Apr 30, 2025

View reviewed changes

packages/cozy-stack-client/src/FileCollection.js Show resolved Hide resolved

taratatach force-pushed the feat/enable-proxied-file-collection-for-shared-drives branch 3 times, most recently from 150847b to 218538b Compare June 19, 2025 14:55

zatteo force-pushed the feat/enable-proxied-file-collection-for-shared-drives branch 2 times, most recently from 1cfb8b1 to 92d7a89 Compare July 18, 2025 09:11

zatteo force-pushed the feat/enable-proxied-file-collection-for-shared-drives branch from 92d7a89 to 758d712 Compare July 22, 2025 06:42

paultranvan reviewed Jul 22, 2025

View reviewed changes

zatteo marked this pull request as ready for review July 22, 2025 08:19

zatteo force-pushed the feat/enable-proxied-file-collection-for-shared-drives branch from 758d712 to 79990f4 Compare July 22, 2025 08:57

paultranvan approved these changes Jul 22, 2025

View reviewed changes

taratatach and others added 5 commits July 22, 2025 16:54

refactor: Remove dead code

fdf00a1

feat: Rename dsl method to sharingById

3f22bf9

dsl method was called sharingId and QueryDefinition class attribute was also called sharingId.

fix: Set sharingId in definition

4cfa340

So we can chain setSharingId in whatever order we want we writing a query.

docs: Update docs

9ed3e6a

zatteo force-pushed the feat/enable-proxied-file-collection-for-shared-drives branch from 79990f4 to 9ed3e6a Compare July 22, 2025 14:58

zatteo merged commit 45e7607 into master Jul 23, 2025
3 checks passed

zatteo deleted the feat/enable-proxied-file-collection-for-shared-drives branch July 23, 2025 07:18

Crash-- mentioned this pull request Aug 18, 2025

feat: Update FileImageLoader to be used in shared drive ✨ cozy/cozy-ui#2828

Merged

feat: FileCollection uses proxy for shared drives #1604

feat: FileCollection uses proxy for shared drives #1604

Uh oh!

Conversation

taratatach commented Apr 30, 2025

Uh oh!

Uh oh!

Uh oh!

zatteo commented Apr 30, 2025

Uh oh!

paultranvan commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taratatach commented May 5, 2025

Uh oh!

zatteo commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zatteo commented Jul 18, 2025

Uh oh!

taratatach commented Jul 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paultranvan Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paultranvan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paultranvan commented May 5, 2025 •

edited

Loading

zatteo commented May 26, 2025 •

edited

Loading

paultranvan Jul 22, 2025 •

edited

Loading