ποΈ PUT-1702: TeamStore β workspace CRUD, handle uniqueness, reserved words - #3709
Conversation
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
e844c2f to
9e89579
Compare
9e89579 to
77e40ef
Compare
77e40ef to
cfb3d07
Compare
cfb3d07 to
847aad0
Compare
847aad0 to
e07708d
Compare
e07708d to
023482a
Compare
023482a to
342b32d
Compare
342b32d to
f1a6bcb
Compare
f1a6bcb to
d0e28cc
Compare
Salazareo
left a comment
There was a problem hiding this comment.
since were calling feature "team" maybe we change table name to teams also, so groups can be more akin to the workspace concept of groups, like within a team?
d0e28cc to
ba55583
Compare
ba55583 to
470ac9b
Compare
470ac9b to
425efd2
Compare
425efd2 to
beb27ad
Compare
beb27ad to
18b8059
Compare
18b8059 to
1bfa430
Compare
`GroupStore` has only addUsers/removeUsers; nothing creates, reads back or lists a group at runtime. `TeamStore` is that missing half, scoped to rows with `kind = 'team'`. A workspace is addressed by `uid`, which `group` has carried as NOT NULL UNIQUE since 0015. `handle` is a mutable display label with no addressing role, so a rename invalidates nothing and a stale reference can never resolve to a different workspace. Soft delete releases the handle and keeps `name`. Nothing points at a handle, so the name returns to the pool instead of being reserved forever by a global unique index that cannot exclude dead rows -- mysql has no partial indexes, so that exclusion was never available. Handles validate to ^[a-z0-9]+(-[a-z0-9]+)*$, 3-64 chars, against a reserved list. The charset is deliberately narrower than the column so the engines' collations cannot disagree: mysql's utf8mb4_unicode_ci also folds accents and eszett, which sqlite's NOCASE and postgres's lower() do not. Every read filters `kind = 'team' AND deleted_at IS NULL`, which is what makes the seeded admin/system groups unreachable rather than merely absent. Handle lookups compare lower(handle) on postgres, where the index is on that expression rather than the column.
1bfa430 to
37cb11d
Compare
Local validationEngine: mysql 8 Β· Create, read, rename Handle uniqueness across owners β a second user taking the same handle: A conflict, not a 500 β the unique index is caught and translated. Handle validation runs before the index. Worth noting for review: this means a case-differing handle is rejected at Reserved handles One observation, not a defect: |
First code of phase 2.
GroupStorehas onlyaddUsers/removeUsersβ nothing creates, reads back or lists a group at runtime.TeamStoreis that missing half, scoped to rows withkind = 'team'.A workspace is addressed by
uid, not by handlegroup.uidhas beenNOT NULL UNIQUEsince0015, so the identity already existed β this just uses it.handleis a mutable display label with no addressing role.That matters because the alternative fails in a way worse than an error. If routes keyed on handle, a rename would invalidate every stored reference β and once the freed handle was claimed by someone else, an old reference would not 404, it would resolve to a different workspace. There is a test (
keeps the uid valid after the handle changes) pinning the property.β This changes the API surface already written up in the design doc and in PUT-1708 / PUT-1735 / PUT-1736 / PUT-1746: every route and SDK method now takes
:uidinstead of:handle. Those are updated. One consequence is worth a second opinion β see the open question at the bottom.Soft delete releases the handle
softDeletesetsdeleted_atand nullshandle, keepingname.Without this, a global unique index with no dead-row exclusion reserves a deleted workspace's handle permanently. MySQL has no partial indexes, so "exclude soft-deleted rows from the index" was never available as a fix β it had to be handled in the write path. Since nothing addresses a workspace by handle, releasing it dangles nothing;
namesurvives so the row still reads sensibly in history.Handle validation
^[a-z0-9]+(-[a-z0-9]+)*$, 3β64 characters, plus a 42-entry reserved list (admin,puter-support,security,billing, β¦) because a handle appears in activation email and share dialogs, where one that reads like Puter itself makes impersonation more convincing.The charset is deliberately narrower than the column accepts, and that is the interesting part. Case-insensitive uniqueness is spelled three different ways across our engines, and they do not mean the same thing:
utf8mb4_unicode_ciΓβssCOLLATE NOCASElower(handle)So
cafΓ©/cafecollide on mysql alone, andΓΓ/Àâon mysql and postgres but not sqlite β a handle accepted on self-hosted sqlite could be refused on prod mysql. Restricting to[a-z0-9-]makes all three differences unreachable rather than relying on the engines to agree.Two details a reviewer should check
Every read filters
kind = 'team' AND deleted_at IS NULL. That predicate is what makes the seededadmin/systemgroups unreachable rather than merely absent β the criterion inherited from PUT-1699.never returns a seeded system group from any methodwalks everykind IS NULLrow and asserts the store returns null for each. A team admin is not a platform admin.Handle lookups are dialect-aware. Postgres indexes
lower(handle), so#handleMatch()compareslower(handle) = lower(?)there and a plain=elsewhere, where the column collation already handles it.Verification
sqlite and postgres β both verified
Confirming the postgres branch is load-bearing, not defensive
Removing the dialect branch so
#handleMatch()always returns`handle` = ?, then re-running on postgres:Note which test survives:
refuses a duplicate handle, including one differing only in casestill passes, because the unique index catches that regardless. It is the lookup that breaks. That is the same distinction flagged in #3704 β uniqueness and lookup are separately case-sensitive, and getting one right does not give you the other.Full suite and typecheck
Open question this raises β not decided here
PUT-1726specifies sharing asputer.fs.share('/me/Docs', { team: 'design-team' }, 'write')β a handle.That is genuinely not the same case as a route: it is typed by a person and resolved immediately, so it is an input rather than a stored reference, and a uuid there would be miserable to write. But a scripted share to a handle keeps working after that handle is released and reclaimed, and would silently start sharing with an unrelated workspace. A 404 would be safe; retargeting is not.
Left open in PUT-1726 and Β§11 of the design doc rather than settled quietly. Three options are written up there.
Closes PUT-1702.