Opened 4 years ago

Closed 3 years ago

#32088 closed New feature (wontfix)

Django Database Sessions - Can't Retrieve expire_date from request.session (SessionStore)

Reported by: Nate Pinchot Owned by: Pallav Parikh
Component: contrib.sessions Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Nate Pinchot)

For Django database sessions, there is no way to retrieve the datetime that the session will expire without directly querying the model. The model stores this value in the expire_date field, but it is not able to be retrieved from the session (i.e. request.session SessionStore) object. This makes it complicated to implement sliding expiration sessions if the sessions are not being modified by some other means.

Of course, we could set SESSION_SAVE_EVERY_REQUEST = True to cause the session to be modified on every request, but this is inefficient.

Currently, to retrieve the expire_date from a request's session we could do this. (Taken from the docs https://docs.djangoproject.com/en/3.1/topics/http/sessions/#using-sessions-out-of-views)

>>> from django.contrib.sessions.models import Session
>>> s = Session.objects.get(pk='2b1189a188b44ad18c35e113ac6ceead')
>>> s.expire_date
datetime.datetime(2005, 8, 20, 13, 35, 12)

This is not ideal since it requires an extra query to the database and if we are in the request lifecycle we already have access to the session via request.session. The ideal solution would be if django.contrib.sessions.backends.db.SessionStore would make expire_date available since it has already queried the session database record in the load() function.

Change History (16)

in reply to:  description ; comment:1 by Pallav Parikh, 4 years ago

Replying to Nate Pinchot:

For Django database sessions, there is no way to retrieve the datetime that the session will expire without directly querying the model. The model stores this value in the expire_date field, but it is not able to be retrieved from the session object. This makes it complicated to implement sliding expiration sessions if the sessions are not being modified by some other means.

Of course, we could set SESSION_SAVE_EVERY_REQUEST = True to cause the session to be modified on every request, but this is inefficient.

what does session object represent here, is it request.session ?
can you please specify more details.

in reply to:  1 comment:2 by Nate Pinchot, 4 years ago

Description: modified (diff)

Replying to Pallav Parikh:

Replying to Nate Pinchot:

For Django database sessions, there is no way to retrieve the datetime that the session will expire without directly querying the model. The model stores this value in the expire_date field, but it is not able to be retrieved from the session object. This makes it complicated to implement sliding expiration sessions if the sessions are not being modified by some other means.

Of course, we could set SESSION_SAVE_EVERY_REQUEST = True to cause the session to be modified on every request, but this is inefficient.

what does session object represent here, is it request.session ?
can you please specify more details.

Correct, I am referring to request.session. I updated the ticket description and added more detail.

comment:3 by Nate Pinchot, 4 years ago

Summary: Django Database Sessions - Can't Retrieve expire_dateDjango Database Sessions - Can't Retrieve expire_date from request.session (SessionStore)

comment:4 by Nate Pinchot, 4 years ago

Description: modified (diff)

in reply to:  description comment:5 by Pallav Parikh, 4 years ago

Replying to Nate Pinchot:

would it make sense to have setter for expire_date in SessionStore?
such as: request.session.expire_date returns current session's expire_date value. and

>>> new_expire_date = request.session.expire_date + timedelta(days=2)
>>> request.session.expire_date
datetime.datetime(2005, 8, 18, 13, 35, 12)
>>> request.session._expire_date = new_expire_date
>>> # only on request.session.save() new_expire_date gets stored. 
>>> request.session.save()
>>> request.session._get_session_from_db().expire_date
datetime.datetime(2005, 8, 20, 13, 35, 12)

setting expire_date cases:

  • on every save() call, sets expire_date for current SessionStore i.e request.session instance.
  • set new datetime value: Valid
  • set anything else except datetime: Invalid -> in this case, does't set new value but preserves current expire_date value

Let me know, if this looks okay, or any other suggestion.

Last edited 4 years ago by Pallav Parikh (previous) (diff)

comment:6 by Pallav Parikh, 4 years ago

Owner: changed from nobody to Pallav Parikh
Status: newassigned

comment:7 by Pallav Parikh, 4 years ago

Version: 3.1master

comment:8 by Pallav Parikh, 4 years ago

Has patch: set
Triage Stage: UnreviewedAccepted

comment:9 by Nate Pinchot, 4 years ago

Replying to Pallav Parikh:

Let me know, if this looks okay, or any other suggestion.

I think this is a great concept to solve it. Thank you!

Can we amend the list of setting expire_date cases?

setting expire_date cases:

  • on every save() call, sets expire_date for current SessionStore i.e request.session instance.
  • set new datetime value: Valid
  • set anything else except datetime: Invalid -> in this case, does't set new value but preserves current expire_date value
  • on every load() call, sets expire_date for current SessionStore i.e request.session instance.

This additional case would set expire_date efficiently each time the session is loaded, so an additional query isn't needed if the session isn't being saved. Currently, for my case since the session is not being saved, I would cause an additional query by referencing request.session.expire_date.

I see the PR you created. https://github.com/django/django/pull/13524/files#diff-0804b404a59dd4551b51b9e62e5a6ca8R44

Perhaps this PR could also update the load() function to something similar to below.

def load(self):
    s = self._get_session_from_db()
    if s:
        self._expire_date = s.expire_date
        return self.decode(s.session_data)
    return {}

I would have put the suggestion in the PR instead, but since we've been conversing here and also because the load() function is currently unchanged, I could not add a targeted comment on the PR. If it's preferable for me to comment on the PR instead with code suggestions, let me know :)

Let me know your thoughts on this?

in reply to:  9 comment:10 by Pallav Parikh, 4 years ago

Replying to Nate Pinchot:

Replying to Pallav Parikh:

Let me know, if this looks okay, or any other suggestion.

I think this is a great concept to solve it. Thank you!

Can we amend the list of setting expire_date cases?

setting expire_date cases:

  • on every save() call, sets expire_date for current SessionStore i.e request.session instance.
  • set new datetime value: Valid
  • set anything else except datetime: Invalid -> in this case, does't set new value but preserves current expire_date value
  • on every load() call, sets expire_date for current SessionStore i.e request.session instance.

This additional case would set expire_date efficiently each time the session is loaded, so an additional query isn't needed if the session isn't being saved. Currently, for my case since the session is not being saved, I would cause an additional query by referencing request.session.expire_date.

I see the PR you created. https://github.com/django/django/pull/13524/files#diff-0804b404a59dd4551b51b9e62e5a6ca8R44

Perhaps this PR could also update the load() function to something similar to below.

def load(self):
    s = self._get_session_from_db()
    if s:
        self._expire_date = s.expire_date
        return self.decode(s.session_data)
    return {}

I would have put the suggestion in the PR instead, but since we've been conversing here and also because the load() function is currently unchanged, I could not add a targeted comment on the PR. If it's preferable for me to comment on the PR instead with code suggestions, let me know :)

Let me know your thoughts on this?

sure, I will update the PR for above. Thanks.

comment:11 by Carlton Gibson, 4 years ago

Resolution: wontfix
Status: assignedclosed

Hi Nate.

I'm struggling to see the use-case here.

Sessions already expose methods to get and set the expiry, and these work perfectly well with DB backend:

>>> import datetime
>>> from django.contrib.sessions.backends.db import SessionStore
>>> session = SessionStore('some_session_key') 
>>> session.get_expiry_date()
datetime.datetime(2020, 10, 27, 9, 2, 7, 300236, tzinfo=<UTC>)
>>> # Bump the expiry date. We could use any base date, but let's choose the existing session expiry: 
>>> session.set_expiry(session.get_expiry_date() + datetime.timedelta(7))
>>> session.get_expiry_date()
datetime.datetime(2020, 11, 3, 9, 10, 43, 930981, tzinfo=<UTC>)
>>> # Session will be saved with new expiry: 
>>> session.modified
True

The example you link to has been in the docs since the very beginning 21c4526557fb50b9b62f32065c7d3952ea35c5df — accessing the saved date on the model isn't really something you need to do — it's sole purpose is to act as an invalidator in the get() call.

You'd be welcome to add extra code to a subclass but I can't see that it's something that we'd want to add to the code itself.
Perhaps I'm missing something but I think we need to say wontfix here.

comment:12 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedUnreviewed

in reply to:  11 comment:13 by Nate Pinchot, 4 years ago

Replying to Carlton Gibson:

Hi Nate.

I'm struggling to see the use-case here.

Sessions already expose methods to get and set the expiry, and these work perfectly well with DB backend:

>>> import datetime
>>> from django.contrib.sessions.backends.db import SessionStore
>>> session = SessionStore('some_session_key') 
>>> session.get_expiry_date()
datetime.datetime(2020, 10, 27, 9, 2, 7, 300236, tzinfo=<UTC>)
>>> # Bump the expiry date. We could use any base date, but let's choose the existing session expiry: 
>>> session.set_expiry(session.get_expiry_date() + datetime.timedelta(7))
>>> session.get_expiry_date()
datetime.datetime(2020, 11, 3, 9, 10, 43, 930981, tzinfo=<UTC>)
>>> # Session will be saved with new expiry: 
>>> session.modified
True

