Handle item overrides

scrapy_poet/commands.py

@@ -1,7 +1,7 @@
 import datetime
 import logging
 from pathlib import Path
-from typing import Optional, Type
+from typing import Dict, Optional, Type
 
 import andi
 import scrapy
@@ -34,10 +34,14 @@
 class SavingInjector(Injector):
     @inlineCallbacks
     def build_instances_from_providers(
-        self, request: Request, response: Response, plan: andi.Plan
+        self,
+        request: Request,
+        response: Response,
+        plan: andi.Plan,
+        prev_instances: Optional[Dict] = None,
     ):
         instances = yield super().build_instances_from_providers(
-            request, response, plan
+            request, response, plan, prev_instances
         )
         if request.meta.get("savefixture", False):
             saved_dependencies.extend(instances.values())

scrapy_poet/downloadermiddlewares.py

@@ -34,7 +34,7 @@
     PageParamsProvider: 700,
     RequestUrlProvider: 800,
     ResponseUrlProvider: 900,
-    ItemProvider: 1000,
+    ItemProvider: 2000,
 }
 
 InjectionMiddlewareTV = TypeVar("InjectionMiddlewareTV", bound="InjectionMiddleware")

scrapy_poet/injection.py

@@ -151,11 +151,20 @@ def build_plan(self, request: Request) -> andi.Plan:
         )
 
     @inlineCallbacks
-    def build_instances(self, request: Request, response: Response, plan: andi.Plan):
+    def build_instances(
+        self,
+        request: Request,
+        response: Response,
+        plan: andi.Plan,
+        prev_instances: Optional[Dict] = None,
+    ):
         """Build the instances dict from a plan including external dependencies."""
         # First we build the external dependencies using the providers
         instances = yield from self.build_instances_from_providers(
-            request, response, plan
+            request,
+            response,
+            plan,
+            prev_instances,
         )
         # All the remaining dependencies are internal so they can be built just
         # following the andi plan.
@@ -169,10 +178,14 @@ def build_instances(self, request: Request, response: Response, plan: andi.Plan)
 
     @inlineCallbacks
     def build_instances_from_providers(
-        self, request: Request, response: Response, plan: andi.Plan
+        self,
+        request: Request,
+        response: Response,
+        plan: andi.Plan,
+        prev_instances: Optional[Dict] = None,
     ):
         """Build dependencies handled by registered providers"""
-        instances: Dict[Callable, Any] = {}
+        instances: Dict[Callable, Any] = prev_instances or {}
         scrapy_provided_dependencies = self.available_dependencies_for_providers(
             request, response
         )
