From 1fd9f7a9a637cc9716c28347dac7cafab17e2c10 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Fri, 25 Aug 2023 22:06:35 +0200 Subject: [PATCH 01/19] RED: 404 and schema test fail --- Makefile | 2 +- utests/test_api.py | 118 ++++++++++++++++++++++++++++++++++++ utests/test_sql_database.py | 37 +++++------ utests/vadrouille.json | 0 4 files changed, 133 insertions(+), 24 deletions(-) create mode 100644 utests/test_api.py create mode 100644 utests/vadrouille.json diff --git a/Makefile b/Makefile index 9fbd449..b6cfdd5 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ run_dev: git ls-files | entr -r pipenv run python dev.py tdd: - git ls-files | entr make test opt='--lf --ff' + git ls-files | entr make test opt=$(opt) git ls-files | entr make functionnal_tests watch_db: diff --git a/utests/test_api.py b/utests/test_api.py new file mode 100644 index 0000000..c67adba --- /dev/null +++ b/utests/test_api.py @@ -0,0 +1,118 @@ +from fastapi.testclient import TestClient +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from sqlalchemy.pool import StaticPool +from sqlalchemy import MetaData + +from database import Base +from dev import app, get_db +from models import Movie + +import pytest +import crud +import contextlib + +import random +import inspect +import unittest + +SQLALCHEMY_DATABASE_URL = "sqlite:///:memory:" + +engine = create_engine( + SQLALCHEMY_DATABASE_URL, + connect_args={"check_same_thread": False}, + poolclass=StaticPool, +) +TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) + + +Base.metadata.create_all(bind=engine) + + +def override_get_db(): + try: + db = TestingSessionLocal() + yield db + finally: + db.close() + + +app.dependency_overrides[get_db] = override_get_db + +client = TestClient(app) + + +def rand_name(): + import sys + + caller = sys._getframe(1).f_code.co_name + name = f"{caller}_{random.randint(1, 1000)}" + return name + + +def test_get_movie_404_if_not_found(): + response = client.get("/movies/-1") + assert response.status_code == 404 + + +def test_list_movies(): + response = client.get("/movies/") + # assert response.json() == [] + + N = 10 + names = [] + for _ in range(N): + name = rand_name() + + names.append(name) + response = client.post("/movies/", json={"name": name}) + assert response.status_code == 200 + + movies = client.get("/movies/") + movies_by_name = {m["name"]: m for m in movies.json()} + found = list(movies_by_name[name] for name in names) + assert all(movies_by_name[name] for name in names) + + +def test_create_movie_api(): + name = f"rand_{random.randint(1, 1000)}" + response = client.post("/movies/", json={"name": name}) + + assert response.status_code == 200 + movie_id = response.json()["id"] + assert f"Created {name}" in response.json()["message"] + response = client.get(f"/movies/{movie_id}") + assert response.json()["name"] == name + + +class ApiTestCase(unittest.TestCase): + def test_payload_content_in_and_out_loopback(self): + be_the_fun_in_de_funes = { + "id": 1, + "title": "La Grande Vadrouille", + "description": "During World War II, two French civilians and a downed English Bomber Crew set " + "out from Paris to cross the demarcation line between Nazi-occupied Northern France and the " + "South. From there they will be able to escape to England. First, they must avoid German troops -" + "and the consequences of their own blunders.", + "genres": ["Comedy", "War"], + "release_date": "1966-12-07", + "vote_average": 7.7, + "vote_count": 1123, + } + + domain_keys = {k for k in be_the_fun_in_de_funes if k not in ["id"]} + payload = {k: be_the_fun_in_de_funes[k] for k in domain_keys} + # FIXME + payload["name"] = payload["title"] + response = client.post("/movies/", json=payload) + + assert response.status_code == 200 + movie_id = response.json()["id"] + + loopback_fetch = client.get(f"/movies/{movie_id}") + assert loopback_fetch.status_code == 200 + loopback_payload = loopback_fetch.json() + # check for keys + for attribute_name in domain_keys: + with self.subTest(attribute_name=attribute_name): + assert attribute_name in loopback_payload diff --git a/utests/test_sql_database.py b/utests/test_sql_database.py index 1fca0e1..560df89 100644 --- a/utests/test_sql_database.py +++ b/utests/test_sql_database.py @@ -28,14 +28,7 @@ TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engin Base.metadata.create_all(bind=engine) -def clear_db(): - # Make this a more generic functional tool for test - meta = MetaData() - with contextlib.closing(engine.connect()) as con: - trans = con.begin() - for table in reversed(meta.sorted_tables): - con.execute(table.delete()) - trans.commit() +client = TestClient(app) def override_get_db(): @@ -47,16 +40,24 @@ def override_get_db(): db.close() +app.dependency_overrides[get_db] = override_get_db + + +def clear_db(): + # Make this a more generic functional tool for test + meta = MetaData() + with contextlib.closing(engine.connect()) as con: + trans = con.begin() + for table in reversed(meta.sorted_tables): + con.execute(table.delete()) + trans.commit() + + @contextlib.contextmanager def db_context(): yield from override_get_db() -app.dependency_overrides[get_db] = override_get_db - -client = TestClient(app) - - def rand_name(): import sys @@ -96,13 +97,3 @@ def test_list_movies(): movies = client.get("movies") movies_by_name = {m["name"]: m for m in movies.json()} assert all(movies_by_name[name] for name in names) - - -def test_create_movie_api(): - name = f"rand_{random.randint(1, 1000)}" - response = client.post("/movies/", json={"name": name}) - assert response.status_code == 200 - movie_id = response.json()["id"] - assert f"Created {name}" in response.json()["message"] - response = client.get(f"/movies/{movie_id}") - assert response.json()["name"] == name diff --git a/utests/vadrouille.json b/utests/vadrouille.json new file mode 100644 index 0000000..e69de29 From e40df4886fee6a39f6fb8cf315244a2b822297bc Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Fri, 25 Aug 2023 22:10:43 +0200 Subject: [PATCH 02/19] Implement 404 on missing id --- crud.py | 9 ++++++--- dev.py | 12 ++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/crud.py b/crud.py index abb3f46..f238ce9 100644 --- a/crud.py +++ b/crud.py @@ -1,5 +1,5 @@ +import sqlalchemy from sqlalchemy.orm import Session - import models # import schemas @@ -28,5 +28,8 @@ def get_movie_by_id(db: Session, id_: str = ""): id_ = int(id_) except ValueError: pass - db_movie = db.query(models.Movie).filter(models.Movie.id == id_) - return db_movie.one() + try: + db_movie = db.query(models.Movie).filter(models.Movie.id == id_).one() + except sqlalchemy.exc.NoResultFound: + raise LookupError + return db_movie diff --git a/dev.py b/dev.py index d7f47e0..0788b24 100644 --- a/dev.py +++ b/dev.py @@ -1,4 +1,4 @@ -from fastapi import FastAPI, Depends, Request +from fastapi import FastAPI, Depends, Request, HTTPException from sqlalchemy.orm import Session @@ -47,9 +47,13 @@ async def create_movie( @app.get("/movies/{id_}") async def get_movie(id_: str, db: Session = Depends(get_db)): - movie = crud.get_movie_by_id(db, id_) - out = {k: v for (k, v) in movie.__dict__.items() if not k.startswith("_")} - return out + try: + movie = crud.get_movie_by_id(db, id_) + out = {k: v for (k, v) in movie.__dict__.items() if not k.startswith("_")} + except LookupError: + raise HTTPException(status_code=404, detail=f"No movie found with id {id_}") + else: + return out @app.get("/movies/") From 5039cd66aab7f7f96f48615d277c668b71748398 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Fri, 25 Aug 2023 22:21:59 +0200 Subject: [PATCH 03/19] Green: ouput schema seems ok --- models.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/models.py b/models.py index f660cc3..d8147d1 100644 --- a/models.py +++ b/models.py @@ -1,5 +1,4 @@ -from sqlalchemy import Column, ForeignKey, Integer, String - +from sqlalchemy import Column, ForeignKey, Integer, String, Float from database import Base @@ -9,3 +8,11 @@ class Movie(Base): id = Column(Integer, primary_key=True, index=True) name = Column(String, index=True) + title = Column(String, index=True) + + vote_count = Column(Integer) + vote_average = Column(Float) + + genres = Column(String) # String array dimention 1 + description = Column(String) + release_date = Column(String) From b309418af41e6932f0b7bc7116a9f2689054c350 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Fri, 25 Aug 2023 22:25:26 +0200 Subject: [PATCH 04/19] RED: not storing values :/ --- utests/test_api.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/utests/test_api.py b/utests/test_api.py index c67adba..7400d0b 100644 --- a/utests/test_api.py +++ b/utests/test_api.py @@ -116,3 +116,7 @@ class ApiTestCase(unittest.TestCase): for attribute_name in domain_keys: with self.subTest(attribute_name=attribute_name): assert attribute_name in loopback_payload + assert ( + loopback_payload[attribute_name] + == be_the_fun_in_de_funes[attribute_name] + ) From 504008b75b8d88401b5383edec78be3e29fefc3d Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Fri, 25 Aug 2023 22:41:44 +0200 Subject: [PATCH 05/19] RED: For now we only test primitive type to move soon to pydantic --- crud.py | 4 +-- dev.py | 11 +++----- utests/test_api.py | 53 +++++++++++++++++++++++++++++++++---- utests/test_sql_database.py | 4 +-- 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/crud.py b/crud.py index f238ce9..41f8535 100644 --- a/crud.py +++ b/crud.py @@ -5,8 +5,8 @@ import models # import schemas -def create_movie(db: Session, name: str = ""): - db_movie = models.Movie(name=name) +def create_movie(db: Session, name: str, genres: list[str], description: str = ""): + db_movie = models.Movie(name=name, genres=str(genres), description=description) db.add(db_movie) db.commit() db.refresh(db_movie) diff --git a/dev.py b/dev.py index 0788b24..d106735 100644 --- a/dev.py +++ b/dev.py @@ -35,12 +35,9 @@ async def create_movie( except: data = {} name = name or data["name"] - movie = models.Movie() - movie.name = name - db.add(movie) - db.flush() - db.commit() - db.refresh(movie) + genres = data.get("genres", ["Unknown"]) + description = data.get("description", "") + movie = crud.create_movie(db, name=name, genres=genres, description=description) out = {"message": f"Created {movie.name} XX", "id": movie.id} return out @@ -57,7 +54,7 @@ async def get_movie(id_: str, db: Session = Depends(get_db)): @app.get("/movies/") -async def create_movie(db: Session = Depends(get_db)): +async def list_movie(db: Session = Depends(get_db)): movies = crud.get_all_movies(db) out = [ diff --git a/utests/test_api.py b/utests/test_api.py index 7400d0b..b00ebd9 100644 --- a/utests/test_api.py +++ b/utests/test_api.py @@ -100,7 +100,11 @@ class ApiTestCase(unittest.TestCase): "vote_count": 1123, } - domain_keys = {k for k in be_the_fun_in_de_funes if k not in ["id"]} + domain_keys = sorted( + {k for k in be_the_fun_in_de_funes if k not in ["id"]} + ) # Make it deterministic + non_primtive = ["genres", "release_date"] + payload = {k: be_the_fun_in_de_funes[k] for k in domain_keys} # FIXME payload["name"] = payload["title"] @@ -116,7 +120,46 @@ class ApiTestCase(unittest.TestCase): for attribute_name in domain_keys: with self.subTest(attribute_name=attribute_name): assert attribute_name in loopback_payload - assert ( - loopback_payload[attribute_name] - == be_the_fun_in_de_funes[attribute_name] - ) + if attribute_name not in non_primtive: + assert ( + loopback_payload[attribute_name] + == be_the_fun_in_de_funes[attribute_name] + ) + + @unittest.expectedFailure + def test_payload_content_in_and_out_loopback_non_primitive(self): + be_the_fun_in_de_funes = { + "id": 1, + "title": "La Grande Vadrouille", + "description": "During World War II, two French civilians and a downed English Bomber Crew set " + "out from Paris to cross the demarcation line between Nazi-occupied Northern France and the " + "South. From there they will be able to escape to England. First, they must avoid German troops -" + "and the consequences of their own blunders.", + "genres": ["Comedy", "War"], + "release_date": "1966-12-07", + "vote_average": 7.7, + "vote_count": 1123, + } + + domain_keys = sorted( + {k for k in be_the_fun_in_de_funes if k not in ["id"]} + ) # Make it deterministic + non_primtive = ["genres", "release_date"] + + payload = {k: be_the_fun_in_de_funes[k] for k in domain_keys} + # FIXME + payload["name"] = payload["title"] + response = client.post("/movies/", json=payload) + + assert response.status_code == 200 + movie_id = response.json()["id"] + + loopback_fetch = client.get(f"/movies/{movie_id}") + assert loopback_fetch.status_code == 200 + loopback_payload = loopback_fetch.json() + # check for keys + for attribute_name in domain_keys: + assert ( + loopback_payload[attribute_name] + == be_the_fun_in_de_funes[attribute_name] + ) diff --git a/utests/test_sql_database.py b/utests/test_sql_database.py index 560df89..019a9a1 100644 --- a/utests/test_sql_database.py +++ b/utests/test_sql_database.py @@ -76,7 +76,7 @@ def test_sample_crud(): name = rand_name() with db_context() as db: - movie = crud.create_movie(db, name=name) + movie = crud.create_movie(db, name=name, genres=["Yes", "No"]) assert movie.name == name @@ -92,7 +92,7 @@ def test_list_movies(): name = rand_name() names.append(name) - crud.create_movie(db, name=name) + crud.create_movie(db, name=name, genres=["Animated", "Paropaganda"]) movies = client.get("movies") movies_by_name = {m["name"]: m for m in movies.json()} From a030fb0b4f719b6f0be285b561f9a8c30b540a41 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Fri, 25 Aug 2023 22:58:09 +0200 Subject: [PATCH 06/19] Green: we can get simple attributes back and forth --- crud.py | 18 ++++++++++++++++-- dev.py | 15 ++++++++++----- models.py | 1 - utests/test_api.py | 15 ++++++--------- utests/test_sql_database.py | 14 +++++++------- 5 files changed, 39 insertions(+), 24 deletions(-) diff --git a/crud.py b/crud.py index 41f8535..f9c20a8 100644 --- a/crud.py +++ b/crud.py @@ -5,8 +5,22 @@ import models # import schemas -def create_movie(db: Session, name: str, genres: list[str], description: str = ""): - db_movie = models.Movie(name=name, genres=str(genres), description=description) +def create_movie( + db: Session, + *, + title: str, + genres: list[str], + description: str = "", + vote_average: float | None = None, + vote_count: int | None = None +): + db_movie = models.Movie( + title=title, + genres=str(genres), + description=description, + vote_average=vote_average, + vote_count=vote_count, + ) db.add(db_movie) db.commit() db.refresh(db_movie) diff --git a/dev.py b/dev.py index d106735..1a1596a 100644 --- a/dev.py +++ b/dev.py @@ -34,11 +34,16 @@ async def create_movie( data = await request.json() except: data = {} - name = name or data["name"] - genres = data.get("genres", ["Unknown"]) - description = data.get("description", "") - movie = crud.create_movie(db, name=name, genres=genres, description=description) - out = {"message": f"Created {movie.name} XX", "id": movie.id} + crud_params = dict( + genres=data.get("genres", ["Unknown"]), + description=data.get("description", ""), + title=data.get("title", ""), + vote_average=data.get("vote_average"), + vote_count=data.get("vote_count"), + ) + + movie = crud.create_movie(db, **crud_params) + out = {"message": f"Created {movie.title} XX", "id": movie.id} return out diff --git a/models.py b/models.py index d8147d1..59e1640 100644 --- a/models.py +++ b/models.py @@ -7,7 +7,6 @@ class Movie(Base): id = Column(Integer, primary_key=True, index=True) - name = Column(String, index=True) title = Column(String, index=True) vote_count = Column(Integer) diff --git a/utests/test_api.py b/utests/test_api.py index b00ebd9..5d3033a 100644 --- a/utests/test_api.py +++ b/utests/test_api.py @@ -65,24 +65,24 @@ def test_list_movies(): name = rand_name() names.append(name) - response = client.post("/movies/", json={"name": name}) + response = client.post("/movies/", json={"title": name}) assert response.status_code == 200 movies = client.get("/movies/") - movies_by_name = {m["name"]: m for m in movies.json()} - found = list(movies_by_name[name] for name in names) - assert all(movies_by_name[name] for name in names) + movies_by_title = {m["title"]: m for m in movies.json()} + found = list(movies_by_title[title] for title in names) + assert all(movies_by_title[title] for title in names) def test_create_movie_api(): name = f"rand_{random.randint(1, 1000)}" - response = client.post("/movies/", json={"name": name}) + response = client.post("/movies/", json={"title": name}) assert response.status_code == 200 movie_id = response.json()["id"] assert f"Created {name}" in response.json()["message"] response = client.get(f"/movies/{movie_id}") - assert response.json()["name"] == name + assert response.json()["title"] == name class ApiTestCase(unittest.TestCase): @@ -107,7 +107,6 @@ class ApiTestCase(unittest.TestCase): payload = {k: be_the_fun_in_de_funes[k] for k in domain_keys} # FIXME - payload["name"] = payload["title"] response = client.post("/movies/", json=payload) assert response.status_code == 200 @@ -147,8 +146,6 @@ class ApiTestCase(unittest.TestCase): non_primtive = ["genres", "release_date"] payload = {k: be_the_fun_in_de_funes[k] for k in domain_keys} - # FIXME - payload["name"] = payload["title"] response = client.post("/movies/", json=payload) assert response.status_code == 200 diff --git a/utests/test_sql_database.py b/utests/test_sql_database.py index 019a9a1..7143dc8 100644 --- a/utests/test_sql_database.py +++ b/utests/test_sql_database.py @@ -68,16 +68,16 @@ def rand_name(): def test_create_moviem_models(): name = rand_name() - movie = Movie(name=name) - assert movie.name == name + movie = Movie(title=name) + assert movie.title == name def test_sample_crud(): name = rand_name() with db_context() as db: - movie = crud.create_movie(db, name=name, genres=["Yes", "No"]) - assert movie.name == name + movie = crud.create_movie(db, title=name, genres=["Yes", "No"]) + assert movie.title == name def test_list_movies(): @@ -92,8 +92,8 @@ def test_list_movies(): name = rand_name() names.append(name) - crud.create_movie(db, name=name, genres=["Animated", "Paropaganda"]) + crud.create_movie(db, title=name, genres=["Animated", "Paropaganda"]) movies = client.get("movies") - movies_by_name = {m["name"]: m for m in movies.json()} - assert all(movies_by_name[name] for name in names) + movies_by_title = {m["title"]: m for m in movies.json()} + assert all(movies_by_title[name] for name in names) From ec2f661a49a9e22db429df30e13ea27b4f39bc73 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Fri, 25 Aug 2023 23:09:55 +0200 Subject: [PATCH 07/19] RED: Add schema, fail on purpose to check --- dev.py | 19 ++++++++++++++++++- schemas.py | 12 ++++++++++++ utests/test_api.py | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 schemas.py diff --git a/dev.py b/dev.py index 1a1596a..9a25447 100644 --- a/dev.py +++ b/dev.py @@ -6,7 +6,7 @@ import uvicorn import database import models import crud - +import schemas app = FastAPI() @@ -25,6 +25,23 @@ async def root(): return {"message": "Hello World"} +@app.post("/pydantic_movies/") +async def create_movie(payload: schemas.MoviePayload, db: Session = Depends(get_db)): + data = payload.dict() + crud_params = dict( + genres=data.get("genres", ["Unknown"]), + description=data.get("description", ""), + title=data.get("title", ""), + vote_average=data.get("vote_average"), + vote_count=data.get("vote_count"), + ) + + movie = crud.create_movie(db, **crud_params) + out = {"message": f"Created {movie.title} XX", "id": movie.id} + raise ValueError() + return out + + @app.post("/movies/") async def create_movie( name: str = "", db: Session = Depends(get_db), request: Request = None diff --git a/schemas.py b/schemas.py new file mode 100644 index 0000000..8609fe6 --- /dev/null +++ b/schemas.py @@ -0,0 +1,12 @@ +from pydantic import BaseModel + + +class MoviePayload(BaseModel): + title: str + + vote_count: int + vote_average: float + + genres: list[str] + description: str + release_date: str diff --git a/utests/test_api.py b/utests/test_api.py index 5d3033a..b6ccb71 100644 --- a/utests/test_api.py +++ b/utests/test_api.py @@ -125,6 +125,45 @@ class ApiTestCase(unittest.TestCase): == be_the_fun_in_de_funes[attribute_name] ) + def test_payload_content_in_and_out_loopback_pydantic(self): + be_the_fun_in_de_funes = { + "id": 1, + "title": "La Grande Vadrouille", + "description": "During World War II, two French civilians and a downed English Bomber Crew set " + "out from Paris to cross the demarcation line between Nazi-occupied Northern France and the " + "South. From there they will be able to escape to England. First, they must avoid German troops -" + "and the consequences of their own blunders.", + "genres": ["Comedy", "War"], + "release_date": "1966-12-07", + "vote_average": 7.7, + "vote_count": 1123, + } + + domain_keys = sorted( + {k for k in be_the_fun_in_de_funes if k not in ["id"]} + ) # Make it deterministic + non_primtive = ["genres", "release_date"] + + payload = {k: be_the_fun_in_de_funes[k] for k in domain_keys} + # FIXME + response = client.post("/pydantic_movies/", json=payload) + + assert response.status_code == 200 + movie_id = response.json()["id"] + + loopback_fetch = client.get(f"/movies/{movie_id}") + assert loopback_fetch.status_code == 200 + loopback_payload = loopback_fetch.json() + # check for keys + for attribute_name in domain_keys: + with self.subTest(attribute_name=attribute_name): + assert attribute_name in loopback_payload + if attribute_name not in non_primtive: + assert ( + loopback_payload[attribute_name] + == be_the_fun_in_de_funes[attribute_name] + ) + @unittest.expectedFailure def test_payload_content_in_and_out_loopback_non_primitive(self): be_the_fun_in_de_funes = { From 17bf8bb645e787d6ad8d8d14df53bfd6b0927005 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Fri, 25 Aug 2023 23:11:29 +0200 Subject: [PATCH 08/19] Green --- dev.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dev.py b/dev.py index 9a25447..8835668 100644 --- a/dev.py +++ b/dev.py @@ -38,7 +38,6 @@ async def create_movie(payload: schemas.MoviePayload, db: Session = Depends(get_ movie = crud.create_movie(db, **crud_params) out = {"message": f"Created {movie.title} XX", "id": movie.id} - raise ValueError() return out From 5ee4cb604141130146756aa727a9245c6126bd3c Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Fri, 25 Aug 2023 23:14:03 +0200 Subject: [PATCH 09/19] Green: Remove custom schema adapter Nice we have the releasse date we missed --- crud.py | 4 +++- dev.py | 11 +---------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/crud.py b/crud.py index f9c20a8..60e8f4b 100644 --- a/crud.py +++ b/crud.py @@ -12,7 +12,8 @@ def create_movie( genres: list[str], description: str = "", vote_average: float | None = None, - vote_count: int | None = None + vote_count: int | None = None, + release_date: str | None = None ): db_movie = models.Movie( title=title, @@ -20,6 +21,7 @@ def create_movie( description=description, vote_average=vote_average, vote_count=vote_count, + release_date=release_date, ) db.add(db_movie) db.commit() diff --git a/dev.py b/dev.py index 8835668..be3bccc 100644 --- a/dev.py +++ b/dev.py @@ -27,16 +27,7 @@ async def root(): @app.post("/pydantic_movies/") async def create_movie(payload: schemas.MoviePayload, db: Session = Depends(get_db)): - data = payload.dict() - crud_params = dict( - genres=data.get("genres", ["Unknown"]), - description=data.get("description", ""), - title=data.get("title", ""), - vote_average=data.get("vote_average"), - vote_count=data.get("vote_count"), - ) - - movie = crud.create_movie(db, **crud_params) + movie = crud.create_movie(db, **payload.dict()) out = {"message": f"Created {movie.title} XX", "id": movie.id} return out From 2a02a839b70c4a6c76df384a59d2cd74bf0d76f8 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Fri, 25 Aug 2023 23:28:03 +0200 Subject: [PATCH 10/19] Candy: add test coverage --- Makefile | 4 +++- Pipfile | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b6cfdd5..bc07dd0 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,5 @@ +coverage_opt=--cov --cov-report=term-missing:skip-covered --durations=10 + clean: pipenv --rm @@ -20,7 +22,7 @@ watch_db: test: - pipenv run pytest $(opt) utests + pipenv run pytest $(coverage_opt) $(opt) utests functionnal_tests: pipenv run python -m pytest functionnal_test.py diff --git a/Pipfile b/Pipfile index c0f4fc3..cd70690 100644 --- a/Pipfile +++ b/Pipfile @@ -14,6 +14,7 @@ pytest = "*" selenium = "*" httpx = "*" pdbpp = "*" +pytest-cov = "*" [requires] python_version = "3.11" From c058c7513c2f83d95030da90030dcd417cba2240 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Sat, 26 Aug 2023 17:19:32 +0200 Subject: [PATCH 11/19] RED: test for message and status_code --- utests/test_api.py | 60 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/utests/test_api.py b/utests/test_api.py index b6ccb71..64ed2ec 100644 --- a/utests/test_api.py +++ b/utests/test_api.py @@ -125,6 +125,66 @@ class ApiTestCase(unittest.TestCase): == be_the_fun_in_de_funes[attribute_name] ) + def test_payload_content_bad_format_status_code(self): + be_the_fun_in_de_funes = { + "id": 1, + "title": "La Grande Vadrouille", + "description": "During World War II, two French civilians and a downed English Bomber Crew set " + "out from Paris to cross the demarcation line between Nazi-occupied Northern France and the " + "South. From there they will be able to escape to England. First, they must avoid German troops -" + "and the consequences of their own blunders.", + "genres": ["Comedy", "War"], + "release_date": "1966-12-07", + "vote_average": 7.7, + "vote_count": 1123, + } + + domain_keys = sorted( + {k for k in be_the_fun_in_de_funes if k not in ["id"]} + ) # Make it deterministic + + payload = {k: be_the_fun_in_de_funes[k] for k in domain_keys} + + missing_key = "title" + + payload.pop(missing_key) + + response = client.post("/pydantic_movies/", json=payload) + + assert 400 <= response.status_code < 500 + + assert missing_key in response.text + + def test_payload_content_bad_format_detail(self): + be_the_fun_in_de_funes = { + "id": 1, + "title": "La Grande Vadrouille", + "description": "During World War II, two French civilians and a downed English Bomber Crew set " + "out from Paris to cross the demarcation line between Nazi-occupied Northern France and the " + "South. From there they will be able to escape to England. First, they must avoid German troops -" + "and the consequences of their own blunders.", + "genres": ["Comedy", "War"], + "release_date": "1966-12-07", + "vote_average": 7.7, + "vote_count": 1123, + } + + domain_keys = sorted( + {k for k in be_the_fun_in_de_funes if k not in ["id"]} + ) # Make it deterministic + + payload = {k: be_the_fun_in_de_funes[k] for k in domain_keys} + + missing_key = "title" + + payload.pop(missing_key) + + response = client.post("/pydantic_movies/", json=payload) + + assert 400 <= response.status_code < 500 + + assert response.status_code == 400 + def test_payload_content_in_and_out_loopback_pydantic(self): be_the_fun_in_de_funes = { "id": 1, From df1dbbbc18da644afa41e543b5fb11039c807117 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Sat, 26 Aug 2023 17:20:32 +0200 Subject: [PATCH 12/19] Green: convert 422 to 400 on input errors --- dev.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/dev.py b/dev.py index be3bccc..4ae14fa 100644 --- a/dev.py +++ b/dev.py @@ -1,4 +1,6 @@ -from fastapi import FastAPI, Depends, Request, HTTPException +from fastapi import FastAPI, Depends, Request, HTTPException, status +from fastapi.exceptions import RequestValidationError +from fastapi.responses import JSONResponse from sqlalchemy.orm import Session @@ -10,6 +12,23 @@ import schemas app = FastAPI() +convert_422_to_400 = True + + +if convert_422_to_400: + # Taken from there. + # https://stackoverflow.com/questions/75958222/can-i-return-400-error-instead-of-422-error + # https://stackoverflow.com/questions/71681068/how-to-customise-error-response-for-a-specific-route-in-fastapi + + @app.exception_handler(RequestValidationError) + async def validation_exception_handler( + request: Request, exc: RequestValidationError + ): + return JSONResponse( + status_code=status.HTTP_400_BAD_REQUEST, + content={"detail": exc.errors()}, + ) + # Dependency def get_db(): From 8b8a1ed30a9c4adf5924744b18525aa18f130acf Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Sat, 26 Aug 2023 17:20:54 +0200 Subject: [PATCH 13/19] Nicer scripts and directive foor databases --- Makefile | 6 +++++- database.py | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index bc07dd0..5b3ddaf 100644 --- a/Makefile +++ b/Makefile @@ -45,7 +45,11 @@ venv_test: make -f MakefileVenv test db_clean: - rm sql_app.db + rm sql_app.db && touch sql_app.db db_fill: pipenv run python database.py + + +db_reset: db_clean db_fill + diff --git a/database.py b/database.py index 655377f..388e28b 100644 --- a/database.py +++ b/database.py @@ -24,10 +24,13 @@ def fill_db(): import crud import random - for _ in range(10): + def _genres(): + random.choice([["Comedy"], ["Comedy", "Drama"], []]) + + for _ in range(3): name = f"fill_db_{random.randint(1, 1000):03}" - out = crud.create_movie(SessionLocal(), name=name) - print(out.name) + out = crud.create_movie(SessionLocal(), title=name, genres=_genres()) + print(out.title) if __name__ == "__main__": From 6e77545ef9655bd707bfc66997e2db690b3e82dd Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Sat, 26 Aug 2023 17:24:58 +0200 Subject: [PATCH 14/19] Refactor: unittest like --- utests/test_api.py | 53 +++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/utests/test_api.py b/utests/test_api.py index 64ed2ec..371e1b9 100644 --- a/utests/test_api.py +++ b/utests/test_api.py @@ -50,39 +50,38 @@ def rand_name(): return name -def test_get_movie_404_if_not_found(): - response = client.get("/movies/-1") - assert response.status_code == 404 +class BaseCrud(unittest.TestCase): + def test_get_movie_404_if_not_found(self): + response = client.get("/movies/-1") + assert response.status_code == 404 + def test_list_movies(self): + response = client.get("/movies/") + # assert response.json() == [] -def test_list_movies(): - response = client.get("/movies/") - # assert response.json() == [] + N = 10 + names = [] + for _ in range(N): + name = rand_name() - N = 10 - names = [] - for _ in range(N): - name = rand_name() + names.append(name) + response = client.post("/movies/", json={"title": name}) + assert response.status_code == 200 - names.append(name) + movies = client.get("/movies/") + movies_by_title = {m["title"]: m for m in movies.json()} + found = list(movies_by_title[title] for title in names) + assert all(movies_by_title[title] for title in names) + + def test_create_movie_api(self): + name = f"rand_{random.randint(1, 1000)}" response = client.post("/movies/", json={"title": name}) + assert response.status_code == 200 - - movies = client.get("/movies/") - movies_by_title = {m["title"]: m for m in movies.json()} - found = list(movies_by_title[title] for title in names) - assert all(movies_by_title[title] for title in names) - - -def test_create_movie_api(): - name = f"rand_{random.randint(1, 1000)}" - response = client.post("/movies/", json={"title": name}) - - assert response.status_code == 200 - movie_id = response.json()["id"] - assert f"Created {name}" in response.json()["message"] - response = client.get(f"/movies/{movie_id}") - assert response.json()["title"] == name + movie_id = response.json()["id"] + assert f"Created {name}" in response.json()["message"] + response = client.get(f"/movies/{movie_id}") + assert response.json()["title"] == name class ApiTestCase(unittest.TestCase): From 6bbea876a0a3cae491e212ceb933be36a754dd36 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Sat, 26 Aug 2023 17:36:22 +0200 Subject: [PATCH 15/19] cosmit: intresting test first --- utests/test_api.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/utests/test_api.py b/utests/test_api.py index 371e1b9..7c41db0 100644 --- a/utests/test_api.py +++ b/utests/test_api.py @@ -55,6 +55,17 @@ class BaseCrud(unittest.TestCase): response = client.get("/movies/-1") assert response.status_code == 404 + def test_create_movie_api(self): + name = f"rand_{random.randint(1, 1000)}" + response = client.post("/movies/", json={"title": name}) + + assert response.status_code == 200 + movie_id = response.json()["id"] + assert f"Created {name}" in response.json()["message"] + response = client.get(f"/movies/{movie_id}") + assert response.json()["title"] == name + + def test_list_movies(self): response = client.get("/movies/") # assert response.json() == [] @@ -73,16 +84,6 @@ class BaseCrud(unittest.TestCase): found = list(movies_by_title[title] for title in names) assert all(movies_by_title[title] for title in names) - def test_create_movie_api(self): - name = f"rand_{random.randint(1, 1000)}" - response = client.post("/movies/", json={"title": name}) - - assert response.status_code == 200 - movie_id = response.json()["id"] - assert f"Created {name}" in response.json()["message"] - response = client.get(f"/movies/{movie_id}") - assert response.json()["title"] == name - class ApiTestCase(unittest.TestCase): def test_payload_content_in_and_out_loopback(self): From 49c1b793295b1fa2c826f7c87e9ff3981c4ccf0f Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Sat, 26 Aug 2023 17:36:43 +0200 Subject: [PATCH 16/19] RED: Test deletion --- utests/test_api.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/utests/test_api.py b/utests/test_api.py index 7c41db0..14780cb 100644 --- a/utests/test_api.py +++ b/utests/test_api.py @@ -65,6 +65,17 @@ class BaseCrud(unittest.TestCase): response = client.get(f"/movies/{movie_id}") assert response.json()["title"] == name + def test_delete_movie(self): + name = f"rand_{random.randint(1, 1000)}" + response = client.post("/movies/", json={"title": name}) + movie_id = response.json()["id"] + created_movie_path = f"/movies/{movie_id}" + + response_delete = client.delete(created_movie_path) + response_missing = client.get(created_movie_path) + + assert response_delete.status_code == 204 + assert response_missing.status_code == 404 def test_list_movies(self): response = client.get("/movies/") From e565b445ff63b1b8f3c3710272d1831271d37e19 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Sat, 26 Aug 2023 17:52:39 +0200 Subject: [PATCH 17/19] green: actually delete movies --- crud.py | 6 ++++++ dev.py | 8 ++++++++ utests/test_api.py | 4 +++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/crud.py b/crud.py index 60e8f4b..8d65073 100644 --- a/crud.py +++ b/crud.py @@ -49,3 +49,9 @@ def get_movie_by_id(db: Session, id_: str = ""): except sqlalchemy.exc.NoResultFound: raise LookupError return db_movie + + +def delete_movie_by_id(db: Session, id_: str = ""): + movie = get_movie_by_id(db, id_) + db.delete(movie) + db.commit() diff --git a/dev.py b/dev.py index 4ae14fa..d77de24 100644 --- a/dev.py +++ b/dev.py @@ -84,6 +84,14 @@ async def get_movie(id_: str, db: Session = Depends(get_db)): return out +@app.delete("/movies/{id_}", status_code=status.HTTP_204_NO_CONTENT) +async def delete_movie(id_: str, db: Session = Depends(get_db)): + try: + movie = crud.delete_movie_by_id(db, id_) + except LookupError: + raise HTTPException(status_code=404, detail=f"No movie found with id {id_}") + + @app.get("/movies/") async def list_movie(db: Session = Depends(get_db)): movies = crud.get_all_movies(db) diff --git a/utests/test_api.py b/utests/test_api.py index 14780cb..ee6a6e3 100644 --- a/utests/test_api.py +++ b/utests/test_api.py @@ -51,9 +51,11 @@ def rand_name(): class BaseCrud(unittest.TestCase): - def test_get_movie_404_if_not_found(self): + def test_get_delete_movie_404_if_not_found(self): response = client.get("/movies/-1") assert response.status_code == 404 + response_delete = client.delete("/movies/-1") + assert response_delete.status_code == 404 def test_create_movie_api(self): name = f"rand_{random.randint(1, 1000)}" From 68e11a3275ce3669eaac116bbd993adee06b63af Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Sat, 26 Aug 2023 22:31:33 +0200 Subject: [PATCH 18/19] RED: Cannot bind list to string for genres --- crud.py | 16 +++++++++++++++- dev.py | 27 +++++++++++++++++++++++++++ utests/test_api.py | 23 +++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/crud.py b/crud.py index 8d65073..b2daf35 100644 --- a/crud.py +++ b/crud.py @@ -13,7 +13,7 @@ def create_movie( description: str = "", vote_average: float | None = None, vote_count: int | None = None, - release_date: str | None = None + release_date: str | None = None, ): db_movie = models.Movie( title=title, @@ -55,3 +55,17 @@ def delete_movie_by_id(db: Session, id_: str = ""): movie = get_movie_by_id(db, id_) db.delete(movie) db.commit() + + +def update_movie(db: Session, id_: str, **payload): + movie = get_movie_by_id(db, id_) + + for name, value in payload.items(): + try: + movie.__mapper__.attrs[name] + except KeyError: + raise ValueError(f"Bad attribute {name}") + setattr(movie, name, value) + db.add(movie) + db.commit() + return movie diff --git a/dev.py b/dev.py index d77de24..2e7fe4c 100644 --- a/dev.py +++ b/dev.py @@ -73,6 +73,33 @@ async def create_movie( return out +@app.put("/movies/{id_}") +async def update_movie( + id_: str, db: Session = Depends(get_db), request: Request = None +): + try: + movie = crud.get_movie_by_id(db, id_) + except LookupError: + raise HTTPException(status_code=404, detail=f"No movie found with id {id_}") + + try: # Bypass for dev + data = await request.json() + except: + data = {} + crud_params = dict( + genres=data.get("genres", ["Unknown"]), + description=data.get("description", ""), + title=data.get("title", ""), + vote_average=data.get("vote_average"), + vote_count=data.get("vote_count"), + ) + + movie = crud.update_movie(db, id_, **crud_params) + + out = {k: v for (k, v) in movie.__dict__.items() if not k.startswith("_")} + return out + + @app.get("/movies/{id_}") async def get_movie(id_: str, db: Session = Depends(get_db)): try: diff --git a/utests/test_api.py b/utests/test_api.py index ee6a6e3..0af0fa1 100644 --- a/utests/test_api.py +++ b/utests/test_api.py @@ -74,11 +74,34 @@ class BaseCrud(unittest.TestCase): created_movie_path = f"/movies/{movie_id}" response_delete = client.delete(created_movie_path) + response_missing = client.get(created_movie_path) assert response_delete.status_code == 204 assert response_missing.status_code == 404 + def test_update_movie_api(self): + name = f"rand_{random.randint(1, 1000)}" + response = client.post("/movies/", json={"title": name, "genres": ["anime"]}) + movie_id = response.json()["id"] + created_movie_path = f"/movies/{movie_id}" + + new_name = name.replace("rand", "update") + new_genres = ["Drama", "war"] + response_get = client.get(f"/movies/{movie_id}") + + assert response_get.json()["title"] != new_name + assert response_get.json()["genres"] != new_genres + + response_update = client.put( + created_movie_path, json={"title": new_name, "genres": new_genres} + ) + assert response_update.status_code == 200 + + response_get = client.get(f"/movies/{movie_id}") + assert response_get.json()["title"] == new_name + assert response_get.json()["genres"] == new_genres + def test_list_movies(self): response = client.get("/movies/") # assert response.json() == [] From 0ac827b95c503f54aace3e1e524f08b5fa05a397 Mon Sep 17 00:00:00 2001 From: Colin Goutte Date: Sat, 26 Aug 2023 22:51:05 +0200 Subject: [PATCH 19/19] Green: implement custom object for arrays, update is ok --- dev.py | 1 + models.py | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/dev.py b/dev.py index 2e7fe4c..1136b09 100644 --- a/dev.py +++ b/dev.py @@ -104,6 +104,7 @@ async def update_movie( async def get_movie(id_: str, db: Session = Depends(get_db)): try: movie = crud.get_movie_by_id(db, id_) + out = {k: v for (k, v) in movie.__dict__.items() if not k.startswith("_")} except LookupError: raise HTTPException(status_code=404, detail=f"No movie found with id {id_}") diff --git a/models.py b/models.py index 59e1640..ace492e 100644 --- a/models.py +++ b/models.py @@ -1,5 +1,20 @@ -from sqlalchemy import Column, ForeignKey, Integer, String, Float +from sqlalchemy import Column, ForeignKey, Integer, String, Float, types from database import Base +import sqlalchemy.types as types + + +class NaiveStringList(types.TypeDecorator): + impl = types.Unicode + sep = "\n" + + def process_bind_param(self, value, dialect): + return self.sep.join(value) + + def process_result_value(self, value, dialect): + return value.split(self.sep) + + def copy(self, **kw): + return NaiveStringList(self.impl.length) class Movie(Base): @@ -12,6 +27,8 @@ class Movie(Base): vote_count = Column(Integer) vote_average = Column(Float) - genres = Column(String) # String array dimention 1 + genres = Column(NaiveStringList) # LLw + # genres = Column(ARRAY(String, dimensions=1)) # String array dimention 1 + description = Column(String) release_date = Column(String)