Page MenuHomePhabricator

[lib] Update `permissionsToBitmask` to `permissionsToBitmaskHex`
ClosedPublic

Authored by atul on Oct 24 2023, 5:01 PM.
Tags
None
Referenced Files
F3361957: D9589.diff
Sun, Nov 24, 8:38 PM
Unknown Object (File)
Fri, Nov 15, 6:16 AM
Unknown Object (File)
Mon, Oct 28, 5:21 PM
Unknown Object (File)
Oct 4 2024, 2:00 AM
Unknown Object (File)
Oct 4 2024, 2:00 AM
Unknown Object (File)
Oct 4 2024, 2:00 AM
Unknown Object (File)
Oct 4 2024, 2:00 AM
Unknown Object (File)
Oct 4 2024, 1:58 AM
Subscribers

Details

Summary

Summmary:
Was initially planning (and drafting) to encode the BigInt as base64 to be included in stringified JSON. However, as I jotted down (ENG-5373 : Use "bitmask" approach to encoding ThreadPermissions and Role permissions blobs), decided to encode as hex instead.

Update permissionsToBitmask and corresponding hasPermission to return and accept string instead of bigint.

Test Plan

Unit test, will add a few more.

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul published this revision for review.Oct 24 2023, 5:02 PM
tomek added inline comments.
lib/permissions/minimally-encoded-thread-permissions.js
43–48 ↗(On Diff #32375)

Why do we need this check?

This revision is now accepted and ready to land.Oct 25 2023, 1:09 AM
lib/permissions/minimally-encoded-thread-permissions.js
43–48 ↗(On Diff #32375)

Flow typechecker doesn't support BigInt/bigint as of the version we're on, so we do runtime type checking via invariant as "the next best thing."

Couldn't think of any better approach and didn't want to put this up without any checks

lib/permissions/minimally-encoded-thread-permissions.js
43–48 ↗(On Diff #32375)

In this particular context, we want to make sure that all bitwise operations are (BigInt, BigInt) or (Number, Number). Mixing the two can lead to undefined behavior (eg losing precision)

This revision was landed with ongoing or failed builds.Oct 25 2023, 10:06 AM
This revision was automatically updated to reflect the committed changes.