Add a database with integration tests (#3)

* add the db sqlalchemy code

* add psycopg2-binary

* add the lock file

* Add Integration tests

* Remove the repository unit tests

* remove need for tty when populating db

* Pipe db load output to file

* complete transition to a database

* Fix unit tests

* update the documentation

* bolster tests

* fix integration tests

* id service, more tests and error handling

* Add exceptions and more tests

.github/workflows/ci.yml

@@ -26,8 +26,14 @@ jobs:
       - name: Build
         run: make build
 
-      - name: Run Tests
-        run: make test
+      - name: Run Unit Tests
+        run: make unit_test
+
+      - name: Setup test db
+        run: make setup_test_db
+
+      - name: Run Integration Tests
+        run: make integration_test
 
       - name: Configure AWS credentials
         uses: aws-actions/configure-aws-credentials@v1-node16

.gitignore

@@ -155,3 +155,6 @@ backend/models
 
 # PyCharm
 .idea/
+
+# Sundry files
+load_blank.txt

DESIGN.md

@@ -0,0 +1,34 @@
+# Design
+
+Everything written here follows the "Strong opinions weakly held" principle.
+
+The approach taken is influenced by:
+- [The Twelve Factor App](https://12factor.net/)
+- [SOLID principles](https://www.baeldung.com/solid-principles)
+- [Domain Driven Design (DDD)](https://martinfowler.com/tags/domain%20driven%20design.html) particularly [folder structure](https://dev.to/stevescruz/domain-driven-design-ddd-file-structure-4pja)
+
+## Overview
+
+There are three main layers to the application:
+- **Routing Layer** - The responsibility here is to manage the network payloads and any authentication middleware. All business logic is handed off to...
+- **Service Layer** - Contains all validation and business logic for the application. This in turn uses the...
+- **Repository Layer** - With the sole responsibility of managing how data is stored and retrieved to/from the database. 
+Note, this split into responsibilities for separate entities, should the need arise to create a transaction (for example creating two separate entities atomically) - 
+then this is the responsibility of the service layer to manage the transaction.
+
+## Testing Strategy
+
+### Unit tests
+
+- Routing Layer - this is tested my mocking out the required services by each individual route. The tests should alter how the service behaves to test out the routing layer responds.
+
+- Service Layer - the required repositories are mocked out so that the tests can check the service returns/raises what is expected.
+
+- Repository Layer - there are no unit tests.
+
+### Integration tests
+
+The strategy here is to test all the layers behave as expected. 
+This is done by creating a real test database and populating it with the expected schema (see `blank.sql`).
+
+**NOTE** The database models are copied directly from the `navigator-backend` which has the responsibility for managing the database model and schemas. These model files and the empty schema should be kept in sync with the `navigator-backend`.

Dockerfile

@@ -12,5 +12,6 @@ RUN pip install --no-cache-dir -r requirements.txt
 # Now code
 COPY ./app ./app
 COPY ./unit_tests ./unit_tests
+COPY ./integration_tests ./integration_tests
 
 CMD python app/main.py

GETTING_STARTED.md

@@ -0,0 +1,66 @@
+# Getting Started
+
+## Environment
+
+It is assumed that you have installed [`pyenv`](https://github.com/pyenv/pyenv) to manage your python environments locally.
+
+Create a new environment and activate it whenever you work on the admin backend. This make command will use `pyenv` to create a new virtualenv called `admin-backend`:
+
+```
+make bootstrap
+```
+This can then be activated in any shell with `pyenv activate admin-backend`.
+
+Also ensure that you have the git commit hooks installed to maintain code quality:
+```
+make git_hooks
+```
+
+## Running locally
+
+With your environment correctly setup (see previous section), you can now run the backend locally using:
+```
+python app/main.py
+```
+
+**NOTE** if you get the error: `ModuleNotFoundError: No module named 'app'` you may need to add you current working directory to `PYTHONPATH`
+
+This should run the app [locally on port 8888](http://0.0.0.0:8888) and the json logging should appear in the console.
+
+## Building
+
+This backend component is tested and deployed as a docker container. This container can be built and used locally with:
+
+```
+make build
+```
+
+This should generate a container image named `navigator-admin-backend`, this image can be run locally with:
+
+```
+make run
+```
+
+## Testing
+
+### Unit Tests
+
+These tests are designed not to require a database and therefore run quickly. These can be run locally with `pytest -vvv unit_tests` or from within the container using `make unit_test`. The second approach is preferred as this is how the tests run in the Github Actions CI pipeline.
+
+### Integration Tests
+
+These tests are designed to require a database and therefore will pull and run a postgres container. These can be run locally with `pytest -vvv integration_tests` - however this will require that you have spun up a local postgres instance (which can be done using `make setup_test_db`).
+
+The preferred way it to use `make setup_test_db integration_tests` as this is how the tests run in the Github Actions CI pipeline. These commands were split so that the output of the integration tests is easier to read.
+
+## Deploying
+
+Currently the deployment is manual, this required the following steps:
+- Create a new tagged release [here](https://github.com/climatepolicyradar/navigator-admin-backend/releases)
+- Wait for the `semver` Action to run in github - this creates and pushes the image into ECR
+- Log into the AWS console in the environment you wish to deploy.
+- In AppRunner - find the running images and hit the `Deploy` button.
+
+## Finally
+
+Before reading or writing any code or submitting a PR, please read [the design guide](DESIGN.md)

Makefile

@@ -19,8 +19,37 @@ test_bashscripts: build_bats
 build:
 	docker build -t navigator-admin-backend .
 
-test: build
-	docker run navigator-admin-backend pytest -vvv unit_tests
+unit_test: build
+	docker run --rm navigator-admin-backend pytest -vvv unit_tests
+
+setup_test_db:
+	@echo Setting up...
+	-docker network create test-network
+	-docker stop test_db
+	@echo Starting Postgres...
+	docker pull postgres:14
+	docker run --rm -d -p 5432:5432 \
+		--name test_db \
+		--network=test-network \
+		-v ${PWD}/integration_tests:/data-load \
+		-e POSTGRES_PASSWORD=password \
+		-e POSTGRES_USER=navigator \
+		postgres:14 
+	sleep 3
+	@echo Loading schema...
+	docker exec test_db psql -U navigator -f /data-load/blank.sql > load_blank.txt
+
+integration_test: build 
+	@echo Assuming setup_test_db has already run.
+	@echo Running tests...
+	docker run --rm \
+		--network=test-network \
+		-e ADMIN_POSTGRES_HOST=test_db \
+		navigator-admin-backend \
+		pytest -vvv integration_tests
+	docker stop test_db
+
+test: unit_test integration_test
 
 run: build
 	docker run -p 8888:8888 -d navigator-admin-backend

README.md

@@ -1,2 +1,18 @@
 # navigator-admin-backend
 Backend for the Admin Pages
+
+## Background
+
+This repository along with the [frontend repository](https://github.com/climatepolicyradar/navigator-admin-frontend) forms the necessary components that make up the Admin Pages/Interface/Service.
+These pages will provide the ability to edit Documents, Families, Collections and Events (DFCE).
+At the moment an MVP is being worked on that will have limited functionality, the specification can be found on the CPR notion pages here:
+[MVP Admin Interface](https://www.notion.so/climatepolicyradar/MVP-Admin-Interface-bf253a7ab30b4779a846d4322ca4c3f3).
+Also on the notion pages is the 
+[Admin API Specification](https://www.notion.so/climatepolicyradar/Admin-API-Specification-2adecc8411324b8181e05184fc6a5431#8da09a31c3f244e6a5acfacc9dfd9e2f).
+
+## Issues / Progress
+
+See [the linear project](https://linear.app/climate-policy-radar/project/admin-interface-2fbc66adc34c).
+## Developers
+
+If you are new to the repository, please ensure you read the [getting starting guide](GETTING_STARTED.md) and the [design guide](DESIGN.md).

app/api/api_v1/routers/family.py

@@ -6,13 +6,18 @@
 layer would just pass through directly to the repo. So the approach
 implemented directly accesses the "repository" layer.
 """
+import logging
 from fastapi import APIRouter, HTTPException
+from app.errors.repository_error import RepositoryError
+from app.errors.validation_error import ValidationError
 
 from app.model.family import FamilyDTO
 import app.service.family as family_service
 
 families_router = r = APIRouter()
 
+_LOGGER = logging.getLogger(__name__)
+
 
 @r.get(
     "/families/{import_id}",
@@ -28,7 +33,13 @@ async def get_family(
     :raises HTTPException: If the family is not found a 404 is returned.
     :return FamilyDTO: returns a FamilyDTO of the family found.
     """
-    family = family_service.get(import_id)
+    try:
+        family = family_service.get(import_id)
+    except ValidationError as e:
+        raise HTTPException(status_code=400, detail=e.message)
+    except RepositoryError as e:
+        raise HTTPException(status_code=503, detail=e.message)
+
     if family is None:
         raise HTTPException(status_code=404, detail=f"Family not found: {import_id}")
 
@@ -68,11 +79,10 @@ async def search_family(q: str = "") -> list[FamilyDTO]:
 
 
 @r.put(
-    "/families/{import_id}",
+    "/families",
     response_model=FamilyDTO,
 )
 async def update_family(
-    import_id: str,
     new_family: FamilyDTO,
 ) -> FamilyDTO:
     """
@@ -82,9 +92,16 @@ async def update_family(
     :raises HTTPException: If the family is not found a 404 is returned.
     :return FamilyDTO: returns a FamilyDTO of the family updated.
     """
-    family = family_service.update(import_id, new_family)
+    try:
+        family = family_service.update(new_family)
+    except ValidationError as e:
+        raise HTTPException(status_code=400, detail=e.message)
+    except RepositoryError as e:
+        raise HTTPException(status_code=503, detail=e.message)
+
     if family is None:
-        raise HTTPException(status_code=404, detail=f"Family not updated: {import_id}")
+        detail = f"Family not updated: {new_family.import_id}"
+        raise HTTPException(status_code=404, detail=detail)
 
     # TODO: Handle db errors when implemented
 
@@ -104,7 +121,13 @@ async def create_family(
     :raises HTTPException: If the family is not found a 404 is returned.
     :return FamilyDTO: returns a FamilyDTO of the new family.
     """
-    family = family_service.create(new_family)
+    try:
+        family = family_service.create(new_family)
+    except ValidationError as e:
+        raise HTTPException(status_code=400, detail=e.message)
+    except RepositoryError as e:
+        raise HTTPException(status_code=503, detail=e.message)
+
     if family is None:
         raise HTTPException(
             status_code=404, detail=f"Family not created: {new_family.import_id}"
@@ -127,6 +150,12 @@ async def delete_family(
     :param str import_id: Specified import_id.
     :raises HTTPException: If the family is not found a 404 is returned.
     """
-    family_deleted = family_service.delete(import_id)
+    try:
+        family_deleted = family_service.delete(import_id)
+    except ValidationError as e:
+        raise HTTPException(status_code=400, detail=e.message)
+    except RepositoryError as e:
+        raise HTTPException(status_code=503, detail=e.message)
+
     if not family_deleted:
         raise HTTPException(status_code=404, detail=f"Family not deleted: {import_id}")

app/config.py

@@ -0,0 +1,11 @@
+import os
+
+ADMIN_POSTGRES_USER = os.getenv("ADMIN_POSTGRES_USER", "navigator")
+ADMIN_POSTGRES_PASSWORD = os.getenv("ADMIN_POSTGRES_PASSWORD", "password")
+ADMIN_POSTGRES_HOST = os.getenv("ADMIN_POSTGRES_HOST", "localhost")
+ADMIN_POSTGRES_DATABASE = os.getenv("ADMIN_POSTGRES_DATABASE", "navigator")
+
+_creds = f"{ADMIN_POSTGRES_USER}:{ADMIN_POSTGRES_PASSWORD}"
+SQLALCHEMY_DATABASE_URI = (
+    f"postgresql://{_creds}@{ADMIN_POSTGRES_HOST}:5432/{ADMIN_POSTGRES_DATABASE}"
+)

app/db/models/__init__.py

@@ -0,0 +1,4 @@
+from app.db.models import document
+from app.db.models import app
+from app.db.models import law_policy
+from app.db.session import Base

app/db/models/app/__init__.py

@@ -0,0 +1 @@
+from .users import AppUser, OrganisationUser, Organisation

app/db/models/app/users.py

@@ -0,0 +1,40 @@
+import sqlalchemy as sa
+from sqlalchemy import PrimaryKeyConstraint
+
+from app.db.session import Base
+
+
+class AppUser(Base):
+    """Table of app users in the system."""
+
+    __tablename__ = "app_user"
+
+    email = sa.Column(sa.String, primary_key=True, nullable=False)
+    name = sa.Column(sa.String)
+    hashed_password = sa.Column(sa.String)
+    is_superuser = sa.Column(sa.Boolean, default=False, nullable=False)
+
+
+class Organisation(Base):
+    """Table of organisations to which admin users may belong."""
+
+    __tablename__ = "organisation"
+
+    id = sa.Column(sa.Integer, primary_key=True)
+    name = sa.Column(sa.String)
+    description = sa.Column(sa.String)
+    organisation_type = sa.Column(sa.String)
+
+
+class OrganisationUser(Base):
+    """Link table for admin and organisation."""
+
+    __tablename__ = "organisation_admin"
+
+    appuser_email = sa.Column(sa.String, sa.ForeignKey(AppUser.email), nullable=False)
+    organisation_id = sa.Column(sa.ForeignKey(Organisation.id), nullable=False)
+    job_title = sa.Column(sa.String)
+    is_active = sa.Column(sa.Boolean, default=False, nullable=False)
+    is_admin = sa.Column(sa.Boolean, default=False, nullable=False)
+
+    PrimaryKeyConstraint(appuser_email, organisation_id)

app/db/models/document/__init__.py

@@ -0,0 +1 @@
+from .physical_document import PhysicalDocument