The example you link to has been in the docs since the very beginning 21c4526557fb50b9b62f32065c7d3952ea35c5df — accessing the saved date on the model isn't really something you need to do — it's sole purpose is to act as an invalidator in the get() call.

You'd be welcome to add extra code to a subclass but I can't see that it's something that we'd want to add to the code itself.
Perhaps I'm missing something but I think we need to say wontfix here.

Hi Carlton,
I did my best to succinctly describe the use case and reasoning that expire_date needs to be accessed. I will try again with more detail.

The SessionStore has the get_expiry_date() method, however the database session does not reference or store the expire_date value from the database when retrieving a session, so the value returned is always the current time + get_expiry_age().

In [1]: from django.contrib.sessions.backends.db import SessionStore
In [2]: from django.contrib.sessions.models import Session
In [3]: session_key = '10bfke86cx0r2r6xkca9d07skl55amwh'
In [4]: session = SessionStore(session_key)
In [5]: session.get_expiry_date()
Out[5]: datetime.datetime(2021, 1, 11, 13, 8, 29, 360181)
In [6]: session_db = Session.objects.get(pk=session_key)
In [7]: session_db.expire_date
Out[7]: datetime.datetime(2021, 1, 6, 19, 51, 12)

The documentation notes that get_expiry_date()
https://docs.djangoproject.com/en/3.1/topics/http/sessions/#django.contrib.sessions.backends.base.SessionBase.get_expiry_date

Returns the date this session will expire. For sessions with no custom expiration (or those set to expire at browser close), this will equal the date SESSION_COOKIE_AGE seconds from now.

Based on the documentation, I thought get_expiry_date() should return 2021-01-06 19:51:12 and perhaps my understanding is incorrect and these values returned in my code example above are correct.

This session will expire at 2021-01-06 19:51:12 UTC because when the database session store queries for a session it ensures expire_date is greater than the current time. However, based on my understanding of get_expiry_date(), it indicates the session will expire at 2021-01-11 13:08:29.360181 UTC.

With database sessions, when the session is modified it will update the expire_date. If the session is not modified, the session will expire at expire_date. Currently, we have no way to access expire_date without querying the session database model.

The use case for having access to expire_date on the SessionStore object is to be able to retrieve the time the database session will expire without an additional query so that we can implement sliding expiration sessions more efficiently.

Please let me know your thoughts?

comment:14 by Carlton Gibson, 4 years ago

HI Nate, yes, thanks.

In the end I think that's something you should add to a custom backend. There's no reason you can't store the value in load, as you suggest, but in 15 years it's not been needed and adding complexity to the API here isn't something I think we should do for such a niche case.

in reply to:  14 comment:15 by ahmadkhalili, 3 years ago

Resolution: wontfix
Status: closednew

Replying to Carlton Gibson:

HI Nate, yes, thanks.

In the end I think that's something you should add to a custom backend. There's no reason you can't store the value in load, as you suggest, but in 15 years it's not been needed and adding complexity to the API here isn't something I think we should do for such a niche case.

I have this problem too, i wanted create new ticket but when i found this ticket it's made me happy. suppose we use database backend session, my experience to face this problem:

from datetime import datetime
from django.contrib.sessions.backends.db import SessionStore

my_session = SessionStore()
# there is no session key specified, means we want create new db session probably
session.get_expiry_date()
# get_expiry_date() func tell us new created sessions expiration time(it is now in django 4, 14 day), that is logical and expected

# we have specific session key means we want work on our db session and don't want new
my_session = SessionStore('some_session_key')

# get_expiry_date() always return new created sessions expiration time (14 days after .now())
if my_session.get_expiry_date() < datetime.now():     # so this is fail
    my_session.create()
    ...

Here in this example django force us using my_session._get_session_from_db().expire_date instead my_session.get_expiry_date(), but one simple question: why we should do additional work when we determined to django 'we have specific db session to work on it' ?

All i want say in one sentence: get_expiry_date() should return expire date of current session but when we have not eny session yet(like SessionStore()) its logical get_expiry_date() returns new created sessions expiration time.

Version 0, edited 3 years ago by ahmadkhalili (next)

comment:16 by Mariusz Felisiak, 3 years ago

Resolution: wontfix
Status: newclosed

I appreciate you'd like to reopen the ticket, but please follow the triaging guidelines with regards to wontfix tickets and take this to DevelopersMailingList.

Note: See TracTickets for help on using tickets.
Back to Top