From 71b06f5bb30cc1528caf1f67949194dd1f2a1da5 Mon Sep 17 00:00:00 2001 From: Thibaut Decombe <68703331+UnknownPlatypus@users.noreply.github.com> Date: Sat, 22 Oct 2022 19:58:59 +0200 Subject: [PATCH] Generic sitemap (#1198) * Add test for both issue cases * Use generic type and add new django 4.1 `get_latest_lastmod` method * Add Sitemap to monkeypatch * Update test case to use generic + add failing case * Test GenericSitemap too --- django-stubs/contrib/sitemaps/__init__.pyi | 24 +++--- django_stubs_ext/django_stubs_ext/patch.py | 2 + .../contrib/sitemaps/test_generic_sitemap.yml | 80 +++++++++++++++++++ 3 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 tests/typecheck/contrib/sitemaps/test_generic_sitemap.yml diff --git a/django-stubs/contrib/sitemaps/__init__.pyi b/django-stubs/contrib/sitemaps/__init__.pyi index f14ae23..fe59b4a 100644 --- a/django-stubs/contrib/sitemaps/__init__.pyi +++ b/django-stubs/contrib/sitemaps/__init__.pyi @@ -1,5 +1,5 @@ from datetime import datetime -from typing import Any, Dict, List, Mapping, Optional, Sequence, Union +from typing import Any, Dict, Generic, Iterable, List, Mapping, Optional, Sequence, TypeVar, Union from django.contrib.sites.models import Site from django.contrib.sites.requests import RequestSite @@ -13,33 +13,39 @@ class SitemapNotFound(Exception): ... def ping_google(sitemap_url: Optional[str] = ..., ping_url: str = ..., sitemap_uses_https: bool = ...) -> None: ... -class Sitemap: +_ItemT = TypeVar("_ItemT") + +class Sitemap(Generic[_ItemT]): limit: int = ... protocol: Optional[str] = ... i18n: bool = ... languages: Optional[Sequence[str]] = ... alternates: bool = ... x_default: bool = ... - def items(self) -> Union[Sequence[Any], QuerySet[Any]]: ... - def location(self, item: Model) -> str: ... + def items(self) -> Iterable[_ItemT]: ... + def location(self, item: _ItemT) -> str: ... @property def paginator(self) -> Paginator: ... def get_urls( self, page: Union[int, str] = ..., site: Optional[Union[Site, RequestSite]] = ..., protocol: Optional[str] = ... ) -> List[Dict[str, Any]]: ... + def get_latest_lastmod(self) -> Optional[datetime]: ... -class GenericSitemap(Sitemap): +_ModelT = TypeVar("_ModelT", bound=Model) + +class GenericSitemap(Sitemap[_ModelT]): priority: Optional[float] = ... changefreq: Optional[str] = ... - queryset: QuerySet[Model] = ... + queryset: QuerySet[_ModelT] = ... date_field: Optional[str] = ... protocol: Optional[str] = ... def __init__( self, - info_dict: Mapping[str, Union[datetime, QuerySet[Model], str]], + info_dict: Mapping[str, Union[datetime, QuerySet[_ModelT], str]], priority: Optional[float] = ..., changefreq: Optional[str] = ..., protocol: Optional[str] = ..., ) -> None: ... - def lastmod(self, item: Model) -> Optional[datetime]: ... - def items(self) -> QuerySet[Model]: ... + def items(self) -> QuerySet[_ModelT]: ... + def lastmod(self, item: _ModelT) -> Optional[datetime]: ... + def get_latest_lastmod(self) -> Optional[datetime]: ... diff --git a/django_stubs_ext/django_stubs_ext/patch.py b/django_stubs_ext/django_stubs_ext/patch.py index 6d2125a..eb32b12 100644 --- a/django_stubs_ext/django_stubs_ext/patch.py +++ b/django_stubs_ext/django_stubs_ext/patch.py @@ -4,6 +4,7 @@ from typing import Any, Generic, Iterable, List, Optional, Tuple, Type, TypeVar from django import VERSION as VERSION from django.contrib.admin import ModelAdmin from django.contrib.admin.options import BaseModelAdmin +from django.contrib.sitemaps import Sitemap from django.contrib.syndication.views import Feed from django.core.files.utils import FileProxyMixin from django.core.paginator import Paginator @@ -60,6 +61,7 @@ _need_generic: List[MPGeneric[Any]] = [ MPGeneric(BaseModelForm), MPGeneric(BaseModelFormSet), MPGeneric(Feed), + MPGeneric(Sitemap), MPGeneric(FileProxyMixin), MPGeneric(Lookup), # These types do have native `__class_getitem__` method since django 3.1: diff --git a/tests/typecheck/contrib/sitemaps/test_generic_sitemap.yml b/tests/typecheck/contrib/sitemaps/test_generic_sitemap.yml new file mode 100644 index 0000000..7048e6e --- /dev/null +++ b/tests/typecheck/contrib/sitemaps/test_generic_sitemap.yml @@ -0,0 +1,80 @@ +- case: test_items_custom_model + main: | + from django.contrib.sitemaps import GenericSitemap, Sitemap + from django.contrib.sitemaps.views import sitemap + from django.db.models import QuerySet + from django.urls import path, reverse + from myapp.models import Offer + + class OfferSitemap(Sitemap[Offer]): + priority = 1 + changefreq = "always" + + def items(self) -> QuerySet[Offer]: + return Offer.objects.all() + + def location(self, item: Offer) -> str: + return reverse( + "myapp:detail-offer", + kwargs={ + "provider_name": item.provider, + "offer_name": item.trc, # E: "Offer" has no attribute "trc" + }, + ) + + class WrongOfferSitemap(Sitemap[Offer]): + def items(self) -> str: + return "Yes" + def location(self, item: str) -> int: + return 1 + + info_dict = {"queryset": Offer.objects.all()} + broken_info_dict = {"queryset": [1, 2]} + + urlpatterns = [ + path( + 'sitemap.xml', sitemap, + {'sitemaps': {'offers': GenericSitemap[Offer](info_dict, priority=0.6)}}, + name='django.contrib.sitemaps.views.sitemap' + ), + path( + 'broken_sitemap.xml', sitemap, + {'sitemaps': {'offers': GenericSitemap[Offer](broken_info_dict, priority=0.6)}}, + name='django.contrib.sitemaps.views.broken_sitemap' + ), + ] + out: | + main:24: error: Return type "str" of "items" incompatible with return type "Iterable[Offer]" in supertype "Sitemap" + main:26: error: Argument 1 of "location" is incompatible with supertype "Sitemap"; supertype defines the argument type as "Offer" + main:26: note: This violates the Liskov substitution principle + main:26: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides + main:26: error: Return type "int" of "location" incompatible with return type "str" in supertype "Sitemap" + main:40: error: Argument 1 to "GenericSitemap" has incompatible type "Dict[str, List[int]]"; expected "Mapping[str, Union[datetime, _QuerySet[Offer, Offer], str]]" + + installed_apps: + - myapp + files: + - path: myapp/__init__.py + - path: myapp/models.py + content: | + from django.db import models + + class Offer(models.Model): + provider = models.ForeignKey("self", on_delete=models.CASCADE) + url_name = models.CharField() + +- case: test_items_string_sequence + main: | + from django.contrib.sitemaps import Sitemap + from typing import List + from django.urls import reverse + + class StaticViewSitemap(Sitemap[str]): + priority = 1 + changefreq = "always" + + def items(self) -> List[str]: + return ["home", "about", "contact", "recommendations", "privacy-policy", "blog"] + + def location(self, item: str) -> str: + return reverse(item)