From fff68bd217c066d2d79c50a029c64aac63391f28 Mon Sep 17 00:00:00 2001 From: Harry Date: Mon, 11 Mar 2019 00:48:49 +0000 Subject: [PATCH 01/39] ORM for _allocations set on Batch --- orm.py | 40 ++++++++++++++++++++++++++---------- test_orm.py | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 14 deletions(-) diff --git a/orm.py b/orm.py index 6a3e728e..3e6b4a70 100644 --- a/orm.py +++ b/orm.py @@ -1,5 +1,5 @@ -from sqlalchemy import Table, MetaData, Column, Integer, String, Date -from sqlalchemy.orm import mapper +from sqlalchemy import Table, MetaData, Column, Integer, String, Date, ForeignKey +from sqlalchemy.orm import mapper, relationship import model @@ -9,21 +9,39 @@ order_lines = Table( "order_lines", metadata, - Column("orderid", String(255), primary_key=True), - Column("sku", String(255), primary_key=True), - Column("qty", Integer), + Column("id", Integer, primary_key=True, autoincrement=True), + Column("sku", String(255)), + Column("qty", Integer, nullable=False), + Column("orderid", String(255)), ) batches = Table( "batches", metadata, - Column("reference", String(255), primary_key=True), - Column("sku", String(255), primary_key=True), - Column("_purchased_qty", Integer), - Column("eta", Date), + Column("id", Integer, primary_key=True, autoincrement=True), + Column("reference", String(255)), + Column("sku", String(255)), + Column("_purchased_quantity", Integer, nullable=False), + Column("eta", Date, nullable=True), +) + +allocations = Table( + "allocations", + metadata, + Column("id", Integer, primary_key=True, autoincrement=True), + Column("orderline_id", ForeignKey("order_lines.id")), + Column("batch_id", ForeignKey("batches.id")), ) def start_mappers(): - mapper(model.OrderLine, order_lines) - mapper(model.Batch, batches) + lines_mapper = mapper(model.OrderLine, order_lines) + mapper( + model.Batch, + batches, + properties={ + "_allocations": relationship( + lines_mapper, secondary=allocations, collection_class=set, + ) + }, + ) diff --git a/test_orm.py b/test_orm.py index 9dc98719..6626a13c 100644 --- a/test_orm.py +++ b/test_orm.py @@ -26,10 +26,14 @@ def test_orderline_mapper_can_save_lines(session): assert rows == [("order1", "DECORATIVE-WIDGET", 12)] -def test_batches(session): - session.execute('INSERT INTO "batches" VALUES ("batch1", "sku1", 100, null)') +def test_retrieving_batches(session): session.execute( - 'INSERT INTO "batches" VALUES ("batch2", "sku2", 200, "2011-04-11")' + "INSERT INTO batches (reference, sku, _purchased_quantity, eta)" + ' VALUES ("batch1", "sku1", 100, null)' + ) + session.execute( + "INSERT INTO batches (reference, sku, _purchased_quantity, eta)" + ' VALUES ("batch2", "sku2", 200, "2011-04-11")' ) expected = [ model.Batch("batch1", "sku1", 100, eta=None), @@ -37,3 +41,51 @@ def test_batches(session): ] assert session.query(model.Batch).all() == expected + + +def test_saving_batches(session): + batch = model.Batch("batch1", "sku1", 100, eta=None) + session.add(batch) + session.commit() + rows = list( + session.execute( + 'SELECT reference, sku, _purchased_quantity, eta FROM "batches"' + ) + ) + assert rows == [("batch1", "sku1", 100, None)] + + +def test_saving_allocations(session): + batch = model.Batch("batch1", "sku1", 100, eta=None) + line = model.OrderLine("order1", "sku1", 10) + batch.allocate(line) + session.add(batch) + session.commit() + rows = list(session.execute('SELECT orderline_id, batch_id FROM "allocations"')) + assert rows == [(batch.id, line.id)] + + +def test_retrieving_allocations(session): + session.execute( + 'INSERT INTO order_lines (orderid, sku, qty) VALUES ("order1", "sku1", 12)' + ) + [[olid]] = session.execute( + "SELECT id FROM order_lines WHERE orderid=:orderid AND sku=:sku", + dict(orderid="order1", sku="sku1"), + ) + session.execute( + "INSERT INTO batches (reference, sku, _purchased_quantity, eta)" + ' VALUES ("batch1", "sku1", 100, null)' + ) + [[bid]] = session.execute( + "SELECT id FROM batches WHERE reference=:ref AND sku=:sku", + dict(ref="batch1", sku="sku1"), + ) + session.execute( + "INSERT INTO allocations (orderline_id, batch_id) VALUES (:olid, :bid)", + dict(olid=olid, bid=bid), + ) + + batch = session.query(model.Batch).one() + + assert batch._allocations == {model.OrderLine("order1", "sku1", 12)} From 8dae12639c6247185877d199e3055d0cacb126ea Mon Sep 17 00:00:00 2001 From: Harry Date: Mon, 11 Mar 2019 01:10:30 +0000 Subject: [PATCH 02/39] repository tests --- test_orm.py | 8 +++--- test_repository.py | 67 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 test_repository.py diff --git a/test_orm.py b/test_orm.py index 6626a13c..07a63491 100644 --- a/test_orm.py +++ b/test_orm.py @@ -47,12 +47,10 @@ def test_saving_batches(session): batch = model.Batch("batch1", "sku1", 100, eta=None) session.add(batch) session.commit() - rows = list( - session.execute( - 'SELECT reference, sku, _purchased_quantity, eta FROM "batches"' - ) + rows = session.execute( + 'SELECT reference, sku, _purchased_quantity, eta FROM "batches"' ) - assert rows == [("batch1", "sku1", 100, None)] + assert list(rows) == [("batch1", "sku1", 100, None)] def test_saving_allocations(session): diff --git a/test_repository.py b/test_repository.py new file mode 100644 index 00000000..7c668a39 --- /dev/null +++ b/test_repository.py @@ -0,0 +1,67 @@ +# pylint: disable=protected-access +import model +import repository + + +def test_repository_can_save_a_batch(session): + batch = model.Batch("batch1", "RUSTY-SOAPDISH", 100, eta=None) + + repo = repository.SqlAlchemyRepository(session) + repo.add(batch) + session.commit() + + rows = session.execute( + 'SELECT reference, sku, _purchased_quantity, eta FROM "batches"' + ) + assert list(rows) == [("batch1", "RUSTY-SOAPDISH", 100, None)] + + +def insert_order_line(session): + session.execute( + "INSERT INTO order_lines (orderid, sku, qty)" + ' VALUES ("order1", "GENERIC-SOFA", 12)' + ) + [[orderline_id]] = session.execute( + "SELECT id FROM order_lines WHERE orderid=:orderid AND sku=:sku", + dict(orderid="order1", sku="GENERIC-SOFA"), + ) + return orderline_id + + +def insert_batch(session, batch_id): + session.execute( + "INSERT INTO batches (reference, sku, _purchased_quantity, eta)" + ' VALUES (:batch_id, "GENERIC-SOFA", 100, null)', + dict(batch_id=batch_id), + ) + [[batch_id]] = session.execute( + 'SELECT id FROM batches WHERE reference=:batch_id AND sku="GENERIC-SOFA"', + dict(batch_id=batch_id), + ) + return batch_id + + +def insert_allocation(session, orderline_id, batch_id): + session.execute( + "INSERT INTO allocations (orderline_id, batch_id)" + " VALUES (:orderline_id, :batch_id)", + dict(orderline_id=orderline_id, batch_id=batch_id), + ) + + +def test_repository_can_retrieve_a_batch_with_allocations(session): + orderline_id = insert_order_line(session) + batch1_id = insert_batch(session, "batch1") + insert_batch(session, "batch2") + insert_allocation(session, orderline_id, batch1_id) + + repo = repository.SqlAlchemyRepository(session) + retrieved = repo.get("batch1") + + expected = model.Batch("batch1", "GENERIC-SOFA", 100, eta=None) + assert retrieved == expected # Batch.__eq__ only compares reference + assert retrieved.sku == expected.sku + assert retrieved._purchased_quantity == expected._purchased_quantity + assert retrieved._allocations == { + model.OrderLine("order1", "GENERIC-SOFA", 12), + } From b635a154d8ddf58a3bbf40a1d5c96c65430bc49d Mon Sep 17 00:00:00 2001 From: Harry Date: Mon, 11 Mar 2019 01:10:39 +0000 Subject: [PATCH 03/39] repository for batches [chapter_02_repository_ends] --- repository.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 repository.py diff --git a/repository.py b/repository.py new file mode 100644 index 00000000..73b85e31 --- /dev/null +++ b/repository.py @@ -0,0 +1,26 @@ +import abc +import model + + +class AbstractRepository(abc.ABC): + @abc.abstractmethod + def add(self, batch: model.Batch): + raise NotImplementedError + + @abc.abstractmethod + def get(self, reference) -> model.Batch: + raise NotImplementedError + + +class SqlAlchemyRepository(AbstractRepository): + def __init__(self, session): + self.session = session + + def add(self, batch): + self.session.add(batch) + + def get(self, reference): + return self.session.query(model.Batch).filter_by(reference=reference).one() + + def list(self): + return self.session.query(model.Batch).all() From fda077db691e09e69eb54e7fbf29feb732da37b5 Mon Sep 17 00:00:00 2001 From: Harry Date: Mon, 11 Mar 2019 04:10:52 +0000 Subject: [PATCH 04/39] first api tests [first_api_test] --- config.py | 15 +++++++++++++ conftest.py | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++ test_api.py | 43 +++++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+) create mode 100644 config.py create mode 100644 test_api.py diff --git a/config.py b/config.py new file mode 100644 index 00000000..f3b55cc9 --- /dev/null +++ b/config.py @@ -0,0 +1,15 @@ +import os + + +def get_postgres_uri(): + host = os.environ.get("DB_HOST", "localhost") + port = 54321 if host == "localhost" else 5432 + password = os.environ.get("DB_PASSWORD", "abc123") + user, db_name = "allocation", "allocation" + return f"postgresql://{user}:{password}@{host}:{port}/{db_name}" + + +def get_api_url(): + host = os.environ.get("API_HOST", "localhost") + port = 5005 if host == "localhost" else 80 + return f"http://{host}:{port}" diff --git a/conftest.py b/conftest.py index 9f7b74b0..f43dd114 100644 --- a/conftest.py +++ b/conftest.py @@ -1,8 +1,13 @@ +# pylint: disable=redefined-outer-name +import time +from pathlib import Path + import pytest from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker, clear_mappers from orm import metadata, start_mappers +from config import get_postgres_uri @pytest.fixture @@ -17,3 +22,60 @@ def session(in_memory_db): start_mappers() yield sessionmaker(bind=in_memory_db)() clear_mappers() + + +@pytest.fixture(scope="session") +def postgres_db(): + engine = create_engine(get_postgres_uri()) + metadata.create_all(engine) + return engine + + +@pytest.fixture +def postgres_session(postgres_db): + start_mappers() + yield sessionmaker(bind=postgres_db)() + clear_mappers() + + +@pytest.fixture +def add_stock(postgres_session): + batches_added = set() + skus_added = set() + + def _add_stock(lines): + for ref, sku, qty, eta in lines: + postgres_session.execute( + "INSERT INTO batches (reference, sku, _purchased_quantity, eta)" + " VALUES (:ref, :sku, :qty, :eta)", + dict(ref=ref, sku=sku, qty=qty, eta=eta), + ) + [[batch_id]] = postgres_session.execute( + "SELECT id FROM batches WHERE reference=:ref AND sku=:sku", + dict(ref=ref, sku=sku), + ) + batches_added.add(batch_id) + skus_added.add(sku) + postgres_session.commit() + + yield _add_stock + + for batch_id in batches_added: + postgres_session.execute( + "DELETE FROM allocations WHERE batch_id=:batch_id", + dict(batch_id=batch_id), + ) + postgres_session.execute( + "DELETE FROM batches WHERE id=:batch_id", dict(batch_id=batch_id), + ) + for sku in skus_added: + postgres_session.execute( + "DELETE FROM order_lines WHERE sku=:sku", dict(sku=sku), + ) + postgres_session.commit() + + +@pytest.fixture +def restart_api(): + (Path(__file__).parent / "flask_app.py").touch() + time.sleep(0.3) diff --git a/test_api.py b/test_api.py new file mode 100644 index 00000000..04baaf43 --- /dev/null +++ b/test_api.py @@ -0,0 +1,43 @@ +import uuid +import pytest +import requests + +import config + + +def random_suffix(): + return uuid.uuid4().hex[:6] + + +def random_sku(name=""): + return f"sku-{name}-{random_suffix()}" + + +def random_batchref(name=""): + return f"batch-{name}-{random_suffix()}" + + +def random_orderid(name=""): + return f"order-{name}-{random_suffix()}" + + +@pytest.mark.usefixtures("restart_api") +def test_api_returns_allocation(add_stock): + sku, othersku = random_sku(), random_sku("other") + earlybatch = random_batchref(1) + laterbatch = random_batchref(2) + otherbatch = random_batchref(3) + add_stock( + [ + (laterbatch, sku, 100, "2011-01-02"), + (earlybatch, sku, 100, "2011-01-01"), + (otherbatch, othersku, 100, None), + ] + ) + data = {"orderid": random_orderid(), "sku": sku, "qty": 3} + url = config.get_api_url() + + r = requests.post(f"{url}/allocate", json=data) + + assert r.status_code == 201 + assert r.json()["batchref"] == earlybatch From 15902a6f69b914015c5d5a4b90cc4d04c5e046d7 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 19 Mar 2019 11:30:31 +0000 Subject: [PATCH 05/39] all the dockerfile gubbins --- Dockerfile | 10 ++++++++++ Makefile | 21 ++++++++++++++++++--- docker-compose.yml | 26 ++++++++++++++++++++++++++ requirements.txt | 5 +++++ 4 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 Dockerfile create mode 100644 docker-compose.yml create mode 100644 requirements.txt diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 00000000..901a93fc --- /dev/null +++ b/Dockerfile @@ -0,0 +1,10 @@ +FROM python:3.9-slim-buster + +COPY requirements.txt /tmp +RUN pip install -r /tmp/requirements.txt + +RUN mkdir -p /code +COPY *.py /code/ +WORKDIR /code +ENV FLASK_APP=flask_app.py FLASK_DEBUG=1 PYTHONUNBUFFERED=1 +CMD flask run --host=0.0.0.0 --port=80 diff --git a/Makefile b/Makefile index 77cbd229..1e248187 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,23 @@ +# these will speed up builds, for docker-compose >= 1.25 +export COMPOSE_DOCKER_CLI_BUILD=1 +export DOCKER_BUILDKIT=1 + +all: down build up test + +build: + docker-compose build + +up: + docker-compose up -d app + +down: + docker-compose down + +logs: + docker-compose logs app | tail -100 + test: pytest --tb=short -watch-tests: - ls *.py | entr pytest --tb=short - black: black -l 86 $$(find * -name '*.py') diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 00000000..313cf946 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,26 @@ +version: "3" +services: + + app: + build: + context: . + dockerfile: Dockerfile + depends_on: + - postgres + environment: + - DB_HOST=postgres + - DB_PASSWORD=abc123 + volumes: + - ./:/code + ports: + - "5005:80" + + + postgres: + image: postgres:9.6 + environment: + - POSTGRES_USER=allocation + - POSTGRES_PASSWORD=abc123 + ports: + - "54321:5432" + diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 00000000..01a974f5 --- /dev/null +++ b/requirements.txt @@ -0,0 +1,5 @@ +pytest +sqlalchemy +flask +psycopg2-binary +requests From 8fc086ef836fd5fcf726d37e285e0e9d4ace49f4 Mon Sep 17 00:00:00 2001 From: Harry Date: Mon, 11 Mar 2019 04:21:51 +0000 Subject: [PATCH 06/39] first cut of flask app [first_cut_flask_app] --- flask_app.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 flask_app.py diff --git a/flask_app.py b/flask_app.py new file mode 100644 index 00000000..21be2902 --- /dev/null +++ b/flask_app.py @@ -0,0 +1,26 @@ +from flask import Flask, request +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +import config +import model +import orm +import repository + + +orm.start_mappers() +get_session = sessionmaker(bind=create_engine(config.get_postgres_uri())) +app = Flask(__name__) + + +@app.route("/allocate", methods=["POST"]) +def allocate_endpoint(): + session = get_session() + batches = repository.SqlAlchemyRepository(session).list() + line = model.OrderLine( + request.json["orderid"], request.json["sku"], request.json["qty"], + ) + + batchref = model.allocate(line, batches) + + return {"batchref": batchref}, 201 From 158e760fed9a7d530d34b3acf4e4159477eac73d Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 19 Mar 2019 11:58:59 +0000 Subject: [PATCH 07/39] test persistence by double-allocating. [second_api_test] --- test_api.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test_api.py b/test_api.py index 04baaf43..9fd51a34 100644 --- a/test_api.py +++ b/test_api.py @@ -41,3 +41,26 @@ def test_api_returns_allocation(add_stock): assert r.status_code == 201 assert r.json()["batchref"] == earlybatch + + +@pytest.mark.usefixtures("restart_api") +def test_allocations_are_persisted(add_stock): + sku = random_sku() + batch1, batch2 = random_batchref(1), random_batchref(2) + order1, order2 = random_orderid(1), random_orderid(2) + add_stock( + [(batch1, sku, 10, "2011-01-01"), (batch2, sku, 10, "2011-01-02"),] + ) + line1 = {"orderid": order1, "sku": sku, "qty": 10} + line2 = {"orderid": order2, "sku": sku, "qty": 10} + url = config.get_api_url() + + # first order uses up all stock in batch 1 + r = requests.post(f"{url}/allocate", json=line1) + assert r.status_code == 201 + assert r.json()["batchref"] == batch1 + + # second order should go to batch 2 + r = requests.post(f"{url}/allocate", json=line2) + assert r.status_code == 201 + assert r.json()["batchref"] == batch2 From d0ee7ada3dbb034dbe5fce7a06e209486e88cccc Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 19 Mar 2019 11:59:20 +0000 Subject: [PATCH 08/39] need to commit [flask_commit] --- flask_app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/flask_app.py b/flask_app.py index 21be2902..5ae4320e 100644 --- a/flask_app.py +++ b/flask_app.py @@ -23,4 +23,5 @@ def allocate_endpoint(): batchref = model.allocate(line, batches) + session.commit() return {"batchref": batchref}, 201 From f20c4792525ee9b138bd6586eb88359d1d70a8db Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 19 Mar 2019 12:10:34 +0000 Subject: [PATCH 09/39] test some 400 error cases [test_error_cases] --- test_api.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test_api.py b/test_api.py index 9fd51a34..2061119d 100644 --- a/test_api.py +++ b/test_api.py @@ -64,3 +64,26 @@ def test_allocations_are_persisted(add_stock): r = requests.post(f"{url}/allocate", json=line2) assert r.status_code == 201 assert r.json()["batchref"] == batch2 + + +@pytest.mark.usefixtures("restart_api") +def test_400_message_for_out_of_stock(add_stock): + sku, smalL_batch, large_order = random_sku(), random_batchref(), random_orderid() + add_stock( + [(smalL_batch, sku, 10, "2011-01-01"),] + ) + data = {"orderid": large_order, "sku": sku, "qty": 20} + url = config.get_api_url() + r = requests.post(f"{url}/allocate", json=data) + assert r.status_code == 400 + assert r.json()["message"] == f"Out of stock for sku {sku}" + + +@pytest.mark.usefixtures("restart_api") +def test_400_message_for_invalid_sku(): + unknown_sku, orderid = random_sku(), random_orderid() + data = {"orderid": orderid, "sku": unknown_sku, "qty": 20} + url = config.get_api_url() + r = requests.post(f"{url}/allocate", json=data) + assert r.status_code == 400 + assert r.json()["message"] == f"Invalid sku {unknown_sku}" From 80dece9d1736a3abf2ae605af25606c85b39bccb Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 19 Mar 2019 12:17:15 +0000 Subject: [PATCH 10/39] flask now does error handling [flask_error_handling] --- flask_app.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/flask_app.py b/flask_app.py index 5ae4320e..3ffa1f4f 100644 --- a/flask_app.py +++ b/flask_app.py @@ -13,6 +13,10 @@ app = Flask(__name__) +def is_valid_sku(sku, batches): + return sku in {b.sku for b in batches} + + @app.route("/allocate", methods=["POST"]) def allocate_endpoint(): session = get_session() @@ -21,7 +25,13 @@ def allocate_endpoint(): request.json["orderid"], request.json["sku"], request.json["qty"], ) - batchref = model.allocate(line, batches) + if not is_valid_sku(line.sku, batches): + return {"message": f"Invalid sku {line.sku}"}, 400 + + try: + batchref = model.allocate(line, batches) + except model.OutOfStock as e: + return {"message": str(e)}, 400 session.commit() return {"batchref": batchref}, 201 From a1903fad959426a0d7bee8c186b0bfb73a2755d2 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 19 Mar 2019 13:44:27 +0000 Subject: [PATCH 11/39] first tests for the services layer [first_services_tests] --- test_services.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 test_services.py diff --git a/test_services.py b/test_services.py new file mode 100644 index 00000000..92dcd517 --- /dev/null +++ b/test_services.py @@ -0,0 +1,22 @@ +import pytest + +import model +import services + + +def test_returns_allocation(): + line = model.OrderLine("o1", "COMPLICATED-LAMP", 10) + batch = model.Batch("b1", "COMPLICATED-LAMP", 100, eta=None) + repo = FakeRepository([batch]) + + result = services.allocate(line, repo, FakeSession()) + assert result == "b1" + + +def test_error_for_invalid_sku(): + line = model.OrderLine("o1", "NONEXISTENTSKU", 10) + batch = model.Batch("b1", "AREALSKU", 100, eta=None) + repo = FakeRepository([batch]) + + with pytest.raises(services.InvalidSku, match="Invalid sku NONEXISTENTSKU"): + services.allocate(line, repo, FakeSession()) From 9fe9a3de7ea0afabf652bacf035e86b39ac02d6c Mon Sep 17 00:00:00 2001 From: Harry Date: Fri, 3 Jan 2020 00:54:06 +0000 Subject: [PATCH 12/39] FakeRepository [fake_repo] --- test_services.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test_services.py b/test_services.py index 92dcd517..fb15f428 100644 --- a/test_services.py +++ b/test_services.py @@ -1,9 +1,23 @@ import pytest - import model +import repository import services +class FakeRepository(repository.AbstractRepository): + def __init__(self, batches): + self._batches = set(batches) + + def add(self, batch): + self._batches.add(batch) + + def get(self, reference): + return next(b for b in self._batches if b.reference == reference) + + def list(self): + return list(self._batches) + + def test_returns_allocation(): line = model.OrderLine("o1", "COMPLICATED-LAMP", 10) batch = model.Batch("b1", "COMPLICATED-LAMP", 100, eta=None) From 15b9bf79d134384a1cac645eba4b055f9bf4ac1d Mon Sep 17 00:00:00 2001 From: Harry Date: Fri, 3 Jan 2020 00:54:48 +0000 Subject: [PATCH 13/39] FakeSession [fake_session] --- test_services.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test_services.py b/test_services.py index fb15f428..380f46c9 100644 --- a/test_services.py +++ b/test_services.py @@ -18,6 +18,13 @@ def list(self): return list(self._batches) +class FakeSession: + committed = False + + def commit(self): + self.committed = True + + def test_returns_allocation(): line = model.OrderLine("o1", "COMPLICATED-LAMP", 10) batch = model.Batch("b1", "COMPLICATED-LAMP", 100, eta=None) From eded7ebf6cc5f0fb05dc547af6daee6549e171ca Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 23 Apr 2019 12:52:08 +0100 Subject: [PATCH 14/39] test commmits [second_services_test] --- test_services.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test_services.py b/test_services.py index 380f46c9..abb13a1e 100644 --- a/test_services.py +++ b/test_services.py @@ -41,3 +41,13 @@ def test_error_for_invalid_sku(): with pytest.raises(services.InvalidSku, match="Invalid sku NONEXISTENTSKU"): services.allocate(line, repo, FakeSession()) + + +def test_commits(): + line = model.OrderLine("o1", "OMINOUS-MIRROR", 10) + batch = model.Batch("b1", "OMINOUS-MIRROR", 100, eta=None) + repo = FakeRepository([batch]) + session = FakeSession() + + services.allocate(line, repo, session) + assert session.committed is True From d1e2e6e59fb8fe4020bec8425bdbd30b9648ff08 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 19 Mar 2019 13:44:44 +0000 Subject: [PATCH 15/39] services layer with valid-sku check [service_function] --- services.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 services.py diff --git a/services.py b/services.py new file mode 100644 index 00000000..cb007393 --- /dev/null +++ b/services.py @@ -0,0 +1,22 @@ +from __future__ import annotations + +import model +from model import OrderLine +from repository import AbstractRepository + + +class InvalidSku(Exception): + pass + + +def is_valid_sku(sku, batches): + return sku in {b.sku for b in batches} + + +def allocate(line: OrderLine, repo: AbstractRepository, session) -> str: + batches = repo.list() + if not is_valid_sku(line.sku, batches): + raise InvalidSku(f"Invalid sku {line.sku}") + batchref = model.allocate(line, batches) + session.commit() + return batchref From cf2f52ba9696b7fa2336b46a55c6f11f180fa203 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 19 Mar 2019 14:33:19 +0000 Subject: [PATCH 16/39] modify flask app to use service layer [flask_app_using_service_layer] --- flask_app.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/flask_app.py b/flask_app.py index 3ffa1f4f..3412cbe9 100644 --- a/flask_app.py +++ b/flask_app.py @@ -6,6 +6,7 @@ import model import orm import repository +import services orm.start_mappers() @@ -13,25 +14,17 @@ app = Flask(__name__) -def is_valid_sku(sku, batches): - return sku in {b.sku for b in batches} - - @app.route("/allocate", methods=["POST"]) def allocate_endpoint(): session = get_session() - batches = repository.SqlAlchemyRepository(session).list() + repo = repository.SqlAlchemyRepository(session) line = model.OrderLine( request.json["orderid"], request.json["sku"], request.json["qty"], ) - if not is_valid_sku(line.sku, batches): - return {"message": f"Invalid sku {line.sku}"}, 400 - try: - batchref = model.allocate(line, batches) - except model.OutOfStock as e: + batchref = services.allocate(line, repo, session) + except (model.OutOfStock, services.InvalidSku) as e: return {"message": str(e)}, 400 - session.commit() return {"batchref": batchref}, 201 From b537364dcb7b22e0348fda4b66219e3bfe4d259b Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 19 Mar 2019 14:48:36 +0000 Subject: [PATCH 17/39] strip out unecessary tests from e2e layer [fewer_e2e_tests] --- test_api.py | 40 ++-------------------------------------- 1 file changed, 2 insertions(+), 38 deletions(-) diff --git a/test_api.py b/test_api.py index 2061119d..5386ab88 100644 --- a/test_api.py +++ b/test_api.py @@ -22,7 +22,7 @@ def random_orderid(name=""): @pytest.mark.usefixtures("restart_api") -def test_api_returns_allocation(add_stock): +def test_happy_path_returns_201_and_allocated_batch(add_stock): sku, othersku = random_sku(), random_sku("other") earlybatch = random_batchref(1) laterbatch = random_batchref(2) @@ -44,43 +44,7 @@ def test_api_returns_allocation(add_stock): @pytest.mark.usefixtures("restart_api") -def test_allocations_are_persisted(add_stock): - sku = random_sku() - batch1, batch2 = random_batchref(1), random_batchref(2) - order1, order2 = random_orderid(1), random_orderid(2) - add_stock( - [(batch1, sku, 10, "2011-01-01"), (batch2, sku, 10, "2011-01-02"),] - ) - line1 = {"orderid": order1, "sku": sku, "qty": 10} - line2 = {"orderid": order2, "sku": sku, "qty": 10} - url = config.get_api_url() - - # first order uses up all stock in batch 1 - r = requests.post(f"{url}/allocate", json=line1) - assert r.status_code == 201 - assert r.json()["batchref"] == batch1 - - # second order should go to batch 2 - r = requests.post(f"{url}/allocate", json=line2) - assert r.status_code == 201 - assert r.json()["batchref"] == batch2 - - -@pytest.mark.usefixtures("restart_api") -def test_400_message_for_out_of_stock(add_stock): - sku, smalL_batch, large_order = random_sku(), random_batchref(), random_orderid() - add_stock( - [(smalL_batch, sku, 10, "2011-01-01"),] - ) - data = {"orderid": large_order, "sku": sku, "qty": 20} - url = config.get_api_url() - r = requests.post(f"{url}/allocate", json=data) - assert r.status_code == 400 - assert r.json()["message"] == f"Out of stock for sku {sku}" - - -@pytest.mark.usefixtures("restart_api") -def test_400_message_for_invalid_sku(): +def test_unhappy_path_returns_400_and_error_message(): unknown_sku, orderid = random_sku(), random_orderid() data = {"orderid": orderid, "sku": unknown_sku, "qty": 20} url = config.get_api_url() From 952a3d2f53ec56ca320b8415097350799f38de10 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 26 Mar 2019 21:21:17 +0000 Subject: [PATCH 18/39] fix conftest waits and travis config [chapter_04_service_layer_ends] --- .travis.yml | 4 ++-- conftest.py | 32 +++++++++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8a74f94b..f601680f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,10 +3,10 @@ language: python python: 3.8 install: -- pip3 install sqlalchemy +- pip3 install -r requirements.txt script: -- make test +- make all branches: except: diff --git a/conftest.py b/conftest.py index f43dd114..da82d88a 100644 --- a/conftest.py +++ b/conftest.py @@ -3,11 +3,14 @@ from pathlib import Path import pytest +import requests +from requests.exceptions import ConnectionError +from sqlalchemy.exc import OperationalError from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker, clear_mappers from orm import metadata, start_mappers -from config import get_postgres_uri +import config @pytest.fixture @@ -24,9 +27,31 @@ def session(in_memory_db): clear_mappers() +def wait_for_postgres_to_come_up(engine): + deadline = time.time() + 10 + while time.time() < deadline: + try: + return engine.connect() + except OperationalError: + time.sleep(0.5) + pytest.fail("Postgres never came up") + + +def wait_for_webapp_to_come_up(): + deadline = time.time() + 10 + url = config.get_api_url() + while time.time() < deadline: + try: + return requests.get(url) + except ConnectionError: + time.sleep(0.5) + pytest.fail("API never came up") + + @pytest.fixture(scope="session") def postgres_db(): - engine = create_engine(get_postgres_uri()) + engine = create_engine(config.get_postgres_uri()) + wait_for_postgres_to_come_up(engine) metadata.create_all(engine) return engine @@ -78,4 +103,5 @@ def _add_stock(lines): @pytest.fixture def restart_api(): (Path(__file__).parent / "flask_app.py").touch() - time.sleep(0.3) + time.sleep(0.5) + wait_for_webapp_to_come_up() From bdf8fe94baffb69097e727135b0c6ce77ac87674 Mon Sep 17 00:00:00 2001 From: Harry Date: Mon, 23 Dec 2019 16:37:06 +0000 Subject: [PATCH 19/39] move to a more nested folder structure --- orm.py => adapters/orm.py | 0 repository.py => adapters/repository.py | 0 model.py => domain/model.py | 0 flask_app.py => entrypoints/flask_app.py | 0 services.py => service_layer/services.py | 0 5 files changed, 0 insertions(+), 0 deletions(-) rename orm.py => adapters/orm.py (100%) rename repository.py => adapters/repository.py (100%) rename model.py => domain/model.py (100%) rename flask_app.py => entrypoints/flask_app.py (100%) rename services.py => service_layer/services.py (100%) diff --git a/orm.py b/adapters/orm.py similarity index 100% rename from orm.py rename to adapters/orm.py diff --git a/repository.py b/adapters/repository.py similarity index 100% rename from repository.py rename to adapters/repository.py diff --git a/model.py b/domain/model.py similarity index 100% rename from model.py rename to domain/model.py diff --git a/flask_app.py b/entrypoints/flask_app.py similarity index 100% rename from flask_app.py rename to entrypoints/flask_app.py diff --git a/services.py b/service_layer/services.py similarity index 100% rename from services.py rename to service_layer/services.py From 1bb572af7f8fce0a5236fc005c4d5bc305464c94 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 31 Dec 2019 13:18:04 +0000 Subject: [PATCH 20/39] nest the tests too --- test_api.py => tests/e2e/test_api.py | 0 test_orm.py => tests/integration/test_orm.py | 0 test_repository.py => tests/integration/test_repository.py | 0 test_allocate.py => tests/unit/test_allocate.py | 0 test_batches.py => tests/unit/test_batches.py | 0 test_services.py => tests/unit/test_services.py | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename test_api.py => tests/e2e/test_api.py (100%) rename test_orm.py => tests/integration/test_orm.py (100%) rename test_repository.py => tests/integration/test_repository.py (100%) rename test_allocate.py => tests/unit/test_allocate.py (100%) rename test_batches.py => tests/unit/test_batches.py (100%) rename test_services.py => tests/unit/test_services.py (100%) diff --git a/test_api.py b/tests/e2e/test_api.py similarity index 100% rename from test_api.py rename to tests/e2e/test_api.py diff --git a/test_orm.py b/tests/integration/test_orm.py similarity index 100% rename from test_orm.py rename to tests/integration/test_orm.py diff --git a/test_repository.py b/tests/integration/test_repository.py similarity index 100% rename from test_repository.py rename to tests/integration/test_repository.py diff --git a/test_allocate.py b/tests/unit/test_allocate.py similarity index 100% rename from test_allocate.py rename to tests/unit/test_allocate.py diff --git a/test_batches.py b/tests/unit/test_batches.py similarity index 100% rename from test_batches.py rename to tests/unit/test_batches.py diff --git a/test_services.py b/tests/unit/test_services.py similarity index 100% rename from test_services.py rename to tests/unit/test_services.py From db8921818a5aaeb3495f0405fa089648ade4b306 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 31 Dec 2019 12:44:27 +0000 Subject: [PATCH 21/39] get all tests passing --- Dockerfile | 2 +- adapters/__init__.py | 0 adapters/orm.py | 2 +- adapters/repository.py | 2 +- domain/__init__.py | 0 entrypoints/__init__.py | 0 entrypoints/flask_app.py | 7 +++---- service_layer/__init__.py | 0 service_layer/services.py | 6 +++--- tests/__init__.py | 0 conftest.py => tests/conftest.py | 4 ++-- tests/integration/test_orm.py | 2 +- tests/integration/test_repository.py | 4 ++-- tests/unit/test_allocate.py | 2 +- tests/unit/test_batches.py | 2 +- tests/unit/test_services.py | 6 +++--- 16 files changed, 19 insertions(+), 20 deletions(-) create mode 100644 adapters/__init__.py create mode 100644 domain/__init__.py create mode 100644 entrypoints/__init__.py create mode 100644 service_layer/__init__.py create mode 100644 tests/__init__.py rename conftest.py => tests/conftest.py (95%) diff --git a/Dockerfile b/Dockerfile index 901a93fc..d4803afb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,5 +6,5 @@ RUN pip install -r /tmp/requirements.txt RUN mkdir -p /code COPY *.py /code/ WORKDIR /code -ENV FLASK_APP=flask_app.py FLASK_DEBUG=1 PYTHONUNBUFFERED=1 +ENV FLASK_APP=entrypoints/flask_app.py FLASK_DEBUG=1 PYTHONUNBUFFERED=1 CMD flask run --host=0.0.0.0 --port=80 diff --git a/adapters/__init__.py b/adapters/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/adapters/orm.py b/adapters/orm.py index 3e6b4a70..8499c2dc 100644 --- a/adapters/orm.py +++ b/adapters/orm.py @@ -1,7 +1,7 @@ from sqlalchemy import Table, MetaData, Column, Integer, String, Date, ForeignKey from sqlalchemy.orm import mapper, relationship -import model +from domain import model metadata = MetaData() diff --git a/adapters/repository.py b/adapters/repository.py index 73b85e31..7e7434b9 100644 --- a/adapters/repository.py +++ b/adapters/repository.py @@ -1,5 +1,5 @@ import abc -import model +from domain import model class AbstractRepository(abc.ABC): diff --git a/domain/__init__.py b/domain/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/entrypoints/__init__.py b/entrypoints/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/entrypoints/flask_app.py b/entrypoints/flask_app.py index 3412cbe9..62e40d44 100644 --- a/entrypoints/flask_app.py +++ b/entrypoints/flask_app.py @@ -3,10 +3,9 @@ from sqlalchemy.orm import sessionmaker import config -import model -import orm -import repository -import services +from domain import model +from adapters import orm, repository +from service_layer import services orm.start_mappers() diff --git a/service_layer/__init__.py b/service_layer/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/service_layer/services.py b/service_layer/services.py index cb007393..7c7ff8f1 100644 --- a/service_layer/services.py +++ b/service_layer/services.py @@ -1,8 +1,8 @@ from __future__ import annotations -import model -from model import OrderLine -from repository import AbstractRepository +from domain import model +from domain.model import OrderLine +from adapters.repository import AbstractRepository class InvalidSku(Exception): diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/conftest.py b/tests/conftest.py similarity index 95% rename from conftest.py rename to tests/conftest.py index da82d88a..11e66b35 100644 --- a/conftest.py +++ b/tests/conftest.py @@ -9,7 +9,7 @@ from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker, clear_mappers -from orm import metadata, start_mappers +from adapters.orm import metadata, start_mappers import config @@ -102,6 +102,6 @@ def _add_stock(lines): @pytest.fixture def restart_api(): - (Path(__file__).parent / "flask_app.py").touch() + (Path(__file__).parent / "../entrypoints/flask_app.py").touch() time.sleep(0.5) wait_for_webapp_to_come_up() diff --git a/tests/integration/test_orm.py b/tests/integration/test_orm.py index 07a63491..3a551977 100644 --- a/tests/integration/test_orm.py +++ b/tests/integration/test_orm.py @@ -1,4 +1,4 @@ -import model +from domain import model from datetime import date diff --git a/tests/integration/test_repository.py b/tests/integration/test_repository.py index 7c668a39..3fa66d9f 100644 --- a/tests/integration/test_repository.py +++ b/tests/integration/test_repository.py @@ -1,6 +1,6 @@ # pylint: disable=protected-access -import model -import repository +from domain import model +from adapters import repository def test_repository_can_save_a_batch(session): diff --git a/tests/unit/test_allocate.py b/tests/unit/test_allocate.py index 7e189307..4e434e28 100644 --- a/tests/unit/test_allocate.py +++ b/tests/unit/test_allocate.py @@ -1,6 +1,6 @@ from datetime import date, timedelta import pytest -from model import allocate, OrderLine, Batch, OutOfStock +from domain.model import allocate, OrderLine, Batch, OutOfStock today = date.today() tomorrow = today + timedelta(days=1) diff --git a/tests/unit/test_batches.py b/tests/unit/test_batches.py index 62288330..7feda3d7 100644 --- a/tests/unit/test_batches.py +++ b/tests/unit/test_batches.py @@ -1,5 +1,5 @@ from datetime import date -from model import Batch, OrderLine +from domain.model import Batch, OrderLine def test_allocating_to_a_batch_reduces_the_available_quantity(): diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index abb13a1e..64ef3158 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -1,7 +1,7 @@ import pytest -import model -import repository -import services +from domain import model +from adapters import repository +from service_layer import services class FakeRepository(repository.AbstractRepository): From c5822aa764b767efabc4975060b9533cef094fbf Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 23 Apr 2019 12:11:47 +0100 Subject: [PATCH 22/39] rewrite service layer to take primitives [service_takes_primitives] --- service_layer/services.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/service_layer/services.py b/service_layer/services.py index 7c7ff8f1..464cdb7c 100644 --- a/service_layer/services.py +++ b/service_layer/services.py @@ -13,7 +13,11 @@ def is_valid_sku(sku, batches): return sku in {b.sku for b in batches} -def allocate(line: OrderLine, repo: AbstractRepository, session) -> str: +def allocate( + orderid: str, sku: str, qty: int, + repo: AbstractRepository, session +) -> str: + line = OrderLine(orderid, sku, qty) batches = repo.list() if not is_valid_sku(line.sku, batches): raise InvalidSku(f"Invalid sku {line.sku}") From f341bee916afc28fbc23d50be2d1979dadc9978d Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 23 Apr 2019 12:12:16 +0100 Subject: [PATCH 23/39] services tests partially converted to primitives [tests_call_with_primitives] --- tests/unit/test_services.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index 64ef3158..9c114ed3 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -26,28 +26,25 @@ def commit(self): def test_returns_allocation(): - line = model.OrderLine("o1", "COMPLICATED-LAMP", 10) - batch = model.Batch("b1", "COMPLICATED-LAMP", 100, eta=None) + batch = model.Batch("batch1", "COMPLICATED-LAMP", 100, eta=None) repo = FakeRepository([batch]) - result = services.allocate(line, repo, FakeSession()) - assert result == "b1" + result = services.allocate("o1", "COMPLICATED-LAMP", 10, repo, FakeSession()) + assert result == "batch1" def test_error_for_invalid_sku(): - line = model.OrderLine("o1", "NONEXISTENTSKU", 10) batch = model.Batch("b1", "AREALSKU", 100, eta=None) repo = FakeRepository([batch]) with pytest.raises(services.InvalidSku, match="Invalid sku NONEXISTENTSKU"): - services.allocate(line, repo, FakeSession()) + services.allocate("o1", "NONEXISTENTSKU", 10, repo, FakeSession()) def test_commits(): - line = model.OrderLine("o1", "OMINOUS-MIRROR", 10) batch = model.Batch("b1", "OMINOUS-MIRROR", 100, eta=None) repo = FakeRepository([batch]) session = FakeSession() - services.allocate(line, repo, session) + services.allocate("o1", "OMINOUS-MIRROR", 10, repo, session) assert session.committed is True From 1e1f2381e348b29e04a9812114feb8ca6f92bbc9 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 23 Apr 2019 12:59:52 +0100 Subject: [PATCH 24/39] fixture function for batches [services_factory_function] --- tests/unit/test_services.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index 9c114ed3..0d404f4a 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -5,6 +5,10 @@ class FakeRepository(repository.AbstractRepository): + @staticmethod + def for_batch(ref, sku, qty, eta=None): + return FakeRepository([model.Batch(ref, sku, qty, eta),]) + def __init__(self, batches): self._batches = set(batches) @@ -26,25 +30,19 @@ def commit(self): def test_returns_allocation(): - batch = model.Batch("batch1", "COMPLICATED-LAMP", 100, eta=None) - repo = FakeRepository([batch]) - + repo = FakeRepository.for_batch("batch1", "COMPLICATED-LAMP", 100, eta=None) result = services.allocate("o1", "COMPLICATED-LAMP", 10, repo, FakeSession()) assert result == "batch1" def test_error_for_invalid_sku(): - batch = model.Batch("b1", "AREALSKU", 100, eta=None) - repo = FakeRepository([batch]) - + repo = FakeRepository.for_batch("b1", "AREALSKU", 100, eta=None) with pytest.raises(services.InvalidSku, match="Invalid sku NONEXISTENTSKU"): services.allocate("o1", "NONEXISTENTSKU", 10, repo, FakeSession()) def test_commits(): - batch = model.Batch("b1", "OMINOUS-MIRROR", 100, eta=None) - repo = FakeRepository([batch]) + repo = FakeRepository.for_batch("b1", "OMINOUS-MIRROR", 100, eta=None) session = FakeSession() - services.allocate("o1", "OMINOUS-MIRROR", 10, repo, session) assert session.committed is True From 5c953dada5e1a136ec21f8eaf15a588e32d4b858 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 23 Apr 2019 12:37:17 +0100 Subject: [PATCH 25/39] new service to add a batch [add_batch_service] --- service_layer/services.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/service_layer/services.py b/service_layer/services.py index 464cdb7c..36c7bd9a 100644 --- a/service_layer/services.py +++ b/service_layer/services.py @@ -1,4 +1,6 @@ from __future__ import annotations +from typing import Optional +from datetime import date from domain import model from domain.model import OrderLine @@ -13,6 +15,14 @@ def is_valid_sku(sku, batches): return sku in {b.sku for b in batches} +def add_batch( + ref: str, sku: str, qty: int, eta: Optional[date], + repo: AbstractRepository, session, +) -> None: + repo.add(model.Batch(ref, sku, qty, eta)) + session.commit() + + def allocate( orderid: str, sku: str, qty: int, repo: AbstractRepository, session From 262eec06146958ed6b8a47d700f9c32fd92638ce Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 23 Apr 2019 12:37:39 +0100 Subject: [PATCH 26/39] service-layer test for add batch [test_add_batch] --- tests/unit/test_services.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index 0d404f4a..f5574ef8 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -29,6 +29,13 @@ def commit(self): self.committed = True +def test_add_batch(): + repo, session = FakeRepository([]), FakeSession() + services.add_batch("b1", "CRUNCHY-ARMCHAIR", 100, None, repo, session) + assert repo.get("b1") is not None + assert session.committed + + def test_returns_allocation(): repo = FakeRepository.for_batch("batch1", "COMPLICATED-LAMP", 100, eta=None) result = services.allocate("o1", "COMPLICATED-LAMP", 10, repo, FakeSession()) From c8fbb60a7bbc918e5ea9a3f7ea70d35118c41620 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 23 Apr 2019 12:37:57 +0100 Subject: [PATCH 27/39] all service-layer tests now services [services_tests_all_services] --- tests/unit/test_services.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index f5574ef8..7d9d8738 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -1,14 +1,9 @@ import pytest -from domain import model from adapters import repository from service_layer import services class FakeRepository(repository.AbstractRepository): - @staticmethod - def for_batch(ref, sku, qty, eta=None): - return FakeRepository([model.Batch(ref, sku, qty, eta),]) - def __init__(self, batches): self._batches = set(batches) @@ -36,20 +31,24 @@ def test_add_batch(): assert session.committed -def test_returns_allocation(): - repo = FakeRepository.for_batch("batch1", "COMPLICATED-LAMP", 100, eta=None) - result = services.allocate("o1", "COMPLICATED-LAMP", 10, repo, FakeSession()) +def test_allocate_returns_allocation(): + repo, session = FakeRepository([]), FakeSession() + services.add_batch("batch1", "COMPLICATED-LAMP", 100, None, repo, session) + result = services.allocate("o1", "COMPLICATED-LAMP", 10, repo, session) assert result == "batch1" -def test_error_for_invalid_sku(): - repo = FakeRepository.for_batch("b1", "AREALSKU", 100, eta=None) +def test_allocate_errors_for_invalid_sku(): + repo, session = FakeRepository([]), FakeSession() + services.add_batch("b1", "AREALSKU", 100, None, repo, session) + with pytest.raises(services.InvalidSku, match="Invalid sku NONEXISTENTSKU"): services.allocate("o1", "NONEXISTENTSKU", 10, repo, FakeSession()) def test_commits(): - repo = FakeRepository.for_batch("b1", "OMINOUS-MIRROR", 100, eta=None) + repo, session = FakeRepository([]), FakeSession() session = FakeSession() + services.add_batch("b1", "OMINOUS-MIRROR", 100, None, repo, session) services.allocate("o1", "OMINOUS-MIRROR", 10, repo, session) assert session.committed is True From 96301d2ca4aeb0f884906a500d2044821839dab5 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 23 Apr 2019 12:44:09 +0100 Subject: [PATCH 28/39] modify flask app to use new service layer api [api_uses_modified_service] --- entrypoints/flask_app.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/entrypoints/flask_app.py b/entrypoints/flask_app.py index 62e40d44..aace66e7 100644 --- a/entrypoints/flask_app.py +++ b/entrypoints/flask_app.py @@ -17,12 +17,14 @@ def allocate_endpoint(): session = get_session() repo = repository.SqlAlchemyRepository(session) - line = model.OrderLine( - request.json["orderid"], request.json["sku"], request.json["qty"], - ) - try: - batchref = services.allocate(line, repo, session) + batchref = services.allocate( + request.json["orderid"], + request.json["sku"], + request.json["qty"], + repo, + session, + ) except (model.OutOfStock, services.InvalidSku) as e: return {"message": str(e)}, 400 From 6b404b52471786fccd439875e311985cecf35ea8 Mon Sep 17 00:00:00 2001 From: Harry Date: Wed, 24 Apr 2019 12:21:59 +0100 Subject: [PATCH 29/39] add api endpoint for add_batch [api_for_add_batch] --- entrypoints/flask_app.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/entrypoints/flask_app.py b/entrypoints/flask_app.py index aace66e7..15ba414b 100644 --- a/entrypoints/flask_app.py +++ b/entrypoints/flask_app.py @@ -1,3 +1,4 @@ +from datetime import datetime from flask import Flask, request from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker @@ -7,12 +8,29 @@ from adapters import orm, repository from service_layer import services - orm.start_mappers() get_session = sessionmaker(bind=create_engine(config.get_postgres_uri())) app = Flask(__name__) +@app.route("/add_batch", methods=["POST"]) +def add_batch(): + session = get_session() + repo = repository.SqlAlchemyRepository(session) + eta = request.json["eta"] + if eta is not None: + eta = datetime.fromisoformat(eta).date() + services.add_batch( + request.json["ref"], + request.json["sku"], + request.json["qty"], + eta, + repo, + session, + ) + return "OK", 201 + + @app.route("/allocate", methods=["POST"]) def allocate_endpoint(): session = get_session() From fd45a6ff9e0ef20fc4dc698d169139f4f830c775 Mon Sep 17 00:00:00 2001 From: Harry Date: Wed, 24 Apr 2019 12:22:34 +0100 Subject: [PATCH 30/39] api tests no longer need hardcoded sql fixture [chapter_05_high_gear_low_gear_ends] --- tests/conftest.py | 37 ------------------------------------- tests/e2e/test_api.py | 24 +++++++++++++++--------- 2 files changed, 15 insertions(+), 46 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 11e66b35..447164f4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -63,43 +63,6 @@ def postgres_session(postgres_db): clear_mappers() -@pytest.fixture -def add_stock(postgres_session): - batches_added = set() - skus_added = set() - - def _add_stock(lines): - for ref, sku, qty, eta in lines: - postgres_session.execute( - "INSERT INTO batches (reference, sku, _purchased_quantity, eta)" - " VALUES (:ref, :sku, :qty, :eta)", - dict(ref=ref, sku=sku, qty=qty, eta=eta), - ) - [[batch_id]] = postgres_session.execute( - "SELECT id FROM batches WHERE reference=:ref AND sku=:sku", - dict(ref=ref, sku=sku), - ) - batches_added.add(batch_id) - skus_added.add(sku) - postgres_session.commit() - - yield _add_stock - - for batch_id in batches_added: - postgres_session.execute( - "DELETE FROM allocations WHERE batch_id=:batch_id", - dict(batch_id=batch_id), - ) - postgres_session.execute( - "DELETE FROM batches WHERE id=:batch_id", dict(batch_id=batch_id), - ) - for sku in skus_added: - postgres_session.execute( - "DELETE FROM order_lines WHERE sku=:sku", dict(sku=sku), - ) - postgres_session.commit() - - @pytest.fixture def restart_api(): (Path(__file__).parent / "../entrypoints/flask_app.py").touch() diff --git a/tests/e2e/test_api.py b/tests/e2e/test_api.py index 5386ab88..991db8db 100644 --- a/tests/e2e/test_api.py +++ b/tests/e2e/test_api.py @@ -21,28 +21,34 @@ def random_orderid(name=""): return f"order-{name}-{random_suffix()}" +def post_to_add_batch(ref, sku, qty, eta): + url = config.get_api_url() + r = requests.post( + f"{url}/add_batch", json={"ref": ref, "sku": sku, "qty": qty, "eta": eta} + ) + assert r.status_code == 201 + + +@pytest.mark.usefixtures("postgres_db") @pytest.mark.usefixtures("restart_api") -def test_happy_path_returns_201_and_allocated_batch(add_stock): +def test_happy_path_returns_201_and_allocated_batch(): sku, othersku = random_sku(), random_sku("other") earlybatch = random_batchref(1) laterbatch = random_batchref(2) otherbatch = random_batchref(3) - add_stock( - [ - (laterbatch, sku, 100, "2011-01-02"), - (earlybatch, sku, 100, "2011-01-01"), - (otherbatch, othersku, 100, None), - ] - ) + post_to_add_batch(laterbatch, sku, 100, "2011-01-02") + post_to_add_batch(earlybatch, sku, 100, "2011-01-01") + post_to_add_batch(otherbatch, othersku, 100, None) data = {"orderid": random_orderid(), "sku": sku, "qty": 3} - url = config.get_api_url() + url = config.get_api_url() r = requests.post(f"{url}/allocate", json=data) assert r.status_code == 201 assert r.json()["batchref"] == earlybatch +@pytest.mark.usefixtures("postgres_db") @pytest.mark.usefixtures("restart_api") def test_unhappy_path_returns_400_and_error_message(): unknown_sku, orderid = random_sku(), random_orderid() From d9c340c26624145c4114f7b2b6f7bf963999885f Mon Sep 17 00:00:00 2001 From: Harry Date: Wed, 27 Feb 2019 06:13:15 +0000 Subject: [PATCH 31/39] start moving files into src folder and add setup.py --- Dockerfile | 15 ++++++++++----- {adapters => src/allocation}/__init__.py | 0 {domain => src/allocation/adapters}/__init__.py | 0 {adapters => src/allocation/adapters}/orm.py | 0 .../allocation/adapters}/repository.py | 0 config.py => src/allocation/config.py | 0 .../allocation/domain}/__init__.py | 0 {domain => src/allocation/domain}/model.py | 0 .../allocation/entrypoints}/__init__.py | 0 .../allocation/entrypoints}/flask_app.py | 0 src/allocation/service_layer/__init__.py | 0 .../allocation/service_layer}/services.py | 11 +++++++---- src/setup.py | 5 +++++ 13 files changed, 22 insertions(+), 9 deletions(-) rename {adapters => src/allocation}/__init__.py (100%) rename {domain => src/allocation/adapters}/__init__.py (100%) rename {adapters => src/allocation/adapters}/orm.py (100%) rename {adapters => src/allocation/adapters}/repository.py (100%) rename config.py => src/allocation/config.py (100%) rename {entrypoints => src/allocation/domain}/__init__.py (100%) rename {domain => src/allocation/domain}/model.py (100%) rename {service_layer => src/allocation/entrypoints}/__init__.py (100%) rename {entrypoints => src/allocation/entrypoints}/flask_app.py (100%) create mode 100644 src/allocation/service_layer/__init__.py rename {service_layer => src/allocation/service_layer}/services.py (79%) create mode 100644 src/setup.py diff --git a/Dockerfile b/Dockerfile index d4803afb..73024d18 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,10 +1,15 @@ FROM python:3.9-slim-buster -COPY requirements.txt /tmp +# RUN apt install gcc libpq (no longer needed bc we use psycopg2-binary) + +COPY requirements.txt /tmp/ RUN pip install -r /tmp/requirements.txt -RUN mkdir -p /code -COPY *.py /code/ -WORKDIR /code -ENV FLASK_APP=entrypoints/flask_app.py FLASK_DEBUG=1 PYTHONUNBUFFERED=1 +RUN mkdir -p /src +COPY src/ /src/ +RUN pip install -e /src +COPY tests/ /tests/ + +WORKDIR /src +ENV FLASK_APP=allocation/entrypoints/flask_app.py FLASK_DEBUG=1 PYTHONUNBUFFERED=1 CMD flask run --host=0.0.0.0 --port=80 diff --git a/adapters/__init__.py b/src/allocation/__init__.py similarity index 100% rename from adapters/__init__.py rename to src/allocation/__init__.py diff --git a/domain/__init__.py b/src/allocation/adapters/__init__.py similarity index 100% rename from domain/__init__.py rename to src/allocation/adapters/__init__.py diff --git a/adapters/orm.py b/src/allocation/adapters/orm.py similarity index 100% rename from adapters/orm.py rename to src/allocation/adapters/orm.py diff --git a/adapters/repository.py b/src/allocation/adapters/repository.py similarity index 100% rename from adapters/repository.py rename to src/allocation/adapters/repository.py diff --git a/config.py b/src/allocation/config.py similarity index 100% rename from config.py rename to src/allocation/config.py diff --git a/entrypoints/__init__.py b/src/allocation/domain/__init__.py similarity index 100% rename from entrypoints/__init__.py rename to src/allocation/domain/__init__.py diff --git a/domain/model.py b/src/allocation/domain/model.py similarity index 100% rename from domain/model.py rename to src/allocation/domain/model.py diff --git a/service_layer/__init__.py b/src/allocation/entrypoints/__init__.py similarity index 100% rename from service_layer/__init__.py rename to src/allocation/entrypoints/__init__.py diff --git a/entrypoints/flask_app.py b/src/allocation/entrypoints/flask_app.py similarity index 100% rename from entrypoints/flask_app.py rename to src/allocation/entrypoints/flask_app.py diff --git a/src/allocation/service_layer/__init__.py b/src/allocation/service_layer/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/service_layer/services.py b/src/allocation/service_layer/services.py similarity index 79% rename from service_layer/services.py rename to src/allocation/service_layer/services.py index 36c7bd9a..d9e1108f 100644 --- a/service_layer/services.py +++ b/src/allocation/service_layer/services.py @@ -16,16 +16,19 @@ def is_valid_sku(sku, batches): def add_batch( - ref: str, sku: str, qty: int, eta: Optional[date], - repo: AbstractRepository, session, + ref: str, + sku: str, + qty: int, + eta: Optional[date], + repo: AbstractRepository, + session, ) -> None: repo.add(model.Batch(ref, sku, qty, eta)) session.commit() def allocate( - orderid: str, sku: str, qty: int, - repo: AbstractRepository, session + orderid: str, sku: str, qty: int, repo: AbstractRepository, session ) -> str: line = OrderLine(orderid, sku, qty) batches = repo.list() diff --git a/src/setup.py b/src/setup.py new file mode 100644 index 00000000..731007b0 --- /dev/null +++ b/src/setup.py @@ -0,0 +1,5 @@ +from setuptools import setup + +setup( + name="allocation", version="0.1", packages=["allocation"], +) From c843a10031e471cfd019bbf6f090afa95a251085 Mon Sep 17 00:00:00 2001 From: Harry Date: Wed, 27 Feb 2019 06:29:22 +0000 Subject: [PATCH 32/39] fix all the imports, get it all working --- docker-compose.yml | 2 +- mypy.ini | 7 ++----- src/allocation/adapters/orm.py | 2 +- src/allocation/adapters/repository.py | 2 +- src/allocation/entrypoints/flask_app.py | 8 ++++---- src/allocation/service_layer/services.py | 6 +++--- tests/conftest.py | 6 +++--- tests/e2e/test_api.py | 2 +- tests/integration/test_orm.py | 2 +- tests/integration/test_repository.py | 4 ++-- tests/mypy.ini | 6 ++++++ tests/unit/test_allocate.py | 2 +- tests/unit/test_batches.py | 2 +- tests/unit/test_services.py | 4 ++-- 14 files changed, 29 insertions(+), 26 deletions(-) create mode 100644 tests/mypy.ini diff --git a/docker-compose.yml b/docker-compose.yml index 313cf946..cc8341f9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -11,7 +11,7 @@ services: - DB_HOST=postgres - DB_PASSWORD=abc123 volumes: - - ./:/code + - ./src:/src ports: - "5005:80" diff --git a/mypy.ini b/mypy.ini index ead5ef09..65ab939b 100644 --- a/mypy.ini +++ b/mypy.ini @@ -1,9 +1,6 @@ [mypy] ignore_missing_imports = False +mypy_path = ./src -[mypy-pytest.*] +[mypy-pytest.*,sqlalchemy.*] ignore_missing_imports = True - -[mypy-sqlalchemy.*] -ignore_missing_imports = True - diff --git a/src/allocation/adapters/orm.py b/src/allocation/adapters/orm.py index 8499c2dc..444a8cbb 100644 --- a/src/allocation/adapters/orm.py +++ b/src/allocation/adapters/orm.py @@ -1,7 +1,7 @@ from sqlalchemy import Table, MetaData, Column, Integer, String, Date, ForeignKey from sqlalchemy.orm import mapper, relationship -from domain import model +from allocation.domain import model metadata = MetaData() diff --git a/src/allocation/adapters/repository.py b/src/allocation/adapters/repository.py index 7e7434b9..85e46076 100644 --- a/src/allocation/adapters/repository.py +++ b/src/allocation/adapters/repository.py @@ -1,5 +1,5 @@ import abc -from domain import model +from allocation.domain import model class AbstractRepository(abc.ABC): diff --git a/src/allocation/entrypoints/flask_app.py b/src/allocation/entrypoints/flask_app.py index 15ba414b..2164c9b6 100644 --- a/src/allocation/entrypoints/flask_app.py +++ b/src/allocation/entrypoints/flask_app.py @@ -3,10 +3,10 @@ from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker -import config -from domain import model -from adapters import orm, repository -from service_layer import services +from allocation import config +from allocation.domain import model +from allocation.adapters import orm, repository +from allocation.service_layer import services orm.start_mappers() get_session = sessionmaker(bind=create_engine(config.get_postgres_uri())) diff --git a/src/allocation/service_layer/services.py b/src/allocation/service_layer/services.py index d9e1108f..845dec39 100644 --- a/src/allocation/service_layer/services.py +++ b/src/allocation/service_layer/services.py @@ -2,9 +2,9 @@ from typing import Optional from datetime import date -from domain import model -from domain.model import OrderLine -from adapters.repository import AbstractRepository +from allocation.domain import model +from allocation.domain.model import OrderLine +from allocation.adapters.repository import AbstractRepository class InvalidSku(Exception): diff --git a/tests/conftest.py b/tests/conftest.py index 447164f4..bcf0fa4e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,8 +9,8 @@ from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker, clear_mappers -from adapters.orm import metadata, start_mappers -import config +from allocation.adapters.orm import metadata, start_mappers +from allocation import config @pytest.fixture @@ -65,6 +65,6 @@ def postgres_session(postgres_db): @pytest.fixture def restart_api(): - (Path(__file__).parent / "../entrypoints/flask_app.py").touch() + (Path(__file__).parent / "../src/allocation/entrypoints/flask_app.py").touch() time.sleep(0.5) wait_for_webapp_to_come_up() diff --git a/tests/e2e/test_api.py b/tests/e2e/test_api.py index 991db8db..29b85761 100644 --- a/tests/e2e/test_api.py +++ b/tests/e2e/test_api.py @@ -2,7 +2,7 @@ import pytest import requests -import config +from allocation import config def random_suffix(): diff --git a/tests/integration/test_orm.py b/tests/integration/test_orm.py index 3a551977..db3a7a68 100644 --- a/tests/integration/test_orm.py +++ b/tests/integration/test_orm.py @@ -1,4 +1,4 @@ -from domain import model +from allocation.domain import model from datetime import date diff --git a/tests/integration/test_repository.py b/tests/integration/test_repository.py index 3fa66d9f..0fa69ab3 100644 --- a/tests/integration/test_repository.py +++ b/tests/integration/test_repository.py @@ -1,6 +1,6 @@ # pylint: disable=protected-access -from domain import model -from adapters import repository +from allocation.domain import model +from allocation.adapters import repository def test_repository_can_save_a_batch(session): diff --git a/tests/mypy.ini b/tests/mypy.ini new file mode 100644 index 00000000..39fadd90 --- /dev/null +++ b/tests/mypy.ini @@ -0,0 +1,6 @@ +[mypy] +ignore_missing_imports = False +mypy_path = ../src + +[mypy-pytest.*,sqlalchemy.*] +ignore_missing_imports = True diff --git a/tests/unit/test_allocate.py b/tests/unit/test_allocate.py index 4e434e28..48dcfe5c 100644 --- a/tests/unit/test_allocate.py +++ b/tests/unit/test_allocate.py @@ -1,6 +1,6 @@ from datetime import date, timedelta import pytest -from domain.model import allocate, OrderLine, Batch, OutOfStock +from allocation.domain.model import allocate, OrderLine, Batch, OutOfStock today = date.today() tomorrow = today + timedelta(days=1) diff --git a/tests/unit/test_batches.py b/tests/unit/test_batches.py index 7feda3d7..8735f41e 100644 --- a/tests/unit/test_batches.py +++ b/tests/unit/test_batches.py @@ -1,5 +1,5 @@ from datetime import date -from domain.model import Batch, OrderLine +from allocation.domain.model import Batch, OrderLine def test_allocating_to_a_batch_reduces_the_available_quantity(): diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index 7d9d8738..af0ae807 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -1,6 +1,6 @@ import pytest -from adapters import repository -from service_layer import services +from allocation.adapters import repository +from allocation.service_layer import services class FakeRepository(repository.AbstractRepository): From 12d2bc2ca5f9ef644ad762eb022c6bb3a9bb6d0d Mon Sep 17 00:00:00 2001 From: Harry Date: Wed, 27 Feb 2019 06:44:34 +0000 Subject: [PATCH 33/39] get tests working in docker container --- .travis.yml | 3 --- Makefile | 17 +++++++++++++---- docker-compose.yml | 3 +++ tests/__init__.py | 0 tests/mypy.ini | 6 ------ tests/pytest.ini | 2 ++ 6 files changed, 18 insertions(+), 13 deletions(-) delete mode 100644 tests/__init__.py delete mode 100644 tests/mypy.ini create mode 100644 tests/pytest.ini diff --git a/.travis.yml b/.travis.yml index f601680f..fcd3ceea 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,9 +2,6 @@ dist: xenial language: python python: 3.8 -install: -- pip3 install -r requirements.txt - script: - make all diff --git a/Makefile b/Makefile index 1e248187..6409e955 100644 --- a/Makefile +++ b/Makefile @@ -11,13 +11,22 @@ up: docker-compose up -d app down: - docker-compose down + docker-compose down --remove-orphans + +test: up + docker-compose run --rm --no-deps --entrypoint=pytest app /tests/unit /tests/integration /tests/e2e + +unit-tests: + docker-compose run --rm --no-deps --entrypoint=pytest app /tests/unit + +integration-tests: up + docker-compose run --rm --no-deps --entrypoint=pytest app /tests/integration + +e2e-tests: up + docker-compose run --rm --no-deps --entrypoint=pytest app /tests/e2e logs: docker-compose logs app | tail -100 -test: - pytest --tb=short - black: black -l 86 $$(find * -name '*.py') diff --git a/docker-compose.yml b/docker-compose.yml index cc8341f9..039400e9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,8 +10,11 @@ services: environment: - DB_HOST=postgres - DB_PASSWORD=abc123 + - API_HOST=app + - PYTHONDONTWRITEBYTECODE=1 volumes: - ./src:/src + - ./tests:/tests ports: - "5005:80" diff --git a/tests/__init__.py b/tests/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/mypy.ini b/tests/mypy.ini deleted file mode 100644 index 39fadd90..00000000 --- a/tests/mypy.ini +++ /dev/null @@ -1,6 +0,0 @@ -[mypy] -ignore_missing_imports = False -mypy_path = ../src - -[mypy-pytest.*,sqlalchemy.*] -ignore_missing_imports = True diff --git a/tests/pytest.ini b/tests/pytest.ini new file mode 100644 index 00000000..bbd083ac --- /dev/null +++ b/tests/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +addopts = --tb=short From 28afa9a64c7b109837475ad683a99fef1f9101a2 Mon Sep 17 00:00:00 2001 From: Harry Date: Fri, 24 May 2019 00:10:48 +0100 Subject: [PATCH 34/39] make mypy slightly stricter --- mypy.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/mypy.ini b/mypy.ini index 65ab939b..62194f35 100644 --- a/mypy.ini +++ b/mypy.ini @@ -1,6 +1,7 @@ [mypy] ignore_missing_imports = False mypy_path = ./src +check_untyped_defs = True [mypy-pytest.*,sqlalchemy.*] ignore_missing_imports = True From 53ad7987bfc8b15d029c45a143743903a973215e Mon Sep 17 00:00:00 2001 From: Harry Date: Mon, 11 Mar 2019 23:08:10 +0000 Subject: [PATCH 35/39] better requirements.txt [appendix_project_structure_ends] --- requirements.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 01a974f5..8c779254 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,10 @@ -pytest +# app sqlalchemy flask psycopg2-binary + +# tests +pytest +pytest-icdiff +mypy requests From 833d48bedf2599eaaf645def0ee419864bb68fe0 Mon Sep 17 00:00:00 2001 From: Harry Date: Mon, 11 Mar 2019 23:11:44 +0000 Subject: [PATCH 36/39] basic uow test, uow and conftest.py changes --- src/allocation/service_layer/services.py | 11 ++---- src/allocation/unit_of_work.py | 50 ++++++++++++++++++++++++ tests/conftest.py | 9 ++++- tests/integration/test_uow.py | 40 +++++++++++++++++++ 4 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 src/allocation/unit_of_work.py create mode 100644 tests/integration/test_uow.py diff --git a/src/allocation/service_layer/services.py b/src/allocation/service_layer/services.py index 845dec39..051de589 100644 --- a/src/allocation/service_layer/services.py +++ b/src/allocation/service_layer/services.py @@ -16,19 +16,16 @@ def is_valid_sku(sku, batches): def add_batch( - ref: str, - sku: str, - qty: int, - eta: Optional[date], - repo: AbstractRepository, - session, + ref: str, sku: str, qty: int, eta: Optional[date], + repo: AbstractRepository, session, ) -> None: repo.add(model.Batch(ref, sku, qty, eta)) session.commit() def allocate( - orderid: str, sku: str, qty: int, repo: AbstractRepository, session + orderid: str, sku: str, qty: int, + repo: AbstractRepository, session, ) -> str: line = OrderLine(orderid, sku, qty) batches = repo.list() diff --git a/src/allocation/unit_of_work.py b/src/allocation/unit_of_work.py new file mode 100644 index 00000000..d89e1138 --- /dev/null +++ b/src/allocation/unit_of_work.py @@ -0,0 +1,50 @@ +# pylint: disable=attribute-defined-outside-init +from __future__ import annotations +import abc +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from sqlalchemy.orm.session import Session + +from allocation import config +from allocation import repository + + +class AbstractUnitOfWork(abc.ABC): + batches: repository.AbstractRepository + + def __enter__(self) -> AbstractUnitOfWork: + return self + + def __exit__(self, *args): + self.rollback() + + @abc.abstractmethod + def commit(self): + raise NotImplementedError + + @abc.abstractmethod + def rollback(self): + raise NotImplementedError + + +DEFAULT_SESSION_FACTORY = sessionmaker(bind=create_engine(config.get_postgres_uri(),)) + + +class SqlAlchemyUnitOfWork(AbstractUnitOfWork): + def __init__(self, session_factory=DEFAULT_SESSION_FACTORY): + self.session_factory = session_factory + + def __enter__(self): + self.session = self.session_factory() # type: Session + self.batches = repository.SqlAlchemyRepository(self.session) + return super().__enter__() + + def __exit__(self, *args): + super().__exit__(*args) + self.session.close() + + def commit(self): + self.session.commit() + + def rollback(self): + self.session.rollback() diff --git a/tests/conftest.py b/tests/conftest.py index bcf0fa4e..3dff0b83 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,12 +21,17 @@ def in_memory_db(): @pytest.fixture -def session(in_memory_db): +def session_factory(in_memory_db): start_mappers() - yield sessionmaker(bind=in_memory_db)() + yield sessionmaker(bind=in_memory_db) clear_mappers() +@pytest.fixture +def session(session_factory): + return session_factory() + + def wait_for_postgres_to_come_up(engine): deadline = time.time() + 10 while time.time() < deadline: diff --git a/tests/integration/test_uow.py b/tests/integration/test_uow.py new file mode 100644 index 00000000..5ee11051 --- /dev/null +++ b/tests/integration/test_uow.py @@ -0,0 +1,40 @@ +import pytest +from allocation import model +from allocation import unit_of_work + + +def insert_batch(session, ref, sku, qty, eta): + session.execute( + "INSERT INTO batches (reference, sku, _purchased_quantity, eta)" + " VALUES (:ref, :sku, :qty, :eta)", + dict(ref=ref, sku=sku, qty=qty, eta=eta), + ) + + +def get_allocated_batch_ref(session, orderid, sku): + [[orderlineid]] = session.execute( + "SELECT id FROM order_lines WHERE orderid=:orderid AND sku=:sku", + dict(orderid=orderid, sku=sku), + ) + [[batchref]] = session.execute( + "SELECT b.reference FROM allocations JOIN batches AS b ON batch_id = b.id" + " WHERE orderline_id=:orderlineid", + dict(orderlineid=orderlineid), + ) + return batchref + + +def test_uow_can_retrieve_a_batch_and_allocate_to_it(session_factory): + session = session_factory() + insert_batch(session, "batch1", "HIPSTER-WORKBENCH", 100, None) + session.commit() + + uow = unit_of_work.SqlAlchemyUnitOfWork(session_factory) + with uow: + batch = uow.batches.get(reference="batch1") + line = model.OrderLine("o1", "HIPSTER-WORKBENCH", 10) + batch.allocate(line) + uow.commit() + + batchref = get_allocated_batch_ref(session, "o1", "HIPSTER-WORKBENCH") + assert batchref == "batch1" From 658e61acd072c7f2bdec8fb0e9b50a9adc0edfdb Mon Sep 17 00:00:00 2001 From: Harry Date: Wed, 20 Mar 2019 10:53:14 +0000 Subject: [PATCH 37/39] use uow in services, flask app --- src/allocation/adapters/orm.py | 4 +- src/allocation/entrypoints/flask_app.py | 18 +++----- src/allocation/service_layer/services.py | 24 ++++++----- .../{ => service_layer}/unit_of_work.py | 8 +++- src/setup.py | 4 +- tests/integration/test_uow.py | 5 +-- tests/unit/test_services.py | 42 ++++++++++--------- 7 files changed, 55 insertions(+), 50 deletions(-) rename src/allocation/{ => service_layer}/unit_of_work.py (88%) diff --git a/src/allocation/adapters/orm.py b/src/allocation/adapters/orm.py index 444a8cbb..c189a739 100644 --- a/src/allocation/adapters/orm.py +++ b/src/allocation/adapters/orm.py @@ -41,7 +41,9 @@ def start_mappers(): batches, properties={ "_allocations": relationship( - lines_mapper, secondary=allocations, collection_class=set, + lines_mapper, + secondary=allocations, + collection_class=set, ) }, ) diff --git a/src/allocation/entrypoints/flask_app.py b/src/allocation/entrypoints/flask_app.py index 2164c9b6..602a09c1 100644 --- a/src/allocation/entrypoints/flask_app.py +++ b/src/allocation/entrypoints/flask_app.py @@ -3,20 +3,16 @@ from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker -from allocation import config from allocation.domain import model -from allocation.adapters import orm, repository -from allocation.service_layer import services +from allocation.adapters import orm +from allocation.service_layer import services, unit_of_work -orm.start_mappers() -get_session = sessionmaker(bind=create_engine(config.get_postgres_uri())) app = Flask(__name__) +orm.start_mappers() @app.route("/add_batch", methods=["POST"]) def add_batch(): - session = get_session() - repo = repository.SqlAlchemyRepository(session) eta = request.json["eta"] if eta is not None: eta = datetime.fromisoformat(eta).date() @@ -25,23 +21,19 @@ def add_batch(): request.json["sku"], request.json["qty"], eta, - repo, - session, + unit_of_work.SqlAlchemyUnitOfWork(), ) return "OK", 201 @app.route("/allocate", methods=["POST"]) def allocate_endpoint(): - session = get_session() - repo = repository.SqlAlchemyRepository(session) try: batchref = services.allocate( request.json["orderid"], request.json["sku"], request.json["qty"], - repo, - session, + unit_of_work.SqlAlchemyUnitOfWork(), ) except (model.OutOfStock, services.InvalidSku) as e: return {"message": str(e)}, 400 diff --git a/src/allocation/service_layer/services.py b/src/allocation/service_layer/services.py index 051de589..c5d46772 100644 --- a/src/allocation/service_layer/services.py +++ b/src/allocation/service_layer/services.py @@ -4,7 +4,7 @@ from allocation.domain import model from allocation.domain.model import OrderLine -from allocation.adapters.repository import AbstractRepository +from allocation.service_layer import unit_of_work class InvalidSku(Exception): @@ -17,20 +17,22 @@ def is_valid_sku(sku, batches): def add_batch( ref: str, sku: str, qty: int, eta: Optional[date], - repo: AbstractRepository, session, -) -> None: - repo.add(model.Batch(ref, sku, qty, eta)) - session.commit() + uow: unit_of_work.AbstractUnitOfWork, +): + with uow: + uow.batches.add(model.Batch(ref, sku, qty, eta)) + uow.commit() def allocate( orderid: str, sku: str, qty: int, - repo: AbstractRepository, session, + uow: unit_of_work.AbstractUnitOfWork, ) -> str: line = OrderLine(orderid, sku, qty) - batches = repo.list() - if not is_valid_sku(line.sku, batches): - raise InvalidSku(f"Invalid sku {line.sku}") - batchref = model.allocate(line, batches) - session.commit() + with uow: + batches = uow.batches.list() + if not is_valid_sku(line.sku, batches): + raise InvalidSku(f"Invalid sku {line.sku}") + batchref = model.allocate(line, batches) + uow.commit() return batchref diff --git a/src/allocation/unit_of_work.py b/src/allocation/service_layer/unit_of_work.py similarity index 88% rename from src/allocation/unit_of_work.py rename to src/allocation/service_layer/unit_of_work.py index d89e1138..bf9196c8 100644 --- a/src/allocation/unit_of_work.py +++ b/src/allocation/service_layer/unit_of_work.py @@ -6,7 +6,7 @@ from sqlalchemy.orm.session import Session from allocation import config -from allocation import repository +from allocation.adapters import repository class AbstractUnitOfWork(abc.ABC): @@ -27,7 +27,11 @@ def rollback(self): raise NotImplementedError -DEFAULT_SESSION_FACTORY = sessionmaker(bind=create_engine(config.get_postgres_uri(),)) +DEFAULT_SESSION_FACTORY = sessionmaker( + bind=create_engine( + config.get_postgres_uri(), + ) +) class SqlAlchemyUnitOfWork(AbstractUnitOfWork): diff --git a/src/setup.py b/src/setup.py index 731007b0..b2b0839a 100644 --- a/src/setup.py +++ b/src/setup.py @@ -1,5 +1,7 @@ from setuptools import setup setup( - name="allocation", version="0.1", packages=["allocation"], + name="allocation", + version="0.1", + packages=["allocation"], ) diff --git a/tests/integration/test_uow.py b/tests/integration/test_uow.py index 5ee11051..23f27619 100644 --- a/tests/integration/test_uow.py +++ b/tests/integration/test_uow.py @@ -1,6 +1,5 @@ -import pytest -from allocation import model -from allocation import unit_of_work +from allocation.domain import model +from allocation.service_layer import unit_of_work def insert_batch(session, ref, sku, qty, eta): diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index af0ae807..091dbb2c 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -1,6 +1,6 @@ import pytest from allocation.adapters import repository -from allocation.service_layer import services +from allocation.service_layer import services, unit_of_work class FakeRepository(repository.AbstractRepository): @@ -17,38 +17,42 @@ def list(self): return list(self._batches) -class FakeSession: - committed = False +class FakeUnitOfWork(unit_of_work.AbstractUnitOfWork): + def __init__(self): + self.batches = FakeRepository([]) + self.committed = False def commit(self): self.committed = True + def rollback(self): + pass + def test_add_batch(): - repo, session = FakeRepository([]), FakeSession() - services.add_batch("b1", "CRUNCHY-ARMCHAIR", 100, None, repo, session) - assert repo.get("b1") is not None - assert session.committed + uow = FakeUnitOfWork() + services.add_batch("b1", "CRUNCHY-ARMCHAIR", 100, None, uow) + assert uow.batches.get("b1") is not None + assert uow.committed def test_allocate_returns_allocation(): - repo, session = FakeRepository([]), FakeSession() - services.add_batch("batch1", "COMPLICATED-LAMP", 100, None, repo, session) - result = services.allocate("o1", "COMPLICATED-LAMP", 10, repo, session) + uow = FakeUnitOfWork() + services.add_batch("batch1", "COMPLICATED-LAMP", 100, None, uow) + result = services.allocate("o1", "COMPLICATED-LAMP", 10, uow) assert result == "batch1" def test_allocate_errors_for_invalid_sku(): - repo, session = FakeRepository([]), FakeSession() - services.add_batch("b1", "AREALSKU", 100, None, repo, session) + uow = FakeUnitOfWork() + services.add_batch("b1", "AREALSKU", 100, None, uow) with pytest.raises(services.InvalidSku, match="Invalid sku NONEXISTENTSKU"): - services.allocate("o1", "NONEXISTENTSKU", 10, repo, FakeSession()) + services.allocate("o1", "NONEXISTENTSKU", 10, uow) -def test_commits(): - repo, session = FakeRepository([]), FakeSession() - session = FakeSession() - services.add_batch("b1", "OMINOUS-MIRROR", 100, None, repo, session) - services.allocate("o1", "OMINOUS-MIRROR", 10, repo, session) - assert session.committed is True +def test_allocate_commits(): + uow = FakeUnitOfWork() + services.add_batch("b1", "OMINOUS-MIRROR", 100, None, uow) + services.allocate("o1", "OMINOUS-MIRROR", 10, uow) + assert uow.committed From 7526014be1a288f967393ed7bb544655c87a2817 Mon Sep 17 00:00:00 2001 From: Harry Date: Tue, 12 Mar 2019 11:52:13 +0000 Subject: [PATCH 38/39] two more tests for rollback behaviour [chapter_06_uow_ends] --- tests/integration/test_uow.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/integration/test_uow.py b/tests/integration/test_uow.py index 23f27619..3887e3ca 100644 --- a/tests/integration/test_uow.py +++ b/tests/integration/test_uow.py @@ -1,3 +1,4 @@ +import pytest from allocation.domain import model from allocation.service_layer import unit_of_work @@ -37,3 +38,28 @@ def test_uow_can_retrieve_a_batch_and_allocate_to_it(session_factory): batchref = get_allocated_batch_ref(session, "o1", "HIPSTER-WORKBENCH") assert batchref == "batch1" + + +def test_rolls_back_uncommitted_work_by_default(session_factory): + uow = unit_of_work.SqlAlchemyUnitOfWork(session_factory) + with uow: + insert_batch(uow.session, "batch1", "MEDIUM-PLINTH", 100, None) + + new_session = session_factory() + rows = list(new_session.execute('SELECT * FROM "batches"')) + assert rows == [] + + +def test_rolls_back_on_error(session_factory): + class MyException(Exception): + pass + + uow = unit_of_work.SqlAlchemyUnitOfWork(session_factory) + with pytest.raises(MyException): + with uow: + insert_batch(uow.session, "batch1", "LARGE-FORK", 100, None) + raise MyException() + + new_session = session_factory() + rows = list(new_session.execute('SELECT * FROM "batches"')) + assert rows == [] From 8a5f64aa40977d0601d5d1d0b16c51a8850eaf15 Mon Sep 17 00:00:00 2001 From: Guillaume Gauvrit Date: Sun, 21 Mar 2021 17:35:54 +0100 Subject: [PATCH 39/39] Add missing method in abstract base class --- src/allocation/adapters/repository.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/allocation/adapters/repository.py b/src/allocation/adapters/repository.py index 85e46076..e1d9e9be 100644 --- a/src/allocation/adapters/repository.py +++ b/src/allocation/adapters/repository.py @@ -11,6 +11,10 @@ def add(self, batch: model.Batch): def get(self, reference) -> model.Batch: raise NotImplementedError + @abc.abstractmethod + def list(self): + raise NotImplementedError + class SqlAlchemyRepository(AbstractRepository): def __init__(self, session):