Add DELETE endpoint for persons and separate update schema

README.md

@@ -61,6 +61,8 @@ Sample backend API built with **FastAPI**, **SQLAlchemy 2.x**, and **Alembic**.
 - **No trailing slashes on collection endpoints.** Routes are registered as `/persons`, `/countries`, and `/continents`. Accessing the trailing-slash variant (e.g., `/persons/`) will receive a `307 Temporary Redirect` to the canonical path.
 - **Sort syntax**: Prefix with `-` for descending (e.g., `-name`), `+` or no prefix for ascending.
 - **Filter syntax**: Query parameters are mapped to schema fields. String fields use `ILIKE` (`%value%`); others use exact equality.
+- **`PUT /persons/{id}` is an upsert.** If the ID exists it is updated; if it does not exist a new person is created with that ID. Returns `204 No Content`.
+- **`DELETE /persons/{id}`** removes the person and returns `204 No Content`. Returns `404 Not Found` if the person does not exist.
 
 ## Data Model
 

http/api.http

@@ -34,7 +34,7 @@ Content-Type: application/json
 
 ###
 
-### Update a person
+### Update a person (upsert)
 PUT {{host}}/persons/1
 Content-Type: application/json
 
@@ -46,6 +46,12 @@ Content-Type: application/json
 
 ###
 
+### Delete a person
+DELETE {{host}}/persons/1
+Accept: application/json
+
+###
+
 ### List all countries (paginated)
 GET {{host}}/countries
 Accept: application/json

pyfastapi/controllers/person.py

@@ -35,12 +35,21 @@ def create_person(body: PersonCreateSchema, service: Annotated[PersonService, De
     return service.create_person(body)
 
 
-@router.put("/{id_}")
+@router.put("/{id_}", status_code=204)
 def update_person(
     id_: Annotated[int, Path(title="person id")],
     body: PersonCreateSchema,
     service: Annotated[PersonService, Depends()]
 ) -> Response:
     logger.debug(f"body {body}")
-    service.update_person(int(id_), body)
+    service.update_or_create_person(int(id_), body)
+    return Response(status_code=204)
+
+
+@router.delete("/{id_}", status_code=204)
+def delete_person(
+    id_: Annotated[int, Path(title="person id")],
+    service: Annotated[PersonService, Depends()]
+) -> Response:
+    service.delete_person(int(id_))
     return Response(status_code=204)

pyfastapi/repositories/person.py

@@ -45,3 +45,6 @@ def update_or_create_person(self, person: Person) -> None:
             }
         )
         self.db.execute(stmt)
+
+    def delete_person(self, person: Person) -> None:
+        self.db.delete(person)

pyfastapi/services/person.py

@@ -44,7 +44,7 @@ def create_person(self, body: PersonCreateSchema) -> Person:
         self.db.refresh(new_person)
         return new_person
 
-    def update_person(self, id_: int, body: PersonCreateSchema) -> None:
+    def update_or_create_person(self, id_: int, body: PersonCreateSchema) -> None:
         if not self.country_repo.get_country(body.country_code):
             raise CountryNotFoundError(body.country_code)
 
@@ -56,3 +56,11 @@ def update_person(self, id_: int, body: PersonCreateSchema) -> None:
         person.id = id_
         self.person_repo.update_or_create_person(person)
         self.db.commit()
+
+    def delete_person(self, id_: int) -> None:
+        person = self.person_repo.get_person(id_)
+        if not person:
+            raise PersonNotFoundError(id_)
+
+        self.person_repo.delete_person(person)
+        self.db.commit()

tests/api_tests/test_e2e.py

@@ -98,19 +98,17 @@ def test_get_person_not_found(init_db: None, client: TestClient) -> None:
 
 def test_update_person_nonexistent_id_still_returns_204(init_db: None, client: TestClient, db_session: Session) -> None:
     """
-    merge() semantics mean updating a non-existent ID may INSERT instead of 404.
-    Document current behavior so regressions are visible.
+    PUT semantics mean updating a non-existent ID creates the resource (upsert).
     """
     data = {
         "first_name": "New",
         "last_name": "Person",
         "country_code": "PH",
     }
     response = client.put("/persons/99999", json=data)
-    # Current implementation returns 204 regardless because merge() can create.
     assert response.status_code == 204
 
-    # Verify merge() actually created the row in the database
+    # Verify the upsert actually created the row in the database
     person_from_db: Person = db_session.execute(
         select(Person).where(Person.id == 99999)
     ).scalar_one()

