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.

Sign up for GitHub

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 SessionBase methods, change SessionStore.get_model_class() return #1517

Merged
merged 3 commits into from
Jun 2, 2023
Merged

Add SessionBase methods, change SessionStore.get_model_class() return #1517

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3c241c7
Improve typing in `contrib.session.backends`
realsuayip May 29, 2023
2ea36d2
Remove new underscore methods per PR review
intgr Jun 2, 2023
35641ce
Add back keys, values, items methods
intgr Jun 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions django-stubs/contrib/sessions/backends/base.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,19 @@ class SessionBase(dict[str, Any]):
modified: bool
serializer: Any
def __init__(self, session_key: str | None = ...) -> None: ...
@property
def key_salt(self) -> str: ...
def set_test_cookie(self) -> None: ...
def test_cookie_worked(self) -> bool: ...
def delete_test_cookie(self) -> None: ...
def encode(self, session_dict: dict[str, Any]) -> str: ...
def decode(self, session_data: bytes | str) -> dict[str, Any]: ...
def has_key(self, key: Any) -> Any: ...
def keys(self) -> Any: ...
def values(self) -> Any: ...
def items(self) -> Any: ...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove these 3 methods? Looks like they're present still: https://github.com/django/django/blob/main/django/contrib/sessions/backends/base.py#L121C1-L129

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added them back for now, so I can merge this for the 4.2.1 release.

If there was a good reason to remove them, please let me know.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In stubs, SessionBase is defined as subclass of dict:

django-stubs/django-stubs/contrib/sessions/backends/base.pyi

Line 9 in 21e541f

class SessionBase(dict[str, Any]):

not in Django though:

https://github.com/django/django/blob/27fed08e5ff597c813de25b6e89dc300a1f4231b/django/contrib/sessions/backends/base.py#LL33C9-L33C9

Subclassing from dict should cover for keys, values and items (and with proper return values, not just Any)

I'm guessing the subclassing is probably done so as not type all the dict related dunder methods, which are present in Django to ensure dict-like behavior.

def has_key(self, key: Any) -> bool: ...
def clear(self) -> None: ...
def is_empty(self) -> bool: ...
def _get_new_session_key(self) -> str: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we usually don't type protected methods, why do you need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_get_or_create_session_key may needed to be used when overriding SessionStore. For example:

https://github.com/QueraTeam/django-qsessions/blob/ecfd7327ebe32483746704b59fca72f05be07284/qsessions/backends/db.py#LL43C14-L43C14

I don't have any use cases for the others, typed them just in case. I could remove those.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, remove them :)
We can always bring them back if there are real use-cases.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed a change to remove the underscore methods.

def _get_or_create_session_key(self) -> str: ...
def _validate_session_key(self) -> bool: ...
def _get_session_key(self) -> str | None: ...
def _set_session_key(self, value: str | None) -> None: ...
@property
Expand All @@ -32,6 +34,8 @@ class SessionBase(dict[str, Any]):
def _session_key(self) -> str | None: ...
@_session_key.setter
def _session_key(self, value: str | None) -> None: ...
def _get_session(self, no_load: bool = ...) -> dict[str, Any]: ...
def get_session_cookie_age(self) -> int: ...
def get_expiry_age(self, **kwargs: Any) -> int: ...
def get_expiry_date(self, **kwargs: Any) -> datetime: ...
def set_expiry(self, value: datetime | int | None) -> None: ...
Expand Down
3 changes: 1 addition & 2 deletions django-stubs/contrib/sessions/backends/db.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
from django.contrib.sessions.backends.base import SessionBase
from django.contrib.sessions.base_session import AbstractBaseSession
from django.contrib.sessions.models import Session
from django.db.models.base import Model

class SessionStore(SessionBase):
def __init__(self, session_key: str | None = ...) -> None: ...
@classmethod
def get_model_class(cls) -> type[Session]: ...
def get_model_class(cls) -> type[AbstractBaseSession]: ...
@property
def model(self) -> type[AbstractBaseSession]: ...
def create_model_instance(self, data: dict[str, Model]) -> AbstractBaseSession: ...