From 989ead4a1200040f277e6afdad77f32ee3ca6bcd Mon Sep 17 00:00:00 2001 From: Thibault Coudray <169165300+tcoudray-pass@users.noreply.github.com> Date: Thu, 9 Jan 2025 13:29:15 +0100 Subject: [PATCH] (PC-33516)[API] feat: remove `ProviderAPI` --- api/src/pcapi/core/providers/models.py | 22 --- .../repository/stock_provider/__init__.py | 0 .../repository/stock_provider/provider_api.py | 143 ------------------ .../pcapi/local_providers/provider_manager.py | 16 +- api/tests/core/providers/test_api.py | 5 - .../repository/stock_provider/__init__.py | 0 .../stock_provider/provider_api_test.py | 130 ---------------- 7 files changed, 2 insertions(+), 314 deletions(-) delete mode 100644 api/src/pcapi/infrastructure/repository/stock_provider/__init__.py delete mode 100644 api/src/pcapi/infrastructure/repository/stock_provider/provider_api.py delete mode 100644 api/tests/infrastructure/repository/stock_provider/__init__.py delete mode 100644 api/tests/infrastructure/repository/stock_provider/provider_api_test.py diff --git a/api/src/pcapi/core/providers/models.py b/api/src/pcapi/core/providers/models.py index 19be8b3cc90..98ae5408772 100644 --- a/api/src/pcapi/core/providers/models.py +++ b/api/src/pcapi/core/providers/models.py @@ -11,7 +11,6 @@ from pcapi.core.offerers.models import Venue import pcapi.core.providers.constants as provider_constants -from pcapi.infrastructure.repository.stock_provider.provider_api import ProviderAPI from pcapi.models import Base from pcapi.models import Model from pcapi.models.deactivable_mixin import DeactivableMixin @@ -85,17 +84,6 @@ def hasProviderEnableCharlie(self) -> bool: def hasOffererProvider(self) -> bool: return bool(self.offererProvider) - @property - def implements_provider_api(self) -> bool: - return self.apiUrl is not None and not self.offererProvider - - def getProviderAPI(self) -> ProviderAPI: - return ProviderAPI( - api_url=self.apiUrl, # type: ignore[arg-type] - name=self.name, - authentication_token=self.authToken, - ) - class VenueProviderExternalUrls(PcObject, Base, Model, DeactivableMixin): """ @@ -279,16 +267,6 @@ class VenueProviderCreationPayload: venueIdAtOfferProvider: str | None = None -@dataclass -class StockDetail: - products_provider_reference: str - offers_provider_reference: str - venue_reference: str - stocks_provider_reference: str - available_quantity: int - price: decimal.Decimal - - class AllocinePivot(PcObject, Base, Model): venueId: int = sa.Column(sa.BigInteger, sa.ForeignKey("venue.id"), index=False, nullable=False, unique=True) diff --git a/api/src/pcapi/infrastructure/repository/stock_provider/__init__.py b/api/src/pcapi/infrastructure/repository/stock_provider/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/api/src/pcapi/infrastructure/repository/stock_provider/provider_api.py b/api/src/pcapi/infrastructure/repository/stock_provider/provider_api.py deleted file mode 100644 index 8be42c61d1c..00000000000 --- a/api/src/pcapi/infrastructure/repository/stock_provider/provider_api.py +++ /dev/null @@ -1,143 +0,0 @@ -import logging - -from pcapi.core.providers.exceptions import ConnexionToProviderApiFailed -from pcapi.utils import requests - - -logger = logging.getLogger(__name__) - - -class ProviderAPIException(Exception): - status_code: int - - def __init__(self, message: str, status_code: int): - super().__init__(message) - self.status_code = status_code - - -REQUEST_TIMEOUT_FOR_PROVIDERS_IN_SECOND = 60 - - -class ProviderAPI: - def __init__(self, api_url: str, name: str, authentication_token: str | None = None): - self.api_url = api_url - self.name = name - self.authentication_token = authentication_token - - def stocks( - self, siret: str, last_processed_reference: str = "", modified_since: str = "", limit: int = 1000 - ) -> dict: - api_url = self._build_local_provider_url(siret) - params = self._build_local_provider_params(last_processed_reference, modified_since, limit) - headers = {} - - if self.authentication_token is not None: - headers = {"Authorization": f"Basic {self.authentication_token}"} - - response = requests.get( - url=api_url, params=params, headers=headers, timeout=REQUEST_TIMEOUT_FOR_PROVIDERS_IN_SECOND - ) - - if response.status_code != 200: - raise ProviderAPIException( - f"Error {response.status_code} when getting {self.name} stocks for SIRET: {siret}", response.status_code - ) - - if not response.content: - return {} - - return response.json() - - def validated_stocks( - self, - siret: str, - last_processed_reference: str = "", - modified_since: str = "", - limit: int = 1000, - ) -> dict: - api_responses = self.stocks(siret, last_processed_reference, modified_since, limit) - stock_responses = api_responses.get("stocks", []) - validated_stock_responses = [] - for stock_response in stock_responses: - if "ref" not in stock_response: - logger.error( - "[%s SYNC] missing ref key in response", - self.name, - extra={"stock": stock_response, "siret": siret}, - ) - continue - - stock_response_ref = stock_response["ref"] - if "available" not in stock_response: - logger.error( - "[%s SYNC] missing available key in response with ref %s", - self.name, - stock_response_ref, - extra={"stock": stock_response, "siret": siret}, - ) - continue - - if stock_response["available"] < 0: - logger.error( - "[%s SYNC] invalid available value %s in response with ref %s", - self.name, - stock_response["available"], - stock_response_ref, - extra={"stock": stock_response, "siret": siret}, - ) - continue - - if stock_response.get("price", None) is None: - logger.error( - "[%s SYNC] missing price in response with ref %s", - self.name, - stock_response_ref, - extra={"stock": stock_response, "siret": siret}, - ) - continue - - validated_stock_responses.append(stock_response) - - api_responses["stocks"] = validated_stock_responses - batch_log_size = 1_000 - if not validated_stock_responses: - logger.info("Got no stocks from Provider API", extra={"siret": siret}) - for i in range(0, len(validated_stock_responses), batch_log_size): - log = f"Got stocks from Provider API (partial log: one log per batch of {batch_log_size})" - logger.info( - log, - extra={ - "stocks": validated_stock_responses[i : i + batch_log_size], - "siret": siret, - }, - ) - return api_responses - - def is_siret_registered(self, siret: str) -> bool: - api_url = self._build_local_provider_url(siret) - headers = {} - - if self.authentication_token is not None: - headers = {"Authorization": f"Basic {self.authentication_token}"} - - try: - response = requests.get(url=api_url, headers=headers, timeout=REQUEST_TIMEOUT_FOR_PROVIDERS_IN_SECOND) - except requests.exceptions.RequestException as error: - extra_infos = {"siret": siret, "api_url": self.api_url, "error": str(error)} - logger.exception("[PROVIDER] request failed", extra=extra_infos) - raise ConnexionToProviderApiFailed - - return response.status_code == 200 - - def _build_local_provider_url(self, siret: str) -> str: - return f"{self.api_url}/{siret}" - - def _build_local_provider_params(self, last_processed_ean: str, modified_since: str, limit: int) -> dict: - params = {"limit": str(limit)} - - if last_processed_ean: - params["after"] = last_processed_ean - if modified_since: - params["modifiedSince"] = modified_since - - return params diff --git a/api/src/pcapi/local_providers/provider_manager.py b/api/src/pcapi/local_providers/provider_manager.py index d1145276e64..393030496e2 100644 --- a/api/src/pcapi/local_providers/provider_manager.py +++ b/api/src/pcapi/local_providers/provider_manager.py @@ -19,7 +19,6 @@ from pcapi.core.providers import repository as providers_repository from pcapi.core.providers.api import update_venue_synchronized_offers_active_status_job from pcapi.core.providers.constants import CINEMA_PROVIDER_NAMES -from pcapi.infrastructure.repository.stock_provider import provider_api import pcapi.local_providers from pcapi.local_providers.cinema_providers.ems.ems_stocks import EMSStocks from pcapi.models import db @@ -58,12 +57,7 @@ def synchronize_venue_providers(venue_providers: list[provider_models.VenueProvi synchronize_venue_provider(venue_provider, limit) except (urllib3_exceptions.HTTPError, requests.exceptions.RequestException) as exception: logger.error("Connexion error while synchronizing venue_provider", extra=log_data | {"exc": exception}) - except provider_api.ProviderAPIException as exception: - logger.error( # pylint: disable=logging-fstring-interpolation - f"ProviderAPIException with code {exception.status_code} while synchronizing venue_provider", - extra=log_data | {"exc": exception}, - ) - except Exception as exception: # pylint: disable=broad-except + except Exception: # pylint: disable=broad-except logger.exception("Unexpected error while synchronizing venue provider", extra=log_data) @@ -129,13 +123,7 @@ def synchronize_ems_venue_providers(from_last_version: bool = False) -> None: except (urllib3_exceptions.HTTPError, requests.exceptions.RequestException) as exception: logger.error("Connexion error while synchronizing venue_provider", extra=log_data | {"exc": exception}) venues_provider_to_sync.discard(venue_provider.id) - except provider_api.ProviderAPIException as exception: - logger.error( # pylint: disable=logging-fstring-interpolation - f"ProviderAPIException with code {exception.status_code} while synchronizing venue_provider", - extra=log_data | {"exc": exception}, - ) - venues_provider_to_sync.discard(venue_provider.id) - except Exception as exception: # pylint: disable=broad-except + except Exception: # pylint: disable=broad-except logger.exception("Unexpected error while synchronizing venue provider", extra=log_data) venues_provider_to_sync.discard(venue_provider.id) diff --git a/api/tests/core/providers/test_api.py b/api/tests/core/providers/test_api.py index 50ff3cb1011..07c0f1d3fa3 100644 --- a/api/tests/core/providers/test_api.py +++ b/api/tests/core/providers/test_api.py @@ -53,15 +53,10 @@ def test_prevent_creation_for_non_existing_provider(self): (offerers_models.VenueTypeCode.DIGITAL, False), ), ) - @patch( - "pcapi.infrastructure.repository.stock_provider.provider_api.ProviderAPI.is_siret_registered", - return_value=True, - ) @patch("pcapi.core.search.async_index_venue_ids") def test_permanent_venue_marking( self, mocked_async_index_venue_ids, - _unused_mock, venue_type, is_permanent, ): diff --git a/api/tests/infrastructure/repository/stock_provider/__init__.py b/api/tests/infrastructure/repository/stock_provider/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/api/tests/infrastructure/repository/stock_provider/provider_api_test.py b/api/tests/infrastructure/repository/stock_provider/provider_api_test.py deleted file mode 100644 index c68610ad91f..00000000000 --- a/api/tests/infrastructure/repository/stock_provider/provider_api_test.py +++ /dev/null @@ -1,130 +0,0 @@ -import json - -import pytest -import requests_mock - -from pcapi.infrastructure.repository.stock_provider.provider_api import ProviderAPI -from pcapi.infrastructure.repository.stock_provider.provider_api import ProviderAPIException - - -class StocksTest: - def setup_method(self): - self.api_url = "http://example.com/stocks" - self.provider_api = ProviderAPI(api_url=self.api_url, name="ProviderAPI") - - def test_get_stocks_with_default_limits(self): - # Given - siret = "1234" - stock_response_data = {"some": "data"} - - # When - with requests_mock.Mocker() as mock: - mock.get(f"{self.api_url}/{siret}?limit=1000", json=stock_response_data) - response = self.provider_api.stocks(siret) - - # Then - assert response == stock_response_data - - def test_get_stocks_with_custom_limits(self): - # Given - siret = "1234" - modified_since = "2019-12-16T00:00:00" - last_processed_ean = "789" - stock_response_data = {"some": "data"} - - # When - expected_url = f"{self.api_url}/{siret}?limit=1000&after={last_processed_ean}&modifiedSince={modified_since}" - with requests_mock.Mocker() as mock: - mock.get(expected_url, json=stock_response_data) - response = self.provider_api.stocks(siret, last_processed_ean, modified_since) - - # Then - assert response == stock_response_data - - def should_call_api_with_authentication_token_if_given(self): - # Given - siret = "1234" - provider_api = ProviderAPI(api_url=self.api_url, name="x", authentication_token="744563534") - - # When - with requests_mock.Mocker() as mock: - mocked_get = mock.get(f"{self.api_url}/{siret}?limit=1000") - provider_api.stocks(siret) - - # Then - assert mocked_get.last_request.headers["Authorization"] == "Basic 744563534" - - def should_raise_error_when_provider_api_request_fails(self): - # Given - siret = "1234" - - # When - with requests_mock.Mocker() as mock: - mock.get(f"{self.api_url}/{siret}?limit=1000", status_code=400) - with pytest.raises(ProviderAPIException) as exception: - self.provider_api.stocks(siret=siret) - - # Then - assert str(exception.value) == f"Error 400 when getting ProviderAPI stocks for SIRET: {siret}" - - def should_return_empty_json_body_when_provider_returns_200_with_no_body(self): - # Given - siret = "1234" - - # When - with requests_mock.Mocker() as mock: - mock.get(f"{self.api_url}/{siret}?limit=1000", content=b"") - response = self.provider_api.stocks(siret=siret) - - # Then - assert response == {} - - def should_raise_error_if_content_is_not_json(self): - siret = "1234" - - with pytest.raises(json.JSONDecodeError): - with requests_mock.Mocker() as mock: - mock.get(f"{self.api_url}/{siret}?limit=1000", content=b"invalid JSON") - self.provider_api.stocks(siret=siret) - - -class IsSiretRegisteredTest: - def setup_method(self): - self.api_url = "http://example.com/stocks" - self.provider_api = ProviderAPI(api_url=self.api_url, name="ProviderAPI") - - def test_is_siret_registered_ok(self): - # Given - siret = "1234" - - # When - with requests_mock.Mocker() as mock: - mock.get(f"{self.api_url}/{siret}") - registered = self.provider_api.is_siret_registered(siret=siret) - - assert registered - - def test_is_siret_registered_nok_if_not_200(self): - # Given - siret = "1234" - - # When - with requests_mock.Mocker() as mock: - mock.get(f"{self.api_url}/{siret}", status_code=400) - registered = self.provider_api.is_siret_registered(siret=siret) - - assert not registered - - def should_call_api_with_authentication_token_if_given(self): - # Given - siret = "1234" - provider_api = ProviderAPI(api_url=self.api_url, name="x", authentication_token="744563534") - - # When - with requests_mock.Mocker() as mock: - mocked_get = mock.get(f"{self.api_url}/{siret}") - registered = provider_api.is_siret_registered(siret=siret) - - # Then - assert registered - assert mocked_get.last_request.headers["Authorization"] == "Basic 744563534"