Skip to content

refactor/introduce IContext - #9062

Draft
max-nextcloud wants to merge 39 commits into
mainfrom
refactor/introduce-icontext
Draft

refactor/introduce IContext#9062
max-nextcloud wants to merge 39 commits into
mainfrom
refactor/introduce-icontext

Conversation

@max-nextcloud

@max-nextcloud max-nextcloud commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator
  • chore(refactor): Introduce IContext and FileContext
  • chore(refactor): simplify IContext API with helper classes

Todo

Backend

  • Handle sync requests with IContext.
  • get text file in AttachmentService based on context.
  • Fix unit tests for ApiService.
  • Fix 412 resonses to push requests.
  • handle saving in IContext.
  • Address refactor/file service #9028 (comment)
  • Address refactor/file service #9028 (comment)
  • check for other requests that rely on the context.
  • Migrate existing documents.
  • Add index for context_type, context_id.
  • Have a lazy context that does not setup the filesystem unless needed.
  • always create the context on the same layer.

Frontend

  • Add context arguments to createCollaborativeEditor.
  • Pass these arguments to the CollaborativeEditor component as props.
  • Use these props to connect.
  • Provide these props when saving. We still use the documentId to save and resolve the context on the backend.

Testing

  • Test with public file shares
  • Check playwright tests
  • Fix cypress tests
  • Fix playwright print test (it's now showing the placeholder at the bottom)
  • Test with collectives

Documentation

  • document how to implement and register a Context

@max-nextcloud
max-nextcloud force-pushed the refactor/introduce-icontext branch 9 times, most recently from 3e17210 to a2260eb Compare August 17, 2026 10:44
@max-nextcloud max-nextcloud changed the title refactor/introduce icontext refactor/introduce IContext Aug 17, 2026
@max-nextcloud
max-nextcloud force-pushed the refactor/introduce-icontext branch 4 times, most recently from ae7bbf6 to 92e1890 Compare August 17, 2026 13:20
@max-nextcloud
max-nextcloud force-pushed the refactor/file-service branch 4 times, most recently from c9d35f7 to cd2d5b9 Compare August 18, 2026 18:29
@max-nextcloud
max-nextcloud force-pushed the refactor/introduce-icontext branch from b0a7b15 to 7cedf66 Compare August 18, 2026 18:50
@max-nextcloud
max-nextcloud force-pushed the refactor/file-service branch from cd2d5b9 to c4cca9f Compare August 19, 2026 04:32
@max-nextcloud
max-nextcloud force-pushed the refactor/introduce-icontext branch from 7cedf66 to 1bb5586 Compare August 19, 2026 04:34
@max-nextcloud
max-nextcloud force-pushed the refactor/file-service branch 2 times, most recently from 7232808 to 5907ff4 Compare August 19, 2026 05:35
@max-nextcloud
max-nextcloud force-pushed the refactor/introduce-icontext branch 3 times, most recently from 1bc577e to 58fe27f Compare August 19, 2026 09:00
Base automatically changed from refactor/file-service to main August 19, 2026 09:07
@blizzz

blizzz commented Aug 19, 2026

Copy link
Copy Markdown
Member

So far I read up to commit chore(tweak): return first editable file early. I love your iterative style and the discipline you keep with having small, digestible and sensible commits! My impression is also that with having the abstraction layers, there are many places where it just gets cleaner, and having specific logic better capsuled!

What I stumble over is that the IContext file contains the additional classes DocumentData, SessionInfo, NewSessionData. Usually this is something we keep separate and I am not sure it will always works with autoloading? Also about fellow developer expectations. Maybe something to consider.

Maybe this is also OK, just to double check: a call to removeInactiveSessionsWithoutSteps() within the create flow was removed, and not introduced back elsewhere. Alright if intentional of course.

No biggie: new classes miss the declare(strict_types=1); statement.

Can consider to mark the new event unstable or experimental, though I am not aware of a phpdoc or internal established annotation.

@blizzz

blizzz commented Aug 19, 2026

Copy link
Copy Markdown
Member

On my dev instance I get an error during Migration:

{
  "reqId": "miQrQfvExjkhgL8Z3V6T",
  "level": 3,
  "time": "2026-08-19T14:14:45+00:00",
  "remoteAddr": "",
  "user": "--",
  "app": "updater",
  "method": "",
  "url": "--",
  "scriptName": "occ",
  "message": "\\OC\\Updater::failure: Exception: Database error when running migration 090000Date20260817110024 for app text\nAn exception occurred while executing a query: SQLSTATE[23502]: Not null violation: 7 ERROR:  column \"context_type\" of relation \"oc_text_documents\" contains null values",
  "userAgent": "--",
  "version": "35.0.0.3",
  "occ_command": [
    "occ",
    "upgrade"
  ],
  "data": {
    "app": "updater"
  }
}

DB is postgres.

Probably happens because the column is added as NOT NULL but the rows would be empty (=> NULL). Maybe set the default to "file"?

UPDATE: same with context_id. Default to 0?

Non-file contexts will need to return null.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
We handle the NoLockProviderException gracefully anyway.

Signed-off-by: Max <max@nextcloud.com>
Addresses #9028 (comment) .

Signed-off-by: Max <max@nextcloud.com>
…entsController

Most controllers use the `token` parameter for shares.
The AttachmentsController however uses the `shareToken` parameter.

This prevented `assertDocumentSession` from passing,
as the middleware always tried to set a userId which was not present.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
* Start without `notnull`.
* Migrate data so `context_type` and `context_id` are filled.
* Set `notnull` on those columns.

Signed-off-by: Max <max@nextcloud.com>
* Return the schema - even if the migrations do nothing.
* Use the plain column name of the column to set.
  Fixes the migration on postgres DB.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
When creating a new session we load the document by context.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Droped in a7a63d2 .

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
File ids used to be document ids. But not anymore.

In all current calls to `resetDocument` the parameter send actually is a file id.

Follow up: Make this generic for all context types.--signoff

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
@max-nextcloud
max-nextcloud force-pushed the refactor/introduce-icontext branch from 351b1b4 to c2cdf5e Compare August 23, 2026 07:19
@max-nextcloud
max-nextcloud force-pushed the refactor/introduce-icontext branch from c2cdf5e to 336adbd Compare August 23, 2026 09:26
Signed-off-by: Max <max@nextcloud.com>
* Use IShare and IUser in the file context api explicitely.
* Make creating the context cheap.
* File is accessed when needed and then cached.

Signed-off-by: Max <max@nextcloud.com>
@max-nextcloud
max-nextcloud force-pushed the refactor/introduce-icontext branch from 336adbd to f1c2797 Compare August 23, 2026 09:51
The two used to be the same - but now we have contexts to store the file id.

Signed-off-by: Max <max@nextcloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants