Conversation
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
YuryShkoda
reviewed
Aug 8, 2023
YuryShkoda
reviewed
Aug 8, 2023
YuryShkoda
reviewed
Aug 8, 2023
YuryShkoda
reviewed
Aug 8, 2023
YuryShkoda
reviewed
Aug 8, 2023
YuryShkoda
reviewed
Aug 8, 2023
YuryShkoda
reviewed
Aug 8, 2023
YuryShkoda
approved these changes
Aug 8, 2023
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.
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
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.
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, andPermissionIDs was found to be broken on Cosmos DB(it set every ID to
1). More generally, sequencing via a shared counterdocument 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
uidacrossUser,Group, andPermission, and make the API consistently exposeuidinstead of
idwherever these entities appear in a response.Implementation
uidproperty to theUser,Group, andPermissionMongoose models, returning
this._id.toString(). No new field is storedand no data migration is required — every document already has
_id,old and new alike.
Countermodel andgetSequenceNextValueutility are gone, along with the bottleneck/racecondition they caused.
identifier — including login (
POST /SASLogon/login), session(
GET /SASjsApi/session), and theuser/group/permissionendpoints— to consistently return
uid.restoration on page load) to read
uidinstead ofid.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
npm run lint:fix).npm test).