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

add save anon session feature #1162

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

juanetch
Copy link
Contributor

@juanetch juanetch commented Jan 19, 2022

What?

Added app configuration option to enable/disable saving anonymous user sessions.

Why?

Feature adds more control to a developer over behavior of session based cookies. Whether for performance (ex. bloating session storage with anonymous user sessions) or for regulatory reasons (ex. GDPR and allowing cookies), a developer can now choose to set a session based cookie when required (ex. after authenticating).

How?:

If disabled - no session is set when request arrives, instead, function access to SessionManager is provided through req->setSessionManager() and then req->setSession polymorphism will work. req-setSession() returns boolean if SessionManagerPtr is available since it might not be if config enable_session is false. We may also just call req-session() and if SessionPtr is not available it will try to setSession using the SessionManager.

Testing?

  • Client integration: Remove Get Cookie test and move it to Test Session (this is the test that makes use of session).
  • TimeFilter - if sessionId is not present - it is created through req->setSession().

Anything Else?

  • Enable/Disable session could potentially be removed since with this feature since it allows developer to set session when required.
  • Moved SessionManager.h to INCLUDES because HttpRequest.h requires it - not sure if this is correct.
  • Providing access to SessionManager in HttpAppFrameworkImpl by passing a naked pointer (SessionManager is a unique_ptr) was done for performance considerations (using shared_ptr could eat up resources if many request share pointer), but could be negligible as I did not test performance - not an expert on this.
  • Not sure if I misused virtual functions.

@an-tao an-tao requested a review from marty1885 January 20, 2022 01:35
@Mis1eader-dev
Copy link
Member

Potentially closes and is related to #1852, that is if it is updated and merge conflicts are resolved

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.

3 participants