Django

Code

Ticket #5549 (new)

Opened 10 months ago

Last modified 4 months ago

Remove duplicate code in SessionBase, SessionManager and Session

Reported by: adrian Assigned to: heracek
Milestone: Component: django.contrib.sessions
Version: SVN Keywords: sprintdec01
Cc: Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

In #5548, leosoto pointed out that these two methods are almost identical:

django.contrib.sessions.models.SessionManager.get_new_session_key

django.contrib.sessions.backends.base.SessionBase._get_new_session_key

These should be refactored to remove the duplication.

Attachments

session-refactoring.diff (5.6 kB) - added by heracek on 12/01/07 09:39:37.
Patch removes duplicity in code and corrects session documentation
ticket_5549__rev_6814.diff (4.1 kB) - added by __hawkeye__ on 12/01/07 16:08:30.
Goes beyond #6081.

Change History

09/19/07 21:26:42 changed by adrian

  • needs_better_patch changed.
  • stage changed from Unreviewed to Accepted.
  • component changed from Core framework to django.contrib.sessions.
  • needs_tests changed.
  • needs_docs changed.

12/01/07 06:14:43 changed by heracek

  • owner changed from nobody to heracek.

12/01/07 09:39:37 changed by heracek

  • attachment session-refactoring.diff added.

Patch removes duplicity in code and corrects session documentation

12/01/07 09:41:13 changed by heracek

  • keywords set to sprintdec01.
  • has_patch set to 1.
  • summary changed from Remove duplicate code in SessionBase andSessionManager to Remove duplicate code in SessionBase, SessionManager and Session.

12/01/07 16:07:33 changed by __hawkeye__

Accidentally missed this and filed #6081. This is more comprehensive, but #6081 has already been checked in.

12/01/07 16:08:30 changed by __hawkeye__

  • attachment ticket_5549__rev_6814.diff added.

Goes beyond #6081.

02/26/08 10:53:51 changed by anonymous

Also, there should be clear instructions how to clear all session data (as opposed to deleting individual keys).

request.session.clear() seems to do it, it should be documented (I assume this is correct looking at the code -- looks like the the corresponding row will be deleted from session table if session dict is empty) or an explicit, documented method for clearing the session should be added.

Perhaps a note like the following should be added:

Use request.session.clear() if you need to clear all session data. The corresponding session row will be automatically removed from the session table when session dictionary is empty.

Test cases should be added too, in the lines of the following:

>>> request = HttpRequest
>>> # attach session to HttpRequest
>>> request.session["foo"] = "bar"
>>> request.session.clear()
>>> request.session["foo"]
Traceback (most recent call last):
    ...
KeyError: 'foo'

02/26/08 11:02:09 changed by anonymous

See e.g. http://beaker.groovie.org/beaker/browser/beaker/session.py#L174 for session invalidation that really should be present in Django.

02/26/08 16:21:48 changed by mtredinnick

Anonymous, please don't hijack tickets for issues of your own. Your item has nothing to do with this ticket. If you want extra functionality added, read contributing.txt for how to propose it (discussion on django-dev and then maybe a ticket).


Add/Change #5549 (Remove duplicate code in SessionBase, SessionManager and Session)




Change Properties
Action