Skip to content

Commit

Permalink
Model query abstractions (#688)
Browse files Browse the repository at this point in the history
* ManifestHandler filter abstract factory

* update out coverage settings and add tool for running tests in parrallel

* update test for manifest handler filter abstract factory

* Reorg profile app classes from the incorrect apps to the profile app (RcrainfoProfile and TrakProfile services, serializers)

the abstracted class views from DRF was causing problems with the model manager query methods

* move the API view for list of organization sites to the site app

* TrakSite model get_by_epa_id query method

* use pytest-xdist to run test in parallel cor CI/CD
  • Loading branch information
dpgraham4401 authored Feb 22, 2024
1 parent 607d9d1 commit 4a1f358
Show file tree
Hide file tree
Showing 56 changed files with 566 additions and 386 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test_server.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,4 @@ jobs:
HT_DB_NAME: ${{ env.POSTGRES_DB }}
HT_DB_PORT: ${{ job.services.postgres.ports[5432] }}
run: |
pytest
pytest -n auto
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
**/.env
**/.secrets
.coverage
.coveragerc
**/*.sqlite3
**/*.rdb
**/.pytest_cache/
Expand Down
3 changes: 2 additions & 1 deletion .idea/runConfigurations/PyTest.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions runhaz.sh
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ run_pre_commit() {

run_coverage() {
# Run the test suite with coverage
(cd $base_dir/server; eval "coverage run -m pytest")
(cd $base_dir/server; eval "coverage report")
(cd "$base_dir"/server || exit; eval "coverage run")
(cd "$base_dir"/server || exit; eval "coverage report")
exit
}

Expand Down
36 changes: 36 additions & 0 deletions server/.coveragerc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
[run]
source = apps
branch = True
command_line = -m pytest

omit =
*admin.py
*apps.py
**/__init__.py
*wsgi.py
*asgi.py
*settings.py

[report]
# Exclude lines that are pragma: no cover
exclude_lines =
pragma: no cover

# Don't complain about missing debug-only code:
def __repr__
if self\.debug

# Don't complain if tests don't hit defensive assertion code:
raise AssertionError
raise NotImplementedError

# Don't complain if non-runnable code isn't run:
if 0:
if __name__ == .__main__.:
pass

# files to run coverage on but omit in the report
omit =
**/migrations/*
**/test_*.py
skip_empty = True
1 change: 1 addition & 0 deletions server/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ static/
.mypy_cache
test_db-journal
**/*coverage.json
/htmlcov/
2 changes: 1 addition & 1 deletion server/apps/core/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 4.2.10 on 2024-02-17 21:09
# Generated by Django 4.2.10 on 2024-02-21 22:17

import django.contrib.auth.models
import django.contrib.auth.validators
Expand Down
2 changes: 1 addition & 1 deletion server/apps/handler/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 4.2.10 on 2024-02-17 21:09
# Generated by Django 4.2.10 on 2024-02-21 22:17

import apps.handler.models.contact_models
from django.db import migrations, models
Expand Down
4 changes: 2 additions & 2 deletions server/apps/handler/migrations/0002_initial.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 4.2.10 on 2024-02-17 21:09
# Generated by Django 4.2.10 on 2024-02-21 22:17

from django.db import migrations, models
import django.db.models.deletion
Expand All @@ -9,8 +9,8 @@ class Migration(migrations.Migration):
initial = True

dependencies = [
('handler', '0001_initial'),
('manifest', '0001_initial'),
('handler', '0001_initial'),
]

operations = [
Expand Down
4 changes: 2 additions & 2 deletions server/apps/handler/models/handler_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@


class HandlerManager(TrakBaseManager):
"""Inter-model related functionality for the Handler Model"""
"""model manager and query interface for Handler model."""

def save(self, instance: Optional["Handler"], **handler_data) -> "Handler":
paper_signature = handler_data.pop("paper_signature", None)
Expand Down Expand Up @@ -93,7 +93,7 @@ def __str__(self):


class TransporterManager(HandlerManager):
"""Inter-model related functionality for Transporter Model"""
"""Transporter Model database querying interface"""

def save(self, instance: Optional["Transporter"], **data: Dict) -> "Transporter":
"""Create a Transporter from a manifest instance and rcra_site dict"""
Expand Down
4 changes: 1 addition & 3 deletions server/apps/handler/models/signature.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,7 @@ def __str__(self):


class ESignatureManager(TrakBaseManager):
"""
Inter-model related functionality for ESignature Model
"""
"""ESignature Model database querying interface"""

def save(self, **e_signature_data):
"""
Expand Down
2 changes: 1 addition & 1 deletion server/apps/manifest/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 4.2.10 on 2024-02-17 21:09
# Generated by Django 4.2.10 on 2024-02-21 22:17

import apps.manifest.models
from django.db import migrations, models
Expand Down
97 changes: 71 additions & 26 deletions server/apps/manifest/models.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
import re
from typing import List, Optional
from abc import ABC, abstractmethod
from typing import List, Literal, Optional

from django.conf import settings
from django.core.exceptions import ValidationError
Expand All @@ -16,46 +17,90 @@


def draft_mtn():
"""
A callable that returns a timestamped draft MTN in lieu of an
official MTN from e-Manifest
"""
"""returns a timestamped draft MTN in lieu of an official MTN from e-Manifest"""
mtn_count: int = Manifest.objects.all().count()
return f"{str(mtn_count).zfill(9)}DFT"


def validate_mtn(value):
"""Validate manifest tracking number format"""
if not re.match(r"[0-9]{9}[A-Z]{3}", value):
raise ValidationError(
_("%(value)s is not in valid MTN format: [0-9]{9}[A-Z]{3}"),
params={"value": value},
)


class ManifestManager(models.Manager):
"""Manifest Model querying interface"""
class ManifestHandlerFilter(ABC):
"""Abstract class with methods for filtering manifests by handler"""

@abstractmethod
def filter_by_epa_id(self, epa_id: str) -> QuerySet:
pass


class GeneratorFilter(ManifestHandlerFilter):
"""ManifestHandlerFilter implementation for filtering manifests by generator"""

def filter_by_epa_id(self, epa_id: str) -> Q:
return Q(generator__rcra_site__epa_id=epa_id)


class TransporterFilter(ManifestHandlerFilter):
"""ManifestHandlerFilter implementation for filtering manifests by Transporter"""

def filter_by_epa_id(self, epa_id: str) -> Q:
return Q(transporters__rcra_site__epa_id=epa_id)


def existing_mtn(self, *args, mtn: List[str]) -> QuerySet:
"""
Filter non-existent manifest tracking numbers (MTN).
Also accepts *args to pass things like django.db.models.Q objects
"""
return self.model.objects.filter(*args, mtn__in=mtn)
class TsdfFilter(ManifestHandlerFilter):
"""ManifestHandlerFilter implementation for filtering manifests by receiving facility"""

def filter_by_epa_id(self, epa_id: str) -> Q:
return Q(tsdf__rcra_site__epa_id=epa_id)


class AllHandlerFilter(ManifestHandlerFilter):
"""ManifestHandlerFilter implementation for filtering manifests by all handlers types"""

def filter_by_epa_id(self, epa_id: str) -> Q:
return Q(
Q(generator__rcra_site__epa_id=epa_id)
| Q(tsdf__rcra_site__epa_id=epa_id)
| Q(transporters__rcra_site__epa_id=epa_id)
)


class HandlerFilterFactory:
"""Abstract Factory for creating ManifestHandlerFilter instances based on site_type"""

@staticmethod
def get_handler_query(site_id: str, site_type: RcraSiteType | str):
"""Returns a Django Query object for filtering by rcra_site type"""
if isinstance(site_type, str) and not isinstance(site_type, RcraSiteType):
site_type = site_type.lower()
match site_type:
case RcraSiteType.GENERATOR | "generator":
return Q(generator__rcra_site__epa_id=site_id)
case RcraSiteType.TRANSPORTER | "transporter":
return Q(transporters__rcra_site__epa_id=site_id)
case RcraSiteType.TSDF | "tsdf":
return Q(tsdf__rcra_site__epa_id=site_id)
case _:
raise ValueError(f"unrecognized site_type argument {site_type}")
def get_filter(site_type: RcraSiteType | Literal["all"]):
if site_type == RcraSiteType.GENERATOR:
return GeneratorFilter()
elif site_type == RcraSiteType.TRANSPORTER:
return TransporterFilter()
elif site_type == RcraSiteType.TSDF:
return TsdfFilter()
elif site_type == "all":
return AllHandlerFilter()
else:
raise ValueError(f"unrecognized site_type argument {site_type}")


class ManifestManager(models.Manager):
"""Manifest Model database querying interface"""

def filter_existing_mtn(self, mtn: List[str]) -> QuerySet:
"""Filter non-existent manifest tracking numbers (MTN)."""
return self.model.objects.filter(mtn__in=mtn)

def filter_by_handler_epa_id(
self, epa_ids: [str], site_type: RcraSiteType | Literal["all"] = "all"
) -> QuerySet:
"""Filter manifests by site_id and site_type"""
handler_filter = HandlerFilterFactory.get_filter(site_type)
return self.filter(handler_filter.filter_by_epa_id(epa_ids))

@classmethod
def save(cls, instance: Optional["Manifest"], **manifest_data: dict) -> "Manifest":
Expand Down
41 changes: 32 additions & 9 deletions server/apps/manifest/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@
logger = logging.getLogger(__name__)


class TrakBaseSerializer(serializers.ModelSerializer):
"""
The Django Trak app base serializers class used to share functionality
across trak app serializers universally.
"""

class ManifestBaseSerializer(serializers.ModelSerializer):
def __str__(self):
return f"{self.__class__.__name__}"

Expand All @@ -47,7 +42,7 @@ def to_representation(self, instance):
return data


class AdditionalInfoSerializer(TrakBaseSerializer):
class AdditionalInfoSerializer(ManifestBaseSerializer):
originalManifestTrackingNumbers = serializers.JSONField(
allow_null=True,
required=False,
Expand Down Expand Up @@ -87,7 +82,7 @@ class Meta:
)


class ManifestSerializer(TrakBaseSerializer):
class ManifestSerializer(ManifestBaseSerializer):
"""
Manifest model serializer for JSON marshalling/unmarshalling
"""
Expand Down Expand Up @@ -281,7 +276,7 @@ class Meta:
]


class PortOfEntrySerializer(TrakBaseSerializer):
class PortOfEntrySerializer(ManifestBaseSerializer):
"""
Serializer for Port Of Entry
"""
Expand All @@ -301,3 +296,31 @@ class PortOfEntrySerializer(TrakBaseSerializer):
class Meta:
model = PortOfEntry
fields = ["state", "cityPort"]


class MtnSerializer(serializers.ModelSerializer):
"""Select details of a manifest including manifest tracking number (mtn)."""

manifestTrackingNumber = serializers.CharField(
source="mtn",
required=False,
)
# status
submissionType = serializers.CharField(
source="submission_type",
required=False,
)
signatureStatus = serializers.BooleanField(
source="signature_status",
allow_null=True,
default=False,
)

class Meta:
model = Manifest
fields = [
"manifestTrackingNumber",
"status",
"submissionType",
"signatureStatus",
]
2 changes: 1 addition & 1 deletion server/apps/manifest/services/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from .emanifest import EManifest, EManifestError, PullManifestsResult, TaskResponse
from .manifest_services import (
from .manifest import (
create_manifest,
get_manifests,
save_emanifest,
Expand Down
7 changes: 4 additions & 3 deletions server/apps/manifest/services/emanifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,10 @@ def save(self, manifest: dict) -> dict:
def _filter_mtn(
*, mtn: list[str], site_id: str, site_type: Literal["Generator", "Tsdf", "Transporter"]
) -> list[str]:
site_filter = Manifest.objects.get_handler_query(site_id, site_type)
existing_mtn = Manifest.objects.existing_mtn(site_filter, mtn=mtn)
return [manifest.mtn for manifest in existing_mtn]
handler_filter = Manifest.objects.filter_by_handler_epa_id([site_id], site_type)
existing_mtn = Manifest.objects.filter_existing_mtn(mtn=mtn)
filtered_mtn = handler_filter & existing_mtn
return [manifest.mtn for manifest in filtered_mtn]

def _retrieve_manifest(self, mtn: str):
"""Retrieve a manifest from RCRAInfo"""
Expand Down
File renamed without changes.
Loading

0 comments on commit 4a1f358

Please sign in to comment.