@@ -182,10 +195,22 @@ def build_instances_from_providers(
             provided_classes = {
                 cls for cls in dependencies_set if provider.is_provided(cls)
             }
-            provided_classes -= instances.keys()  # ignore already provided types
+
+            # ignore already provided types if provider doesn't need to use them
+            if not provider.allow_prev_instances:
+                provided_classes -= instances.keys()
+
             if not provided_classes:
                 continue
 
+            # If dependency instances were already made by previously invoked
+            # providers, don't try to build them again since it may result in
+            # incorrect values (e.g. PO modifying an item > 2 times).
+            required_deps = set(plan.dependencies[-1][1].values())
+            built_deps = set(instances.keys())
+            if required_deps and required_deps == built_deps:
+                continue
+
             objs, fingerprint = [], None
             cache_hit = False
             if self.cache:
@@ -221,6 +246,8 @@ def build_instances_from_providers(
                     externally_provided=scrapy_provided_dependencies,
                     full_final_kwargs=False,
                 ).final_kwargs(scrapy_provided_dependencies)
+                if provider.allow_prev_instances:
+                    kwargs.update({"prev_instances": instances})
                 try:
                     # Invoke the provider to get the data
                     objs = yield maybeDeferred_coro(
@@ -414,6 +441,7 @@ class MySpider(Spider):
     spider = MySpider()
     spider.settings = settings
     crawler.spider = spider
+    crawler.stats = load_object(crawler.settings["STATS_CLASS"])(crawler)
     if not registry:
         registry = create_registry_instance(RulesRegistry, crawler)
     return Injector(crawler, registry=registry)

scrapy_poet/page_input_providers.py

@@ -106,6 +106,12 @@ def __call__(self, to_provide, response: Response):
     provided_classes: Union[Set[Callable], Callable[[Callable], bool]]
     name: ClassVar[str] = ""  # It must be a unique name. Used by the cache mechanism
 
+    # If set to True, the Injector will not skip the Provider when the dependency has
+    # been built. Instead, the Injector will pass the previously built instances (by
+    # the other providers) to the Provider. The Provider can then choose to modify
+    # these previous instances before returning them to the Injector.
+    allow_prev_instances: bool = False
+
     def is_provided(self, type_: Callable) -> bool:
         """
         Return ``True`` if the given type is provided by this provider based
@@ -230,6 +236,8 @@ class ItemProvider(PageObjectInputProvider):
         "trying to resolve this plan: {plan}"
     )
 
+    allow_prev_instances: bool = True
+
     def __init__(self, injector):
         super().__init__(injector)
         self.registry = self.injector.registry
@@ -280,11 +288,11 @@ async def __call__(
         to_provide: Set[Callable],
         request: Request,
         response: Response,
+        prev_instances: Dict,
     ) -> List[Any]:
         results = []
         for cls in to_provide:
-            item = self.get_from_cache(request, cls)
-            if item:
+            if item := self.get_from_cache(request, cls):
                 results.append(item)
                 continue
 
@@ -308,7 +316,9 @@ async def __call__(
 
             try:
                 deferred_or_future = maybe_deferred_to_future(
-                    self.injector.build_instances(request, response, plan)
+                    self.injector.build_instances(
+                        request, response, plan, prev_instances
+                    )
                 )
                 # RecursionError NOT raised when ``AsyncioSelectorReactor`` is used.
                 # Could be related: https://github.com/python/cpython/issues/93837

tests/test_response_required_logic.py

@@ -8,6 +8,7 @@
 from scrapy.crawler import Crawler
 from scrapy.http import HtmlResponse, Request, TextResponse
 from scrapy.settings import Settings
+from scrapy.utils.misc import load_object
 from web_poet import ItemPage, WebPage
 
 from scrapy_poet import DummyResponse, callback_for
@@ -32,18 +33,15 @@
 
 @attr.s(auto_attribs=True)
 class DummyProductResponse:
-
     data: Dict[str, Any]
 
 
 @attr.s(auto_attribs=True)
 class FakeProductResponse:
-
     data: Dict[str, Any]
 
 
 class DummyProductProvider(PageObjectInputProvider):
-
     provided_classes = {DummyProductResponse}
 
     def __call__(self, to_provide, request: scrapy.Request):
@@ -57,7 +55,6 @@ def __call__(self, to_provide, request: scrapy.Request):
 
 
 class FakeProductProvider(PageObjectInputProvider):
-
     provided_classes = {FakeProductResponse}
 
     def __call__(self, to_provide):
@@ -71,7 +68,6 @@ def __call__(self, to_provide):
 
 
 class TextProductProvider(HttpResponseProvider):
-
     # This is wrong. You should not annotate provider dependencies with classes
     # like TextResponse or HtmlResponse, you should use Response instead.
     def __call__(self, to_provide, response: TextResponse):
@@ -85,7 +81,6 @@ def __call__(self, to_provide, response: str):
 
 @attr.s(auto_attribs=True)
 class DummyProductPage(ItemPage):
-
     response: DummyProductResponse
 
     @property
@@ -99,7 +94,6 @@ def to_item(self):
 
 @attr.s(auto_attribs=True)
 class FakeProductPage(ItemPage):
-
     response: FakeProductResponse
 
     @property
@@ -117,7 +111,6 @@ def to_item(self):
 
 
 class MySpider(scrapy.Spider):
-
     name = "foo"
     custom_settings = {
         "SCRAPY_POET_PROVIDERS": {
@@ -360,6 +353,7 @@ def test_is_response_going_to_be_used():
     crawler = Crawler(MySpider)
     spider = MySpider()
     crawler.spider = spider
+    crawler.stats = load_object(crawler.settings["STATS_CLASS"])(crawler)
 
     def response(request):
         return HtmlResponse(request.url, request=request, body=b"<html></html>")