tests/api_tests/test_persons.py

@@ -196,7 +196,6 @@ def test_update_person(init_db: None, client: TestClient, db_session: Session) -
 
 
 def test_upsert_create(init_db: None, client: TestClient, db_session: Session) -> None:
-    # Use an ID that doesn't exist in seed data (seed data has 13 persons, IDs likely 1-13)
     new_id = 999
     first_name = "Upsert"
     last_name = "Create"
@@ -207,15 +206,12 @@ def test_upsert_create(init_db: None, client: TestClient, db_session: Session) -
         "country_code": country_code,
     }
 
-    # Verify it doesn't exist
     stmt_check = select(Person).where(Person.id == new_id)
     assert db_session.execute(stmt_check).scalar_one_or_none() is None
 
-    # Perform PUT (UPSERT)
     response = client.put(f"/persons/{new_id}", json=data)
     assert response.status_code == 204
 
-    # Verify it was created
     person_from_db = db_session.execute(stmt_check).scalar_one()
     assert person_from_db.id == new_id
     assert person_from_db.first_name == first_name
@@ -224,7 +220,6 @@ def test_upsert_create(init_db: None, client: TestClient, db_session: Session) -
 
 
 def test_upsert_update(init_db: None, client: TestClient, db_session: Session) -> None:
-    # Use an existing ID from seed data
     existing_id = 1
     first_name = "Upsert"
     last_name = "Update"
@@ -235,19 +230,42 @@ def test_upsert_update(init_db: None, client: TestClient, db_session: Session) -
         "country_code": country_code,
     }
 
-    # Verify it exists and is different
     stmt_check = select(Person).where(Person.id == existing_id)
     person_before = db_session.execute(stmt_check).scalar_one()
     assert person_before.first_name != first_name
 
-    # Perform PUT (UPSERT)
     response = client.put(f"/persons/{existing_id}", json=data)
     assert response.status_code == 204
 
-    # Verify it was updated
-    db_session.expire_all()  # Ensure we get fresh data
+    db_session.expire_all()
     person_after = db_session.execute(stmt_check).scalar_one()
     assert person_after.id == existing_id
     assert person_after.first_name == first_name
     assert person_after.last_name == last_name
     assert person_after.country_code == country_code
+
+
+def test_delete_person(init_db: None, client: TestClient, db_session: Session) -> None:
+    person_id = 1
+    # Verify the person exists before deletion
+    stmt_check = select(Person).where(Person.id == person_id)
+    person_before = db_session.execute(stmt_check).scalar_one()
+    assert person_before is not None
+
+    response = client.delete(f"/persons/{person_id}")
+    assert response.status_code == 204
+
+    # Verify the person no longer exists
+    person_after = db_session.execute(stmt_check).scalar_one_or_none()
+    assert person_after is None
+
+    # Verify total count decreased by 1
+    count: int = db_session.execute(select(func.count()).select_from(Person)).scalar_one()
+    assert count == NUM_SEED_DATA - 1
+
+
+def test_delete_person_not_found(init_db: None, client: TestClient, db_session: Session) -> None:
+    person_id = 999
+    response = client.delete(f"/persons/{person_id}")
+    assert response.status_code == 404
+    assert response.json() == {"detail": f"Person {person_id} not found"}

tests/unit_tests/services/test_person_service.py

@@ -26,7 +26,9 @@ def mock_db() -> MagicMock:
 
 
 @pytest.fixture
-def person_service(mock_person_repo: MagicMock, mock_country_repo: MagicMock, mock_db: MagicMock) -> PersonService:
+def person_service(
+    mock_person_repo: MagicMock, mock_country_repo: MagicMock, mock_db: MagicMock
+) -> PersonService:
     return PersonService(mock_person_repo, mock_country_repo, mock_db)
 
 
