-
Notifications
You must be signed in to change notification settings - Fork 8
feat: Add memory session manager #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes introduce a session management system featuring a Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
lib/index.ts (1)
1-1: Potential unresolved references due to removal of export statement fortypes.The removal of the export statement in
lib/index.tsmight lead to unresolved references in the following files that import types directly from./types:
lib/main.test.tslib/main.tsPlease ensure that all necessary types are still accessible where needed.
Analysis chain
Verify the impact of removing the export statement for
types.The removal of the export statement for
typesmight affect other parts of the codebase that rely on these types. Ensure that all necessary types are still accessible where needed.Run the following script to verify the usage of the
typesexport:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `types` export. # Test: Search for the usage of the `types` export. Expect: No unresolved references. rg --type typescript 'from "./types"'Length of output: 74
Script:
#!/bin/bash # Description: Verify the usage of the `types` export. # Test: Search for the usage of the `types` export. Expect: No unresolved references. rg --type ts 'from "./types"'Length of output: 232
lib/sessionManager/types.ts (1)
1-5: Fix typographical errors in the comment.There are minor typographical errors in the comment. Apply this diff to fix them:
- * This interfaces provides the contract that an session management utility must - * satisfiy in order to work with this SDK, please vist the example provided in the + * This interface provides the contract that a session management utility must + * satisfy in order to work with this SDK. Please visit the example provided in the
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
Files selected for processing (9)
- lib/index.ts (1 hunks)
- lib/main.test.ts (3 hunks)
- lib/sessionManager/index.ts (1 hunks)
- lib/sessionManager/stores/memory.test.ts (1 hunks)
- lib/sessionManager/stores/memory.ts (1 hunks)
- lib/sessionManager/types.ts (1 hunks)
- lib/sessionManager/utils.test.ts (1 hunks)
- lib/sessionManager/utils.ts (1 hunks)
- readme.md (1 hunks)
Additional comments not posted (32)
lib/index.ts (1)
3-3: LGTM!The addition of the export statement for
sessionManagermakes the session management functionalities available to other parts of the codebase.The code changes are approved.
lib/sessionManager/utils.ts (1)
1-10: Add type annotation for theresultarray.The function is correctly implemented and handles edge cases. Adding a type annotation for the
resultarray would improve readability.export function splitString(str: string, length: number): string[] { if (length <= 0) { return []; } - const result = []; + const result: string[] = []; for (let i = 0; i < str.length; i += length) { result.push(str.slice(i, i + length)); } return result; }lib/sessionManager/index.ts (4)
1-5: LGTM!The type declaration for
StorageSettingsTypeis correctly implemented.The code changes are approved.
7-22: Clarify the usage of thestorePasswordfield.The constant
storageSettingsis correctly initialized with default values. However, thestorePasswordfield is specific to cookies, which might be confusing. Add a comment to clarify its usage.export const storageSettings: StorageSettingsType = { /** * The password to encrypt the store. (cookies only) */ storePassword: "", /** * The prefix to use for the storage keys. */ keyPrefix: "kinde-", /** * The maximum length of the storage. * * If the length is exceeded the items will be split into multiple storage items. */ maxLength: 2000, };
24-24: LGTM!The addition of the export statement for
MemoryStoragemakes the memory storage functionalities available to other parts of the codebase.The code changes are approved.
25-25: LGTM!The addition of the export statement for
typesmakes the types available to other parts of the codebase.The code changes are approved.
readme.md (1)
30-41: LGTM!The new section on "Session Managers" is well-written and provides clear information about
storageSettingsandMemoryStorage.lib/sessionManager/types.ts (3)
8-12: LGTM!The
StorageKeysenum is well-defined and provides a clear set of keys for session management.
14-18: LGTM!The
StorageSettingsTypetype is well-defined and provides a clear structure for storage settings.
20-50: LGTM!The
SessionManagerinterface is well-defined and provides a clear contract for session management utilities.lib/sessionManager/utils.test.ts (2)
1-3: LGTM!The import statements are correct and necessary for the tests.
5-47: LGTM!The test suite for the
splitStringfunction is comprehensive and covers various scenarios, including edge cases.lib/sessionManager/stores/memory.ts (5)
1-3: LGTM!The imports are necessary and correctly included.
12-18: LGTM!The
destroySessionmethod is correctly implemented.
42-64: LGTM!The
getSessionItemmethod is correctly implemented.
67-79: LGTM!The
removeSessionItemmethod is correctly implemented.
20-39: LGTM! But verify the handling of non-string values.The
setSessionItemmethod is correctly implemented. However, ensure that non-string values are handled correctly.The code changes are approved.
Run the following script to verify the handling of non-string values:
lib/sessionManager/stores/memory.test.ts (10)
1-3: LGTM!The imports are necessary and correctly included.
16-21: LGTM!The test for setting and getting an item in session storage is correctly implemented.
23-33: LGTM!The test for removing an item from session storage is correctly implemented.
35-45: LGTM!The test for clearing all items from session storage is correctly implemented.
55-60: LGTM!The test for setting and getting an item in storage: StorageKeys is correctly implemented.
62-72: LGTM!The test for removing an item from storage: StorageKeys is correctly implemented.
74-84: LGTM!The test for clearing all items from storage: StorageKeys is correctly implemented.
86-91: LGTM!The test for setting and getting an item in extra storage is correctly implemented.
93-101: LGTM!The test for removing an item from extra storage is correctly implemented.
103-111: LGTM!The test for clearing all items from extra storage is correctly implemented.
lib/main.test.ts (5)
Line range hint
1-8: LGTM!The imports are necessary and correctly included.
Line range hint
10-13: LGTM!The test for encoding a string to base64 URL safe format is correctly implemented.
Line range hint
15-25: LGTM!The tests for removing trailing slashes from URLs and returning the same URL if no trailing slash are correctly implemented.
Line range hint
27-41: LGTM!The tests for mapping login method params to URL params and handling undefined values in options are correctly implemented.
Line range hint
43-115: LGTM!The tests for generating the correct auth URL with required parameters, including optional parameters if provided, and handling default responseType if not provided are correctly implemented. The inclusion of the new
stateparameter is correctly handled.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- lib/sessionManager/index.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- lib/sessionManager/index.ts
Explain your changes
This adds an implementation of a session manager with configuration.
Currently there is a single manager of
memoryStorage, more will be added including cookie and session.The memory manager keys have a default set of keys of
accessToken,idTokenandrefreshToken, this can be added to by passing in an enum as a generic when setting the storage typeChecklist
🛟 If you need help, consider asking for advice over in the Kinde community.