refactor: graph listing - #3321
Draft
dschmidt wants to merge 23 commits into
Draft
Conversation
Picks up the driveItem stat and children operations with $select and $expand, plus lockInfo, pendingOperations, tags, following, allowedValues and shareTypes on the driveItem type. Generated from opencloud-eu/libre-graph-api#70 and #72, neither merged yet.
Stat and children now go through the generated v1.0 operations, so $select and $expand are typed and the driveItem facets no longer need casts. The path form still cannot use the generated operation: it percent-encodes the item id, which turns the ':/' the server matches on into '%3A%2F'. It now builds the request with the generated param creator and rewrites the item segment, encoding each path segment as the server expects. An unencoded ':' in a folder name was read as the colon-syntax delimiter before. Dropping the casts also surfaced that graph reports share types by key while every consumer compares against the numeric values.
The vault translation lived inside the webdav decorator, so the graph listing walked past it: a vault folder showed the encrypted names the server stores, and isInVault stayed false. The path translation and the decrypt/flag pass move to helpers/vaultTranslate, the webdav decorator and the graph listing both go through them. The graph listing moves next to the loader so it can be tested without importing the loader, which pulls the folderService singleton along.
The listing asked for $expand=children and guessed a preview for every file, since graph had no counterpart to PROPFIND's has-preview property. It now asks for thumbnails as well and reports a preview exactly for the items the server expanded them for, folder and children alike. Needs opencloud-eu/opencloud#3471 on the server side.
A share space is rooted at the shared item, its webDavPath points straight at it. Graph answers in drive coordinates though: the stat of a received share reports the share root as "/folderToShare" rather than "/", so every child ended up one segment too deep and requests against it hit a 404. PROPFIND never showed this, its paths come back relative to the requested webdav root. Inside a share the requested path is the authoritative one.
Navigation waited for a PROPFIND, which spaces and shares no longer send now that they list through graph. The helpers accept either response, so public links and the trash bin keep working on webdav.
An upload that is expected to fail never produces the 201/204 the helper waits for, so the promise stayed pending and rejected once the page closed, failing a scenario that had otherwise passed.
A public link is a drive of its own, the client builds its id from the link token. With that the listing has no webdav branch left: the loader always goes through graph, and the dav properties that only the public link needed go with it. The link token and password now ride along on the graph client too, they were only set on the webdav one.
getFileInfo was a PROPFIND with depth 0, which is what a driveItem stat is. A decorator under the vault one answers it through graph now, so all of its callers keep their signature and get permissions, share types, the download url and the preview information in the same request. The vault wrapper stays outermost: the graph request has to carry the encrypted names the server stores, and its answer is decrypted on the way back.
loadAncestorMetaData asked webdav for each ancestor with a depth 0 PROPFIND and a hand picked property list. getFileInfo answers the same thing through graph now, so the ancestors ride along and the dav properties go away. Still one request per level, graph has no ancestor endpoint.
A failing stat threw a raw axios error, so callers lost the status code they branch on, and a public link never showed its password prompt: that one hangs on telling "needs a password" from "wrong password" apart. Graph carries both as an error code, publicLinkPasswordRequired and publicLinkPasswordInvalid, which map onto the dav codes the callers already know.
Opening a file stats it, and that stat goes through graph now. The helpers match a stat on either api, the listing predicate builds on the same check and additionally looks for the expanded children.
The link's root stat returns the space the app navigates in, so it carries the link's own properties: the role comes from the actions the server capped at it, the owner from the mountpoint drive. The expiration, the share date and the item type came from dav properties with no graph counterpart, and nothing reads them. The link token rides on the request itself rather than the auth store, the very first stat runs before the store knows about it. Addressing the root is one rule now, shared by the stat and the listing: a root has no path to look up, and for a public link it is the mountpoint drive itself.
The link root carries its type: a folder for a link to a folder, a file for a link to a single file. Dav could not say, it reported "folder" for both, so the presence of a file id had to stand in for the distinction. That stand-in broke when the link started resolving through graph, where the id comes from the item rather than from a dav property: every folder link looked like a single file link and opened its first child instead of listing.
listFiles joins getFileInfo in the graph decorator, so the remaining callers move over too: upload conflict checks, the rename parent listing, save-as, the space duplicate and the app folder handling. getPathForFileId follows, the item knows where it sits. The listing itself lost its vault translation, the decorator above does that for every method, and the loader calls listFiles again like any other caller. What is left on webdav is what graph cannot answer: the trash bin and the file versions.
…ed it A drop opened by pointer, a context menu on right click for instance, only listened for escape on the drop element itself, and the key never reached it while the focus sat outside. The listener now sits on the document, registered before the drop positions itself so an early key press is not lost either, and showDrop bails out when the drop was closed again while it was still waiting for a frame.
A drive root carries neither the folder nor the file facet, it reports itself as a root, so every stat of a space root came back as a file. The app defaults folder handling reads that type to decide whether the listing or the item itself belongs in the resource list, which left the media viewer without files to show.
A caller asking for its own dav properties, the vault's integrity token for one, can only be answered by a PROPFIND, so those listings fall back to webdav. The preview follows the cleartext type as well: the server only ever sees the encrypted blob and reports no thumbnail for it.
dschmidt
added
Type:Maintenance
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Moves the client from WebDAV to the graph API for everything that reads metadata: the folder listing, the single stat behind
getFileInfo, the breadcrumb ancestors, the path lookup by file id and the public link resolution.A folder is one request now. The listing stats the folder with
$expand=children,thumbnailsand$selectfor the allowed actions, the share types and the download url, which is the same shape a PROPFIND withDepth: 1returned, minus the XML.The remaining callers keep their signatures.
listFiles,getFileInfoandgetPathForFileIdare answered by a decorator that sits under the vault one, so all 36 stat callers and the 16 listing callers moved over without touching a single call site. The vault translation moved out of the webdav decorator into a client agnostic helper, so a vault behaves the same no matter which API served the listing.Public links go through graph as well. A link is a drive of its own, the client builds its id from the token, and the token rides on the request rather than the auth store: the very first stat runs before the store knows about the link, and that stat is what tells the client whether a password is needed at all.
What stays on WebDAV is what graph has no answer for yet: the trash bin listing and the file versions. Those are the last two PROPFIND call sites in the client, and both have a spec by now, see the table below.
What this needs
Not mergeable before these are in:
@libre.graph.shareTypesin the driveItem$selectthumbnailsin the driveItem$expand$expand=thumbnailson driveItems, whichhasPreview()reads@microsoft.graph.downloadUrl, which the stat asks forThe generated client in here is built from the two unmerged spec branches (#70 and #72). Regenerate it from
mainonce they land, the diff should be the same file.Deliberately left out
getFileContentsand the inlinegetFileUrl/contentredirect, server in opencloud-eu/opencloud#3248, v1beta1 only so farputFileContentscreateUploadSession, documented in opencloud-eu/libre-graph-api#56createFolderdeleteFile,moveFilesdeleteDriveItemandupdateDriveItemare modelled and generated, this PR just does not use them yetcopyFilessetPropertiesgetFileUrlwebdav.search, they follow the graph search work in opencloud-eu/libre-graph-api#34 and opencloud-eu/opencloud#3211davPropertiesparameter that four callers still passBehaviour worth knowing
hasPreview()no longer guesses from the mime type, it reports what the server expanded. Without feat(graph): expand thumbnails on driveItems opencloud#3471 in the backend it is false everywhere and no previews are shown.public-link-item-typedav property could not: the server answered "folder" for both, so the presence of a file id had to stand in for it.Related Issue
How Has This Been Tested?
distthrough the repo's docker-composeTypes of changes