Mark abstract base classes as such. (#6930)

* Mark abstract base classes as such.

* Remove an unneeded method.

* Fix exporter test coverage.
This commit is contained in:
Andrey Rakhmatullin 2025-06-30 15:14:34 +05:00 committed by GitHub
parent 8b3c3ea4ae
commit 3843091c5f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 84 additions and 29 deletions

View File

@ -15,6 +15,35 @@ Backward-incompatible changes
``True`` when running Scrapy via :ref:`its command-line tool
<topics-commands-crawlerprocess>` to avoid a reactor mismatch exception.
- The classes listed below are now :term:`abstract base classes <abstract
base class>`. They cannot be instantiated directly and their subclasses
need to override the abstract methods listed below to be able to be
instantiated. If you previously instantiated these classes directly, you
will now need to subclass them and provide trivial (e.g. empty)
implementations for the abstract methods.
- :class:`scrapy.commands.ScrapyCommand`
- :meth:`~scrapy.commands.ScrapyCommand.run`
- :meth:`~scrapy.commands.ScrapyCommand.short_desc`
- :class:`scrapy.exporters.BaseItemExporter`
- :meth:`~scrapy.exporters.BaseItemExporter.export_item`
- :class:`scrapy.extensions.feedexport.BlockingFeedStorage`
- :meth:`~scrapy.extensions.feedexport.BlockingFeedStorage._store_in_thread`
- :class:`scrapy.middleware.MiddlewareManager`
- :meth:`~scrapy.middleware.MiddlewareManager._get_mwlist_from_settings`
- :class:`scrapy.spidermiddlewares.referer.ReferrerPolicy`
- :meth:`~scrapy.spidermiddlewares.referer.ReferrerPolicy.referrer`
.. _release-2.13.2:

View File

@ -354,7 +354,7 @@ Default: ``'scrapy.spidermiddlewares.referer.DefaultReferrerPolicy'``
Acceptable values for REFERRER_POLICY
*************************************
- either a path to a ``scrapy.spidermiddlewares.referer.ReferrerPolicy``
- either a path to a :class:`scrapy.spidermiddlewares.referer.ReferrerPolicy`
subclass — a custom policy or one of the built-in ones (see classes below),
- or one or more comma-separated standard W3C-defined string values,
- or the special ``"scrapy-default"``.
@ -373,6 +373,8 @@ String value Class name (as a string)
`"unsafe-url"`_ :class:`scrapy.spidermiddlewares.referer.UnsafeUrlPolicy`
======================================= ========================================================================
.. autoclass:: ReferrerPolicy
.. autoclass:: DefaultReferrerPolicy
.. warning::
Scrapy's default referrer policy — just like `"no-referrer-when-downgrade"`_,

View File

@ -7,6 +7,7 @@ from __future__ import annotations
import argparse
import builtins
import os
from abc import ABC, abstractmethod
from pathlib import Path
from typing import TYPE_CHECKING, Any
@ -22,7 +23,7 @@ if TYPE_CHECKING:
from scrapy.settings import Settings
class ScrapyCommand:
class ScrapyCommand(ABC):
requires_project: bool = False
requires_crawler_process: bool = True
crawler_process: CrawlerProcessBase | None = None # set in scrapy.cmdline
@ -46,6 +47,7 @@ class ScrapyCommand:
"""
return ""
@abstractmethod
def short_desc(self) -> str:
"""
A short description of the command
@ -128,6 +130,7 @@ class ScrapyCommand:
if opts.pdb:
failure.startDebugMode()
@abstractmethod
def run(self, args: list[str], opts: argparse.Namespace) -> None:
"""
Entry point for running commands

View File

@ -8,6 +8,7 @@ import csv
import marshal
import pickle
import pprint
from abc import ABC, abstractmethod
from collections.abc import Callable, Iterable, Mapping
from io import BytesIO, TextIOWrapper
from typing import TYPE_CHECKING, Any
@ -35,7 +36,7 @@ __all__ = [
]
class BaseItemExporter:
class BaseItemExporter(ABC):
def __init__(self, *, dont_fail: bool = False, **kwargs: Any):
self._kwargs: dict[str, Any] = kwargs
self._configure(kwargs, dont_fail=dont_fail)
@ -54,6 +55,7 @@ class BaseItemExporter:
if not dont_fail and options:
raise TypeError(f"Unexpected options: {', '.join(options.keys())}")
@abstractmethod
def export_item(self, item: Any) -> None:
raise NotImplementedError
@ -63,10 +65,10 @@ class BaseItemExporter:
serializer: Callable[[Any], Any] = field.get("serializer", lambda x: x)
return serializer(value)
def start_exporting(self) -> None:
def start_exporting(self) -> None: # noqa: B027
pass
def finish_exporting(self) -> None:
def finish_exporting(self) -> None: # noqa: B027
pass
def _get_serialized_fields(

View File

@ -11,6 +11,7 @@ import logging
import re
import sys
import warnings
from abc import ABC, abstractmethod
from collections.abc import Callable
from datetime import datetime, timezone
from pathlib import Path, PureWindowsPath
@ -140,7 +141,7 @@ class FeedStorageProtocol(Protocol):
@implementer(IFeedStorage)
class BlockingFeedStorage:
class BlockingFeedStorage(ABC):
def open(self, spider: Spider) -> IO[bytes]:
path = spider.crawler.settings["FEED_TEMPDIR"]
if path and not Path(path).is_dir():
@ -151,6 +152,7 @@ class BlockingFeedStorage:
def store(self, file: IO[bytes]) -> Deferred[None] | None:
return deferToThread(self._store_in_thread, file)
@abstractmethod
def _store_in_thread(self, file: IO[bytes]) -> None:
raise NotImplementedError

View File

@ -3,6 +3,7 @@ from __future__ import annotations
import logging
import pprint
import warnings
from abc import ABC, abstractmethod
from collections import defaultdict, deque
from typing import TYPE_CHECKING, Any, TypeVar, cast
@ -32,10 +33,10 @@ _T = TypeVar("_T")
_T2 = TypeVar("_T2")
class MiddlewareManager:
class MiddlewareManager(ABC):
"""Base class for implementing middleware managers"""
component_name = "foo middleware"
component_name: str
def __init__(self, *middlewares: Any) -> None:
self.middlewares = middlewares
@ -48,6 +49,7 @@ class MiddlewareManager:
self._add_middleware(mw)
@classmethod
@abstractmethod
def _get_mwlist_from_settings(cls, settings: Settings) -> list[Any]:
raise NotImplementedError

View File

@ -6,6 +6,7 @@ originated it.
from __future__ import annotations
import warnings
from abc import ABC, abstractmethod
from typing import TYPE_CHECKING, cast
from urllib.parse import urlparse
@ -45,10 +46,13 @@ POLICY_UNSAFE_URL = "unsafe-url"
POLICY_SCRAPY_DEFAULT = "scrapy-default"
class ReferrerPolicy:
class ReferrerPolicy(ABC):
"""Abstract base class for referrer policies."""
NOREFERRER_SCHEMES: tuple[str, ...] = LOCAL_SCHEMES
name: str
@abstractmethod
def referrer(self, response_url: str, request_url: str) -> str | None:
raise NotImplementedError

View File

@ -25,9 +25,17 @@ if TYPE_CHECKING:
import os
class EmptyCommand(ScrapyCommand):
def short_desc(self) -> str:
return ""
def run(self, args: list[str], opts: argparse.Namespace) -> None:
pass
class TestCommandSettings:
def setup_method(self):
self.command = ScrapyCommand()
self.command = EmptyCommand()
self.command.settings = Settings()
self.parser = argparse.ArgumentParser(
formatter_class=ScrapyHelpFormatter, conflict_handler="resolve"

View File

@ -4,6 +4,7 @@ import marshal
import pickle
import re
import tempfile
from abc import ABC, abstractmethod
from datetime import datetime
from io import BytesIO
from typing import Any
@ -53,7 +54,7 @@ class CustomFieldDataclass:
age: int = dataclasses.field(metadata={"serializer": custom_serializer})
class TestBaseItemExporter:
class TestBaseItemExporter(ABC):
item_class: type = MyItem
custom_field_item_class: type = CustomFieldItem
@ -62,10 +63,11 @@ class TestBaseItemExporter:
self.output = BytesIO()
self.ie = self._get_exporter()
def _get_exporter(self, **kwargs):
return BaseItemExporter(**kwargs)
@abstractmethod
def _get_exporter(self, **kwargs) -> BaseItemExporter:
raise NotImplementedError
def _check_output(self):
def _check_output(self): # noqa: B027
pass
def _assert_expected_item(self, exported_dict):
@ -83,11 +85,7 @@ class TestBaseItemExporter:
def assertItemExportWorks(self, item):
self.ie.start_exporting()
try:
self.ie.export_item(item)
except NotImplementedError:
if self.ie.__class__ is not BaseItemExporter:
raise
self.ie.export_item(item)
self.ie.finish_exporting()
# Delete the item exporter object, so that if it causes the output
# file handle to be closed, which should not be the case, follow-up
@ -132,11 +130,6 @@ class TestBaseItemExporter:
assert ie.serialize_field(a.get_field_meta("age"), "age", a["age"]) == "24"
class TestBaseItemExporterDataclass(TestBaseItemExporter):
item_class = MyDataClass
custom_field_item_class = CustomFieldDataclass
class TestPythonItemExporter(TestBaseItemExporter):
def _get_exporter(self, **kwargs):
return PythonItemExporter(**kwargs)
@ -670,6 +663,9 @@ class TestCustomExporterItem:
return str(int(value) + 1)
return super().serialize_field(field, name, value)
def export_item(self, item: Any) -> None:
pass
i = self.item_class(name="John", age="22")
a = ItemAdapter(i)
ie = CustomItemExporter()

View File

@ -18,7 +18,7 @@ from io import BytesIO
from logging import getLogger
from pathlib import Path
from string import ascii_letters, digits
from typing import TYPE_CHECKING, Any
from typing import IO, TYPE_CHECKING, Any
from unittest import mock
from urllib.parse import quote, urljoin
from urllib.request import pathname2url
@ -236,6 +236,11 @@ class TestFTPFeedStorage(unittest.TestCase):
assert st.password == string.punctuation
class MyBlockingFeedStorage(BlockingFeedStorage):
def _store_in_thread(self, file: IO[bytes]) -> None:
return
class TestBlockingFeedStorage:
def get_test_spider(self, settings=None):
class TestSpider(scrapy.Spider):
@ -245,14 +250,14 @@ class TestBlockingFeedStorage:
return TestSpider.from_crawler(crawler)
def test_default_temp_dir(self):
b = BlockingFeedStorage()
b = MyBlockingFeedStorage()
storage_file = b.open(self.get_test_spider())
storage_dir = Path(storage_file.name).parent
assert str(storage_dir) == tempfile.gettempdir()
def test_temp_file(self, tmp_path):
b = BlockingFeedStorage()
b = MyBlockingFeedStorage()
spider = self.get_test_spider({"FEED_TEMPDIR": str(tmp_path)})
storage_file = b.open(spider)
@ -260,7 +265,7 @@ class TestBlockingFeedStorage:
assert storage_dir == tmp_path
def test_invalid_folder(self, tmp_path):
b = BlockingFeedStorage()
b = MyBlockingFeedStorage()
invalid_path = tmp_path / "invalid_path"
spider = self.get_test_spider({"FEED_TEMPDIR": str(invalid_path)})

View File

@ -39,6 +39,8 @@ class MOff:
class MyMiddlewareManager(MiddlewareManager):
component_name = "my"
@classmethod
def _get_mwlist_from_settings(cls, settings):
return [M1, MOff, M3]
@ -65,7 +67,7 @@ class TestMiddlewareManager:
def test_enabled(self):
m1, m2, m3 = M1(), M2(), M3()
mwman = MiddlewareManager(m1, m2, m3)
mwman = MyMiddlewareManager(m1, m2, m3)
assert mwman.middlewares == (m1, m2, m3)
def test_enabled_from_settings(self):