feat(file-service): accept string|resource content for streamed writes - #2184
Open
rjzondervan wants to merge 8 commits into
Open
feat(file-service): accept string|resource content for streamed writes#2184rjzondervan wants to merge 8 commits into
rjzondervan wants to merge 8 commits into
Conversation
Widen FileService::saveFile/addFile and the Create/UpdateFileHandler content parameter from string to string|resource (native mixed + @param string|resource, as PHP has no resource type keyword). On the resource path the handlers skip string-only base64 decoding, run the executable-file guard against a bounded magic-byte prefix (rewinding afterwards), and compute the change-detection md5 via hash_update_stream — so a synchronized file streams straight to storage via putContent() without being buffered 2-3x in memory. String callers are unaffected (pure type widening). Provider side of openconnector#110 (stream-file-content); the consumer-side fetchFile streaming lands in openconnector. Assisted-by: ClaudeCode:claude-opus-4-8
/js/ is gitignored but these two files remained tracked from earlier web-push commits; the ignore rule only prevents future re-adds. Assisted-by: ClaudeCode:claude-opus-5
… path contract.md for stream-file-content requires native `mixed` PLUS a `@param string|resource` docblock on every `$content` surface: PHP has no `resource` type keyword, so the docblock is what PHPStan/Psalm enforce at the call sites. `FileService::updateFile` and `UpdateFileHandler::updateFile` declared `@param mixed`, which silently widened the enforced contract on the update path — an int, array or object would pass static analysis there. Both are nullable (null = update tags/metadata only), so the accurate narrowing is `string|resource|null`. saveFile/addFile and both CreateFileHandler methods already matched the contract and are untouched. No runtime behaviour change: the is_resource() branching in both handlers was already correct and is covered by the existing handler tests (313 green). Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Robert Zondervan <robert@conduction.nl>
…n shape
`objectStore.massDeleteObject()` was called from three places
(MassDeleteObject.vue, SearchIndex.vue) but never defined, in neither this app's
store nor @conduction/nextcloud-vue. It threw `TypeError: not a function` before
any request, so the confirm button did nothing at all.
Add it, delegating to the package store's `deleteObjects(type, ids)` whose
`{successfulIds, failedIds}` return shape is exactly what the callers destructure.
Two follow-on defects that only became reachable once the action existed:
- Selection shape. The store holds plain id STRINGS (SearchIndex's
`selectedIdsForPage` does `list.map(String)` and binds `:selected-ids`, so that
contract cannot change), but the dialog renders objects and submitted
`map(obj => obj.id)` — undefined for every entry. The package store's
`_buildUrl(type, id)` appends `/${id}` only `if (id)`, so an undefined id
silently became a COLLECTION-scoped DELETE: HTTP 405 per selected row instead of
one delete per object. Resolve ids to rows inside the dialog (the only consumer
that needs objects), keep writing ids back to the store, and refuse in
massDeleteObject when nothing resolves so a falsy id can never target the
collection again.
- Post-delete refresh. `refreshObjectList()` refetches the register/schema
collection derived from registerStore/schemaStore, but the search view renders
`objectStore.searchCollection`, refilled only by `refetchSearchCollection()`.
Deleting from search left the deleted rows on screen. Call both, and close the
dialog immediately instead of after 2s — the selection is empty by then, so the
template fell back to an empty "no objects selected" confirmation the user had
to dismiss by hand.
Verified: per-id DELETE 204, collection DELETE 405, object total 40 -> 39.
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Robert Zondervan <robert@conduction.nl>
The dev instance runs Nextcloud 35.0.0.1 while the app declared `max-version="34"`, so `occ app:enable` refused it and the app had to be force-enabled. Verified working on 35: the app's own API endpoints answer with real payloads and no unresolved-service errors remain in the log. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Robert Zondervan <robert@conduction.nl>
…o feature/stream-file-content
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 555/555 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-07-29 10:07 UTC
Download the full PDF report from the workflow artifacts.
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.
Summary
$contentparameter onFileService::saveFile/addFileand theCreateFileHandler/UpdateFileHandlercontent paths fromstringtostring|resource, so a stream can be handed straight toOCP\Files\File::putContent()(which already accepts one) instead of being buffered into a PHP string. This is the provider half of stream-file-content; the OpenConnector consumer half is fix(sync): stream file downloads to disk and record the contract before file rules openconnector#1087.resourcetype keyword, so the runtime declaration ismixedand the enforced contract lives in the@param string|resourcedocblock that PHPStan/Psalm check at each call site.hash_update_stream(chunked, never buffered) and the stream rewound, so an unchanged file still skips both the write and the version bump.FileService::updateFileandUpdateFileHandler::updateFilesaid@param mixed, which silently widened the enforced contract there — an int, array or object would have passed static analysis. Both are nullable (null = update tags/metadata only), so the accurate narrowing isstring|resource|null.js/openregister-push-*.jsbundles./js/is gitignored but these stayed tracked from earlier web-push work — the ignore rule only prevents future adds.Also in this PR
Two fixes that are not part of stream-file-content but were found while verifying it, and are separable if you'd prefer them split out:
objectStore.massDeleteObject()was called from three places but never defined, in neither this app's store nor@conduction/nextcloud-vue— it threwTypeError: not a functionbefore any request, so the confirm button did nothing. Added, delegating to the package store'sdeleteObjects(type, ids). Two follow-on defects only became reachable once it existed: the store holds id strings while the dialog submittedmap(obj => obj.id)(undefined for every entry), and_buildUrl(type, id)appends/${id}onlyif (id)— so an undefined id became a collection-scoped DELETE, i.e. HTTP 405 per selected row. And the post-delete refresh calledrefreshObjectList(), which refetches a different collection than the search view renders, leaving deleted rows on screen.appinfo/info.xml.Checks
tests/Unit/Service/File/greenupdated: 5, created: 0DELETE→ 204 and collectionDELETE→ 405, and that the object total moved 40 → 39 on a mass deleteIssues
Refs ConductionNL/openconnector#1079 — this is the OpenRegister provider half of that cross-repo change, so it does not auto-close the issue.
Test plan