Skip to content

feat: replace ID with UID - #363

Open
sabhas wants to merge 9 commits into
mainfrom
issue-361
Open

feat: replace ID with UID#363
sabhas wants to merge 9 commits into
mainfrom
issue-361

Conversation

@sabhas

@sabhas sabhas commented May 9, 2023

Copy link
Copy Markdown
Member

BREAKING CHANGE: remove auto incremental ids from user, group and permissions and add a virtual uid property that returns string value of documents object id

Issue

Closes #361.

While addressing #359, the auto-incrementing sequence counter used for
User, Group, and Permission IDs was found to be broken on Cosmos DB
(it set every ID to 1). More generally, sequencing via a shared counter
document is not a good fit: it's a bottleneck on every
insert and a source of race conditions under concurrent writes.

Intent

Replace auto-incremental numeric IDs with a string uid across User,
Group, and Permission, and make the API consistently expose uid
instead of id wherever these entities appear in a response.

Implementation

  • Added a virtual uid property to the User, Group, and Permission
    Mongoose models, returning this._id.toString(). No new field is stored
    and no data migration is required — every document already has _id,
    old and new alike.
  • Removed the old sequencing mechanism entirely: the Counter model and
    getSequenceNextValue utility are gone, along with the bottleneck/race
    condition they caused.
  • Updated all API responses that expose a user, group, or permission
    identifier — including login (POST /SASLogon/login), session
    (GET /SASjsApi/session), and the user/group/permission endpoints
    — to consistently return uid.
  • Updated the web frontend (permission management, user profile, session
    restoration on page load) to read uid instead of id.

This is a breaking change for any existing client relying on numeric
IDs from this API — endpoints now return an opaque string identifier
instead.

Checks

  • Code is formatted correctly (npm run lint:fix).
  • Any new functionality has been unit tested.
  • All unit tests are passing (npm test).
  • All CI checks are green.
  • Reviewer is assigned.

sabhas added 4 commits May 9, 2023 15:01
BREAKING CHANGE: remove auto incremental ids from user, group and permissions and add a virtual uid property that returns string value of documents object id
@sabhas
sabhas requested a review from YuryShkoda May 11, 2023 06:21
Comment thread api/src/routes/api/spec/auth.spec.ts Outdated
Comment thread api/src/routes/api/spec/web.spec.ts Outdated
Comment thread web/src/context/appContext.tsx Outdated
Comment thread web/src/context/appContext.tsx Outdated
Comment thread web/src/utils/types.ts Outdated
Comment thread web/src/utils/types.ts Outdated
Comment thread web/src/utils/types.ts Outdated

@YuryShkoda YuryShkoda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments above

@sabhas
sabhas requested a review from YuryShkoda August 8, 2023 10:08
Execution.spec.ts, processProgram.spec.ts and code.spec.ts didn't
exist when issue-361 (ID -> UID) branched, so they were written
against the old userId: number shape. Update them to match the
merged-in string-based uid now that main has been merged in.
POST /SASLogon/login and GET /SASjsApi/session still returned the
old `id` field, while the rest of the ID->UID migration (#363)
standardized on `uid`. Not functionally broken - Mongoose provides
a built-in `id` virtual by default (_id.toHexString()) that happened
to resolve to the same value as the new `uid` virtual - but it's an
inconsistent public API surface, and relying on that coincidence
wasn't the intent of the migration.

Neither of these files was touched by any of issue-361's own
commits, so this predates the merge rather than being caused by it.

- web.ts: login response and session storage now source from
  user.uid explicitly
- session.ts: SessionResponse dropped its Omit<UserResponse, 'uid'>
  + id override in favor of just extending UserResponse
- verifyTokenInDB.ts: token-refresh path, same fix
- login.tsx / appContext.tsx: updated to read the corrected field

Verified with a real end-to-end request (genuine app boot, real
MongoDB, real CSRF handshake) - not just type-checking - to confirm
the actual HTTP response bodies carry uid, not id.
@YuryShkoda YuryShkoda self-assigned this Jul 15, 2026
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.

replace ID with UID

2 participants