From e48fdb26ce015db3b1bf8791c2b81e5bff887616 Mon Sep 17 00:00:00 2001 From: David Paul Graham <43794491+dpgraham4401@users.noreply.github.com> Date: Thu, 19 Oct 2023 14:14:54 -0400 Subject: [PATCH] Fix sync rcra profile (#613) * adjust docker-compose and sync rcra profile url parameters * add RCRAInfo Environment to celery worker and beat configs * use emanifest package new method of initiation * remove sync method from RcraProfile model * use RcraProfileServiceError to control what exceptions are raised from service module * introduce SiteServiceError exception class * add regrassion tests showing RcrainfoService sets state in __init__ by environment --- client/src/features/profile/RcraProfile.tsx | 2 +- docker-compose.yaml | 8 +++ infra/README.md | 2 +- infra/gcp/dev/.terraform.lock.hcl | 4 +- server/apps/core/models.py | 7 --- server/apps/core/services/rcrainfo_service.py | 28 +++++---- .../apps/core/tests/test_rcrainfo_service.py | 22 +++++++ server/apps/core/urls.py | 2 +- server/apps/core/views/profile_views.py | 8 +-- .../apps/sites/services/profile_services.py | 58 ++++++++++++------- server/apps/sites/services/site_services.py | 10 +++- server/apps/sites/tasks/profile_tasks.py | 7 ++- server/apps/trak/tasks/manifest_task.py | 8 +-- 13 files changed, 111 insertions(+), 55 deletions(-) create mode 100644 server/apps/core/tests/test_rcrainfo_service.py diff --git a/client/src/features/profile/RcraProfile.tsx b/client/src/features/profile/RcraProfile.tsx index f7ec1cac5..03a45c8c7 100644 --- a/client/src/features/profile/RcraProfile.tsx +++ b/client/src/features/profile/RcraProfile.tsx @@ -183,7 +183,7 @@ export function RcraProfile({ profile }: ProfileViewProps) { variant="primary" onClick={() => htApi - .get(`profile/${profile.user}/sync`) + .get(`rcra/profile/sync`) .then((response) => { dispatch( addNotification({ diff --git a/docker-compose.yaml b/docker-compose.yaml index 6fb6fd6cb..191311f6f 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -28,6 +28,12 @@ services: HT_DB_PASSWORD: ${HT_DB_PASSWORD} HT_DB_HOST: postgres HT_DB_PORT: ${HT_DB_PORT} + command: | + sh -c " + python manage.py makemigrations && + python manage.py migrate && + python manage.py loaddata dev_data.yaml && + python manage.py runserver 0.0.0.0:8000" depends_on: postgres: condition: service_healthy @@ -48,6 +54,7 @@ services: CELERY_BROKER_URL: ${CELERY_BROKER_URL} CELERY_RESULT_BACKEND: ${CELERY_RESULT_BACKEND} CELERY_LOG_LEVEL: ${CELERY_LOG_LEVEL} + HT_RCRAINFO_ENV: ${HT_RCRAINFO_ENV} HT_DB_ENGINE: ${HT_DB_ENGINE} HT_DB_NAME: ${HT_DB_NAME} HT_DB_USER: ${HT_DB_USER} @@ -69,6 +76,7 @@ services: CELERY_BROKER_URL: ${CELERY_BROKER_URL} CELERY_RESULT_BACKEND: ${CELERY_RESULT_BACKEND} CELERY_LOG_LEVEL: ${CELERY_LOG_LEVEL} + HT_RCRAINFO_ENV: ${HT_RCRAINFO_ENV} HT_DB_ENGINE: ${HT_DB_ENGINE} HT_DB_NAME: ${HT_DB_NAME} HT_DB_USER: ${HT_DB_USER} diff --git a/infra/README.md b/infra/README.md index 602537acb..c2ecfe424 100644 --- a/infra/README.md +++ b/infra/README.md @@ -13,7 +13,7 @@ Contributions to the IaC should follow the general directory structure below: │ │ ├── rest-api # Helm chart for the HTTP REST API │ │ └── worker # Helm chart for the Celery Async worker │ └── templates -└── terrafrom # Terraform configs for provisioning resources +└── gcp # Terraform configs for provisioning resources ├── dev # Terraform configs for the dev environment ├── modules # Resuable Terraform modules ├── org # Global Terraform configs for the organization diff --git a/infra/gcp/dev/.terraform.lock.hcl b/infra/gcp/dev/.terraform.lock.hcl index 506719ede..1a919a0fa 100644 --- a/infra/gcp/dev/.terraform.lock.hcl +++ b/infra/gcp/dev/.terraform.lock.hcl @@ -3,8 +3,9 @@ provider "registry.terraform.io/hashicorp/google" { version = "4.78.0" - constraints = "4.78.0" + constraints = ">= 3.33.0, >= 3.83.0, >= 4.25.0, >= 4.64.0, 4.78.0, < 5.0.0" hashes = [ + "h1:SGwfLgQ7zDlGeZKn92QUn5zWVhFqO4LvHPTo3VqVq+0=", "h1:wxYr3Z7xTg+rugpIu/DKOW88nJ7V76lYvq50+auW5cc=", "zh:09a09e79ac404ea9ce2030185973130ed5f25e7f2c1d46093ee67fcc8f94e220", "zh:1dd579d1200fd9cb57b84b326f401674cc5c62670c85fff7bb90642fd2379d10", @@ -25,6 +26,7 @@ provider "registry.terraform.io/hashicorp/google-beta" { version = "4.84.0" constraints = ">= 4.64.0, < 5.0.0" hashes = [ + "h1:0w1Y03/eJrW6VqsC4GQknCDaiRHSF2eWmiN7c90Tgtk=", "h1:BkdCMbyvAkOMslh6tGz+5K/bsziWUmZvt+4pfR3xtA4=", "zh:0c17bd21a0d98a5063b5bbdad0feac559913061264953d96b3b82289b9938d83", "zh:138dd45494953f6ce0f837ab29ca61ff91e2001e7cf49356021a962030ccf217", diff --git a/server/apps/core/models.py b/server/apps/core/models.py index bc5f0d494..f2c8e044d 100644 --- a/server/apps/core/models.py +++ b/server/apps/core/models.py @@ -65,13 +65,6 @@ class Meta: def __str__(self): return f"{self.user.username}" - def sync(self): - """Launch task to sync use profile. ToDo: remove this method""" - from apps.sites.tasks import sync_user_sites - - task = sync_user_sites.delay(str(self.user.username)) - return task - @property def is_api_user(self) -> bool: """Returns true if the use has Rcrainfo API credentials""" diff --git a/server/apps/core/services/rcrainfo_service.py b/server/apps/core/services/rcrainfo_service.py index 7c361fbbb..58f748fa1 100644 --- a/server/apps/core/services/rcrainfo_service.py +++ b/server/apps/core/services/rcrainfo_service.py @@ -1,7 +1,7 @@ import logging -import os -from typing import Optional +from typing import Literal, Optional +import emanifest from django.db import IntegrityError from emanifest import RcrainfoClient, RcrainfoResponse # type: ignore @@ -19,16 +19,25 @@ class RcrainfoService(RcrainfoClient): datetime_format = "%Y-%m-%dT%H:%M:%S.%f%z" - def __init__(self, *, api_username: str, rcrainfo_env: Optional[str] = None, **kwargs): + def __init__( + self, + *, + api_username: str, + rcrainfo_env: Optional[Literal["preprod"] | Literal["prod"]] = None, + **kwargs, + ): self.api_user = api_username if RcraProfile.objects.filter(user__username=self.api_user).exists(): self.profile = RcraProfile.objects.get(user__username=self.api_user) else: self.profile = None - if rcrainfo_env is None: - rcrainfo_env = os.getenv("HT_RCRAINFO_ENV", "preprod") - self.rcrainfo_env = rcrainfo_env - super().__init__(rcrainfo_env, **kwargs) + if rcrainfo_env == "prod": + self.rcrainfo_env: Literal["preprod"] | Literal["prod"] = rcrainfo_env + base_url = emanifest.RCRAINFO_PROD + else: + self.rcrainfo_env = "preprod" + base_url = emanifest.RCRAINFO_PREPROD + super().__init__(base_url, **kwargs) @property def has_api_user(self) -> bool: @@ -56,15 +65,14 @@ def retrieve_key(self, api_key=None) -> str: return super().retrieve_key(self.profile.rcra_api_key) return super().retrieve_key() - def get_user_profile(self, username: Optional[str] = None): + def get_user_profile(self, username: Optional[str] = None) -> RcrainfoResponse: """ Retrieve a user's site permissions from RCRAInfo, It expects the haztrak user to have their unique RCRAInfo user and API credentials in their RcraProfile """ profile = RcraProfile.objects.get(user__username=username or self.api_user) - response = self.search_users(userId=profile.rcra_username) - return response.json() + return self.search_users(userId=profile.rcra_username) def sync_federal_waste_codes(self): """ diff --git a/server/apps/core/tests/test_rcrainfo_service.py b/server/apps/core/tests/test_rcrainfo_service.py new file mode 100644 index 000000000..c8d84d0b0 --- /dev/null +++ b/server/apps/core/tests/test_rcrainfo_service.py @@ -0,0 +1,22 @@ +import emanifest +import pytest + +from apps.core.services import RcrainfoService + + +class TestRcrainfoService: + """Tests the for the RcrainfoService class""" + + @pytest.fixture + def user(self, user_factory): + return user_factory() + + def test_rcrainfo_inits_to_correct_environment(self, user): + rcrainfo = RcrainfoService(api_username=user.username, rcrainfo_env="preprod") + assert rcrainfo.rcrainfo_env == "preprod" + + def test_rcrainfo_inits_base_url_by_env(self, user): + rcrainfo_preprod = RcrainfoService(api_username=user.username, rcrainfo_env="preprod") + assert rcrainfo_preprod.base_url == emanifest.RCRAINFO_PREPROD + rcrainfo_prod = RcrainfoService(api_username=user.username, rcrainfo_env="prod") + assert rcrainfo_prod.base_url == emanifest.RCRAINFO_PROD diff --git a/server/apps/core/urls.py b/server/apps/core/urls.py index dd6019fa1..a927125a1 100644 --- a/server/apps/core/urls.py +++ b/server/apps/core/urls.py @@ -15,7 +15,7 @@ "rcra/", include( [ - path("profile//sync", RcraProfileSyncView.as_view()), + path("profile/sync", RcraProfileSyncView.as_view()), path("profile/", RcraProfileView.as_view()), ] ), diff --git a/server/apps/core/views/profile_views.py b/server/apps/core/views/profile_views.py index 454e19b5d..619274536 100644 --- a/server/apps/core/views/profile_views.py +++ b/server/apps/core/views/profile_views.py @@ -1,4 +1,5 @@ from celery.exceptions import CeleryError +from celery.result import AsyncResult as CeleryTask from rest_framework import status from rest_framework.generics import GenericAPIView, RetrieveUpdateAPIView from rest_framework.request import Request @@ -6,6 +7,7 @@ from apps.core.models import HaztrakUser, RcraProfile from apps.core.serializers import HaztrakUserSerializer, RcraProfileSerializer +from apps.sites.tasks import sync_user_sites class HaztrakUserView(RetrieveUpdateAPIView): @@ -38,14 +40,12 @@ class RcraProfileSyncView(GenericAPIView): with their haztrak (Rcra)profile. """ - queryset = RcraProfile.objects.all() + queryset = None response = Response def get(self, request: Request) -> Response: - """Sync Profile GET method rcra_site""" try: - profile = RcraProfile.objects.get(user=request.user) - task = profile.sync() + task: CeleryTask = sync_user_sites.delay(str(self.request.user)) return self.response({"task": task.id}) except RcraProfile.DoesNotExist as exc: return self.response(data={"error": str(exc)}, status=status.HTTP_404_NOT_FOUND) diff --git a/server/apps/sites/services/profile_services.py b/server/apps/sites/services/profile_services.py index 4eda293dc..70efc09f9 100644 --- a/server/apps/sites/services/profile_services.py +++ b/server/apps/sites/services/profile_services.py @@ -8,13 +8,12 @@ from apps.sites.serializers import RcraPermissionSerializer # type: ignore from ...core.models import RcraProfile # type: ignore -from .site_services import RcraSiteService, SiteService # type: ignore +from .site_services import RcraSiteService, SiteService, SiteServiceError # type: ignore logger = logging.getLogger(__name__) -# ToDo, may be better to have a service level module exception. -class RcraServiceError(Exception): +class RcraProfileServiceError(Exception): """Exception for errors specific to the RcraProfileService""" def __init__(self, message: str): @@ -47,7 +46,7 @@ def __repr__(self): f"<{self.__class__.__name__}(username='{self.username}', rcrainfo='{self.rcrainfo}')>" ) - def pull_rcra_profile(self, *, username: Optional[str] = None): + def pull_rcra_profile(self, *, username: Optional[str] = None) -> None: """ This high level function makes several requests to RCRAInfo to pull... 1. A user's rcrainfo site permissions, it creates a RcraSitePermission for each @@ -56,30 +55,43 @@ def pull_rcra_profile(self, *, username: Optional[str] = None): 3. If a Haztrak Site is not present, create one """ try: - handler_service = RcraSiteService(username=self.username, rcrainfo=self.rcrainfo) - site_service = SiteService(username=self.username, rcrainfo=self.rcrainfo) - if username: - user_to_update: str = username - else: - user_to_update = self.username - user_profile_response = self.rcrainfo.get_user_profile(username=user_to_update) - permissions = self._parse_rcra_response(rcra_response=user_profile_response) + if username is None: + username = self.username + profile_response = self.rcrainfo.get_user_profile(username=username) + permissions = self._parse_rcra_response(rcra_response=profile_response.json()) + self._update_profile_permissions(permissions) + except (RcraProfile.DoesNotExist, Site.DoesNotExist) as exc: + raise RcraProfileServiceError(exc) + + def _update_profile_permissions(self, permissions: list[dict]): + """ + This function creates or updates a user's RcraSitePermission for each site permission + :param permissions: body of response from RCRAInfo + :return: None + """ + try: + handler = RcraSiteService(username=self.username, rcrainfo=self.rcrainfo) + haztrak_site = SiteService(username=self.username, rcrainfo=self.rcrainfo) for rcra_site_permission in permissions: - rcra_site = handler_service.get_or_pull_rcra_site(rcra_site_permission["siteId"]) - site = site_service.create_or_update_site(rcra_site=rcra_site) + rcra_site = handler.get_or_pull_rcra_site(rcra_site_permission["siteId"]) + site = haztrak_site.create_or_update_site(rcra_site=rcra_site) self._create_or_update_rcra_permission( epa_permission=rcra_site_permission, site=site ) - - except (RcraProfile.DoesNotExist, Site.DoesNotExist) as exc: - raise Exception(exc) + except SiteServiceError as exc: + raise RcraProfileServiceError(f"Error creating or updating Haztrak Site {exc}") + except KeyError as exc: + raise RcraProfileServiceError(f"Error parsing RCRAInfo response: {str(exc)}") @staticmethod def _parse_rcra_response(*, rcra_response: dict) -> list: - permissions = [] - for permission_json in rcra_response["users"][0]["sites"]: - permissions.append(permission_json) - return permissions + try: + permissions = [] + for permission_json in rcra_response["users"][0]["sites"]: + permissions.append(permission_json) + return permissions + except KeyError as exc: + raise RcraProfileServiceError(f"Error parsing RCRAInfo response: {str(exc)}") @transaction.atomic def _create_or_update_rcra_permission( @@ -91,4 +103,6 @@ def _create_or_update_rcra_permission( **permission_serializer.validated_data, site=site, profile=self.profile ) return obj - raise Exception("Error Attempting to create RcraSitePermission") + raise RcraProfileServiceError( + f"Error creating instance of RcraSitePermission {permission_serializer.errors}" + ) diff --git a/server/apps/sites/services/site_services.py b/server/apps/sites/services/site_services.py index ea058c1a8..df2342eaa 100644 --- a/server/apps/sites/services/site_services.py +++ b/server/apps/sites/services/site_services.py @@ -14,6 +14,11 @@ logger = logging.getLogger(__name__) +class SiteServiceError(Exception): + def __init__(self, message: str): + super().__init__(message) + + class SiteService: """ SiteService encapsulates the Haztrak site subdomain business logic and use cases. @@ -47,20 +52,19 @@ def sync_rcra_manifest(self, *, site_id: Optional[str] = None) -> PullManifestsR site_id=site_id, start_date=site.last_rcra_sync ) # ToDo: uncomment this after we have manifest development fixtures - # limit the number of manifest to sync at a time + # limit the number of manifest to sync at a time per RCRAInfo rate limits tracking_numbers = tracking_numbers[:30] logger.info(f"Pulling {tracking_numbers} from RCRAInfo") results: PullManifestsResult = manifest_service.pull_manifests( tracking_numbers=tracking_numbers ) # ToDo: uncomment this after we have manifest development fixtures - # Update the Rcrainfo last sync date for future sync operations # site.last_rcra_sync = datetime.now().replace(tzinfo=timezone.utc) site.save() return results except Site.DoesNotExist: logger.warning(f"Site Does not exists {site_id}") - raise Exception + raise SiteServiceError(f"Site Does not exists {site_id}") @transaction.atomic def create_or_update_site( diff --git a/server/apps/sites/tasks/profile_tasks.py b/server/apps/sites/tasks/profile_tasks.py index ccd180adb..356c6dd63 100644 --- a/server/apps/sites/tasks/profile_tasks.py +++ b/server/apps/sites/tasks/profile_tasks.py @@ -4,6 +4,8 @@ from celery.exceptions import Ignore, Reject from requests import RequestException +from apps.sites.services.profile_services import RcraProfileServiceError + logger = logging.getLogger(__name__) @@ -28,6 +30,9 @@ def sync_user_sites(self: RcraProfileTasks, username: str) -> None: except (ConnectionError, RequestException, TimeoutError): # ToDo retry if network error, see celery docs raise Reject() + except RcraProfileServiceError as exc: + self.update_state(state=states.FAILURE, meta={"error": f"{str(exc)}"}) + raise Ignore() except Exception as exc: - self.update_state(state=states.FAILURE, meta=f"unknown error: {exc}") + self.update_state(state=states.FAILURE, meta={"unknown error": f"{str(exc)}"}) raise Ignore() diff --git a/server/apps/trak/tasks/manifest_task.py b/server/apps/trak/tasks/manifest_task.py index 906d5d582..a47ca53ad 100644 --- a/server/apps/trak/tasks/manifest_task.py +++ b/server/apps/trak/tasks/manifest_task.py @@ -70,10 +70,10 @@ def sign_manifest( except (ConnectionError, TimeoutError) as exc: raise Reject(exc) except ValueError as exc: - self.update_state(state=states.FAILURE, meta=f"ValueError: {exc}") + self.update_state(state=states.FAILURE, meta={"Error": f"{repr(exc)}"}) raise Ignore() except Exception as exc: - self.update_state(state=states.FAILURE, meta=f"unknown error: {exc}") + self.update_state(state=states.FAILURE, meta={"unknown error": f"{exc}"}) raise Ignore() @@ -88,7 +88,7 @@ def sync_site_manifests(self, *, site_id: str, username: str): return results except Exception as exc: logger.error(f"failed to sync {site_id} manifest") - self.update_state(state=states.FAILURE, meta=f"Internal Error {exc}") + self.update_state(state=states.FAILURE, meta={f"Error: {exc}"}) raise Ignore() @@ -115,5 +115,5 @@ def create_rcra_manifest(self, *, manifest: dict, username: str): return resp.json() except Exception as exc: logger.error("error: ", exc) - task_status.update_task_status(status="FAILURE", results=str(exc)) + task_status.update_task_status(status="FAILURE", results={"result": str(exc)}) return {"error": f"Internal Error: {exc}"}