@@ -41,7 +43,9 @@ def test_get_persons(self, person_service: PersonService, mock_person_repo: Magi
         mock_person_repo.get_persons.assert_called_once_with(q, sort)
         assert result == mock_person_repo.get_persons.return_value
 
-    def test_get_person_success(self, person_service: PersonService, mock_person_repo: MagicMock) -> None:
+    def test_get_person_success(
+        self, person_service: PersonService, mock_person_repo: MagicMock
+    ) -> None:
         person_id = 1
         expected_person = Person(id=person_id, first_name="John", last_name="Doe")
         mock_person_repo.get_person.return_value = expected_person
@@ -51,7 +55,9 @@ def test_get_person_success(self, person_service: PersonService, mock_person_rep
         mock_person_repo.get_person.assert_called_once_with(person_id)
         assert result == expected_person
 
-    def test_get_person_not_found(self, person_service: PersonService, mock_person_repo: MagicMock) -> None:
+    def test_get_person_not_found(
+        self, person_service: PersonService, mock_person_repo: MagicMock
+    ) -> None:
         person_id = 999
         mock_person_repo.get_person.return_value = None
 
@@ -62,7 +68,11 @@ def test_get_person_not_found(self, person_service: PersonService, mock_person_r
         assert str(person_id) in exc.value.message
 
     def test_create_person_success(
-        self, person_service: PersonService, mock_person_repo: MagicMock, mock_country_repo: MagicMock, mock_db: MagicMock
+        self,
+        person_service: PersonService,
+        mock_person_repo: MagicMock,
+        mock_country_repo: MagicMock,
+        mock_db: MagicMock,
     ) -> None:
         body = PersonCreateSchema(first_name="Jane", last_name="Doe", country_code="US")
         mock_country_repo.get_country.return_value = MagicMock(spec=Country)
@@ -78,7 +88,9 @@ def test_create_person_success(
         mock_db.refresh.assert_called_once_with(created_person)
         assert result == created_person
 
-    def test_create_person_country_not_found(self, person_service: PersonService, mock_country_repo: MagicMock) -> None:
+    def test_create_person_country_not_found(
+        self, person_service: PersonService, mock_country_repo: MagicMock
+    ) -> None:
         body = PersonCreateSchema(first_name="Jane", last_name="Doe", country_code="XX")
         mock_country_repo.get_country.return_value = None
 
@@ -87,31 +99,64 @@ def test_create_person_country_not_found(self, person_service: PersonService, mo
 
         mock_country_repo.get_country.assert_called_once_with("XX")
 
-    def test_update_person_success(
-        self, person_service: PersonService, mock_person_repo: MagicMock, mock_country_repo: MagicMock, mock_db: MagicMock
+    def test_update_or_create_person_success(
+        self,
+        person_service: PersonService,
+        mock_person_repo: MagicMock,
+        mock_country_repo: MagicMock,
+        mock_db: MagicMock,
     ) -> None:
         person_id = 1
         body = PersonCreateSchema(first_name="Jane", last_name="Doe", country_code="US")
         mock_country_repo.get_country.return_value = MagicMock(spec=Country)
 
-        person_service.update_person(person_id, body)
+        person_service.update_or_create_person(person_id, body)
 
         mock_country_repo.get_country.assert_called_once_with("US")
         mock_person_repo.update_or_create_person.assert_called_once()
-        # Verify the person object passed to repo has correct ID
         args, _ = mock_person_repo.update_or_create_person.call_args
         passed_person = args[0]
         assert passed_person.id == person_id
         assert passed_person.first_name == "Jane"
 
         mock_db.commit.assert_called_once()
 
-    def test_update_person_country_not_found(self, person_service: PersonService, mock_country_repo: MagicMock) -> None:
+    def test_update_or_create_person_country_not_found(
+        self, person_service: PersonService, mock_country_repo: MagicMock
+    ) -> None:
         person_id = 1
         body = PersonCreateSchema(first_name="Jane", last_name="Doe", country_code="XX")
         mock_country_repo.get_country.return_value = None
 
         with pytest.raises(CountryNotFoundError):
-            person_service.update_person(person_id, body)
+            person_service.update_or_create_person(person_id, body)
 
         mock_country_repo.get_country.assert_called_once_with("XX")
+
+    def test_delete_person_success(
+        self,
+        person_service: PersonService,
+        mock_person_repo: MagicMock,
+        mock_db: MagicMock,
+    ) -> None:
+        person_id = 1
+        existing_person = Person(id=person_id, first_name="John", last_name="Doe", country_code="US")
+        mock_person_repo.get_person.return_value = existing_person
+
+        person_service.delete_person(person_id)
+
+        mock_person_repo.get_person.assert_called_once_with(person_id)
+        mock_person_repo.delete_person.assert_called_once_with(existing_person)
+        mock_db.commit.assert_called_once()
+
+    def test_delete_person_not_found(
+        self, person_service: PersonService, mock_person_repo: MagicMock
+    ) -> None:
+        person_id = 999
+        mock_person_repo.get_person.return_value = None
+
+        with pytest.raises(PersonNotFoundError) as exc:
+            person_service.delete_person(person_id)
+
+        assert exc.value.status_code == 404
+        assert str(person_id) in exc.value.message