Repository navigation
Export file data in GET /whfs/object as a public resource - #196
RichardHoekstra wants to merge 1 commit into
Conversation
The virtual.objectdata instance returned the live ResourceDescriptor, including internal fields, instead of an exported resource. Use the export path with the request's exportOptions, as the other instances and buildExportMetadata already do.
that's broken, and probably caused by WHFS Export leaking through But WHFS Export supplies data in a separate file - I wonder if we shouldn't we have a GET /whfs/data instead that delivers the body immediately, without a subfetch. Or offer both routes. Similarly there could be a PUT /whfs/data that would allow uploads - up to a certain size (very large uploads up to an administrator specified limit (5MB? 50MB?) should be chunked using @webhare/upload). This would also help to move some current users off webdav and to the webhare api (they're now using PUT and GET on the Webdav endpoints using simple curl) |
|
I think we should fix getVirtualObjectData not returning an ExportedResource. Keeping the data there would be good for atomic updates, so I don't think we should remove it. I'm not sure if the the POST/GET /whfs/data would be necessary. It can only work for simple/small files (because of the POST size restrictions), so it would probably need chunking upload code as fallback anyway - negating the 'simple implementation' argument . |
it might be hard to do @webhare/upload-protocol uploads from curl, and submitting it base64 inside a hand-crafted JSON request will hit limits even faster But it looks like nginx should be able to do upload validation in a js_content handler, so we may be able to increase limits for specific URLs for specific authenticated users. So /whfs/data endpoints may still be nice to have OTOH I agree that atomic updates are useful - specifically so you can change file type together with the new data. So let's do both eventually but fix /whfs/object for now |
| const actor = test.getUser("sysop").wrdId; | ||
| const hsSchema = await loadlib("mod::wrd/lib/api.whlib").openWRDSchema("webhare_testsuite:testschema"); | ||
| const rights = await loadlib("mod::wrd/lib/auth.whlib").GetWRDAuthUserAPI(hsSchema); | ||
| const owner = await rights.GetUser(actor) as HSVMObject; |
There was a problem hiding this comment.
can you merge this test into test_whapi_whfs to reuse all the (user) setup already being done there ?
| const rights = await loadlib("mod::wrd/lib/auth.whlib").GetWRDAuthUserAPI(hsSchema); | ||
| const owner = await rights.GetUser(actor) as HSVMObject; | ||
| const root = await test.getTestSiteJSTemp(); | ||
| await runInSeparateWork(() => owner.UpdateGrant("grant", "system:fs_browse", root.id, owner, {})); |
There was a problem hiding this comment.
why is this necessary? owner = sysop = has system:sysop granted above. is this working around another bug?
@RichardHoekstra I agree with this - getVirtualObjectData has only 2 callsites, and only one sets includeData: true (and doesn't pass on exportOptions, which getVirtualObjectData seems to support) so why not invoke export on |
Problem
GET /whfs/object?instances=*returns the file contents in theplatform:virtual.objectdatainstance as a serialised internalResourceDescriptor, with fields such as_resourceand_dbLoc. An API client cannot use that to get the bytes. Every other instance in the same response, andbuildExportMetadatainwhfs/src/export.ts, go through the export path withexportOptionsand produceExportedResourcevalues (file.base64orfile.fetch).Cause
whtree/modules/platform/openapi/api/whfs.ts, ingetInstances:With
includeData: true,getVirtualObjectDataputs the liveResourceDescriptorindata, andexportOptionsis not passed in.Fix
getVirtualObjectData(obj, { includeData: false }, exportOptions), asbuildExportMetadataalready does.data: await file.data?.export(exportOptions) ?? null.data: nullfor an empty file, so clients can still tell "empty file" apart from "object without data". Folders and richdocuments get nodatakey.There is no new endpoint and no schema change. One side effect:
exportOptionsnow reachesgetVirtualObjectData, so link targets in this instance are exported in the same way as in the other instances.Test
New
system.whapi.test_whapi_whfs_data. It creates files and reads them through the real OpenAPI client with a first-party token:café 🌍), a PNG and an empty file: the bytes match exactly, throughbase64or by followingfetch, and no_resource/_dbLocfields remain;datakey.The test failed on the internal descriptor fields before the fix and passed after it, on a dev container built from main
601237c9. The patch applies cleanly to mainf63eb69b. We found this while reading WHFS files through the API from an external tool.