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.

Sign up for GitHub

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

Specify "origin private file system"-ness on a FileSystemHandle #110

Merged
merged 2 commits into from
Jun 14, 2023
Merged

Specify "origin private file system"-ness on a FileSystemHandle #110

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3320303
Specify "origin private file system"-ness
a-sully Apr 6, 2023
355151d
flip bit (whoops) + clarify assignment
a-sully Jun 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ Each [=/file system locator=] has an associated <dfn export for="file system loc
a <dfn export for="file system locator" id=locator-kind>kind</dfn> (a {{FileSystemHandleKind}}), and
a <dfn export for="file system locator" id=locator-root>root</dfn> (a [=file system root=]).

Issue(103): Consider giving each locator a Storage Bucket.
Issue(109): Consider giving each locator a [=storage bucket=].

A <dfn export>file locator</dfn> is a [=/file system locator=] whose
[=file system locator/kind=] is {{FileSystemHandleKind/"file"}}.
Expand Down Expand Up @@ -341,6 +341,20 @@ A {{FileSystemHandle}} object is associated with a <dfn for=FileSystemHandle exp
Note: Multiple {{FileSystemHandle}} objects can have
[=the same locator as|the same=] [=/file system locator=].

A {{FileSystemHandle}}
<dfn for=FileSystemHandle export>is in an origin private file system</dfn>
if the first [=list/item=] of its [=FileSystemHandle/locator=]'s
[=file system locator/path=] is the empty string.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Another option here is to define "the root directory of the origin private file system" and check whether the handle "resolves" to that, but that doesn't set us up well for a multi-OPFS future (#92)

Eventually it would be nice to use Storage Buckets here, but I think this is the best we can do for now

Copy link
Collaborator Author

@a-sully a-sully Apr 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Just kidding, I just realized that I've been confusing the incubating Storage Buckets API with the already-specified storage bucket concept.

We could give a "file system root" the option to be/have a "storage bucket" (or bottle?), but there's currently no way to get a bucket from the current environment AFAICT? Sorry, I'm not as familiar with how this model is intended to work and I don't have all the context for the issues listed here https://wicg.github.io/storage-buckets/#storage-bucket-getdirectory @evanstade @ayuishii

Copy link

Choose a reason for hiding this comment

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

OPFS is a storage endpoint and there's one storage bottle inside a storage bucket for each storage endpoint. (The endpoint is like a class, whereas the bottle is an instance of that class.) A storage shelf equates to a storage partition defined by a StorageKey. So a shelf contains buckets which contains bottles.

The issue mentioned in the storage buckets spec is that we need a version of this algorithm which works for any bucket. You'll notice if you drill down into "obtain a local storage bottle map" that it currently assumes the "default" bucket. This just means passing an additional parameter.

We do not currently have a way to go from bottle to bucket for any storage endpoint.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the clarification @estade. I filed #112 to track making getDirectory() work for non-default buckets. Addressing whatwg/storage#2 seems like a prerequisite for adding another parameter to "obtain a local storage bottle map"?

We do not currently have a way to go from bottle to bucket for any storage endpoint.

We just need some way to tie a FileSystemHandle to its root (i.e. two files of the same "file system path" must be considered different if they're from different buckets). There's one OPFS instance per storage bucket, though since there's also only one "fileSystem" bottle per bucket, then using the bottle would also work... though currently, it seems like it's only possible to get a "storage bottle map" and not the storage bottle itself. It seems a bit clumsy to associate a "storage bottle map" with each "file system locator"

Which raises the question of... why don't we just point to the directory entry at map["root"] directly? That would essentially make every "file system locator" a relative path, which on one hand could improve the hand-waviness of the recently-added "locate an entry" and "get the locator" methods, but on the other hand re-raises some of the questions of #59 (i.e. what happens if a root is deleted or moved? #9 (comment))

So, for now at least, I'd prefer to keep "file system locators" absolute and find a way to point to a bottle/bucket :)

Copy link

Choose a reason for hiding this comment

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

I don't think there's any reason not to have a way to retrieve a bottle. We do have ways to get a bucket, but they're currently specified here instead of in the storage spec.

Copy link
Member

Choose a reason for hiding this comment

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

I think if we define OPFS in terms of https://storage.spec.whatwg.org/#obtain-a-local-storage-bottle-map you could make this rely less on magic. But that's a more substantial overhaul.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ack. I'll leave this PR as just specifying OPFS-ness on a FileSystemHandle, then we can leave better defining the OPFS itself (in terms of the Storage spec) to a follow-up


Note: This is a bit magical, but it works since only the root directory of an
[=origin private file system=] can have a [=file system locator/path=] which
[=list/contains=] an empty string. See {{StorageManager/getDirectory()}}.
All other [=list/item=]s of a [=file system locator/path=] will be a
[=valid file name=].

Issue(109): Consider improving this situation by giving each locator a
[=storage bucket=].

<div algorithm="serialization steps">
{{FileSystemHandle}} objects are [=serializable objects=].

Expand Down Expand Up @@ -594,8 +608,8 @@ The <dfn method for=FileSystemFileHandle>createWritable(|options|)</dfn> method
The returned {{FileSystemSyncAccessHandle}} offers synchronous methods. This allows for higher performance
on contexts where asynchronous operations come with high overhead, e.g., WebAssembly.

For the time being, this method will only succeed when the |fileHandle| belongs to the
[=origin private file system=].
For the time being, this method will only succeed when the |fileHandle|
[=FileSystemHandle/is in an origin private file system=].
</div>

<div algorithm>
Expand All @@ -605,6 +619,9 @@ The <dfn method for=FileSystemFileHandle>createSyncAccessHandle()</dfn> method s
1. Let |locator| be [=this=]'s [=FileSystemHandle/locator=].
1. Let |realm| be [=this=]'s [=relevant Realm=].
1. Let |global| be [=this=]'s [=relevant global object=].
1. Let |isInAnOriginPrivateFileSystem| be true if
[=this=] [=FileSystemHandle/is in an origin private file system=];
otherwise false.
1. [=Enqueue the following steps=] to the [=file system queue=]:
1. Let |entry| be the result of [=locating an entry=] given |locator|.
1. Let |accessResult| be the result of running |entry|'s
Expand All @@ -615,15 +632,15 @@ The <dfn method for=FileSystemFileHandle>createSyncAccessHandle()</dfn> method s
|accessResult|'s [=file system access result/error name=] and
abort these steps.

1. If |isInAnOriginPrivateFileSystem| is false,
[=queue a storage task=] with |global| to
[=/reject=] |result| with an "{{InvalidStateError}}" {{DOMException}} and
abort these steps.

1. If |entry| is `null`, [=queue a storage task=] with |global| to [=/reject=]
|result| with a "{{NotFoundError}}" {{DOMException}} and abort these steps.
1. [=Assert=]: |entry| is a [=file entry=].

1. If |entry| does not represent a [=/file system entry=] in an
[=origin private file system=], [=queue a storage task=] with |global| to
[=/reject=] |result| with an "{{InvalidStateError}}" {{DOMException}} and
abort these steps.

1. Let |lockResult| be the result of [=file entry/lock/take|taking a lock=]
with "`exclusive`" on |entry|.

Expand Down Expand Up @@ -1653,7 +1670,7 @@ partial interface StorageManager {

<div class="note domintro">
: |directoryHandle| = await navigator . storage . {{StorageManager/getDirectory()}}
:: Returns the root directory of the origin private file system.
:: Returns the root directory of the [=origin private file system=].
</div>

<div algorithm>
Expand Down