Skip to content
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

memorystore: simplify, refactor, re-enable #3332

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

itzexor
Copy link
Contributor

@itzexor itzexor commented Aug 24, 2024

Removes a lot of unused (in ABS) functionality, refactors to ES6 style class, and re-enables this custom implementation with check period and ttl of 1 day, and 1000 max entries.

The class now only implents the required (as per express-session docs) methods and removes optional methods, except touch() which allows the TTL of an entry to be refreshed without affecting its LRU recency.

There is no longer a way to stop the prune timer, but I don't believe the function was ever being called beforehand. The session store's lifetime is the same as the application's, and since it is unref()'d should not cause any shutdown issues.

ref: #3251
fixes: #2538

Removes a lot of unused (in ABS) functionality, refactors to ES6
style class, and re-enables this custom implementation with check
period and ttl of 1 day, and 1000 max entries.

The class now only implments the required (as per express-session docs)
methods and removes optional methods, except touch() which allows the
TTL of an entry to be refreshed without affecting its LRU recency.

There is no longer a way to stop the prune timer, but I don't belive
the function was ever being called beforehand. The session store's
lifetime is the same as the application's, and since it is unref()'d
should not cause any shutdown issues.
@itzexor
Copy link
Contributor Author

itzexor commented Aug 24, 2024

Store API reference: https://www.npmjs.com/package/express-session#session-store-implementation

Note that this doesn't tell the whole story, and there are a few other methods required that are non-trivial to implement. For this reason every custom implementation that I've looked at just subclassed Store (or required the user to pass express-session as an argument) rather than being an independent implementation subclassing EventEmitter

@advplyr
Copy link
Owner

advplyr commented Aug 27, 2024

I tested with lower prune/ttl values and it is working great. Thanks for fixing that!

@advplyr advplyr merged commit 1326d29 into advplyr:master Aug 27, 2024
5 checks passed
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.

[Enhancement]: Warning: connect.session() MemoryStore is not designed for a production environment
2 participants