Skip to content

Commit 1747121

Browse files
authored
Complete mongo db integration for put /books/{id}) (#10)
* Add update_book_by_id which uses replace_one for full MongoDB doc replacement * Implementing proper payload validation with validate_book_put_payload() * Add/update tests with updated mocks/dummy data to reflect mongodb integration * Sort missing fields so response is predictable * Move shared constants to tests/test_data.py to reduce duplication (R0801) * Move append_hostname test to separate file to fix pylint length * Update openapi to reflect new PUT * Update tests/test_app_utils_helper.py unpack with '_' vs kwargs, indicate intentional throwaway * Update app/datastore/mongo_helper.py Use pymongo Collection type for book_collection annotation
1 parent 4b98e55 commit 1747121

File tree

8 files changed

+688
-410
lines changed

8 files changed

+688
-410
lines changed

app/datastore/mongo_helper.py

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
"""Module containing pymongo helper functions."""
22

3-
from bson.objectid import ObjectId
3+
from bson.objectid import InvalidId, ObjectId
44
from pymongo.cursor import Cursor
5+
from pymongo.collection import Collection
56

67

78
def insert_book_to_mongo(book_data, collection):
@@ -65,7 +66,7 @@ def find_books(collection, query_filter=None, projection=None, limit=None) -> Cu
6566
return cursor
6667

6768

68-
def delete_book_by_id(book_collection, book_id):
69+
def delete_book_by_id(book_collection: Collection, book_id: str):
6970
"""
7071
Soft delete book with given id
7172
Returns: The original document if found and updated, otherwise None.
@@ -81,3 +82,55 @@ def delete_book_by_id(book_collection, book_id):
8182
result = book_collection.find_one_and_update(filter_query, update_operation)
8283

8384
return result
85+
86+
# ------ PUT helpers ------------
87+
88+
def validate_book_put_payload(payload: dict):
89+
"""
90+
Validates the payload for a PUT request.
91+
A PUT must contain all required fields and no extra fields
92+
93+
Returns:
94+
A tuple: (is_valid, error_dictionary)
95+
If valid, error_dictionary is None.
96+
"""
97+
if not isinstance(payload, dict):
98+
return False, {"error": "JSON payload must be a dictionary"}
99+
100+
required_fields = {"title", "synopsis", "author"}
101+
payload_keys = set(payload.keys())
102+
103+
# Check 1: any missing fields?
104+
missing_fields = required_fields - payload_keys
105+
if missing_fields:
106+
# Convert the set to a list and sort it.
107+
sorted_missing = sorted(list(missing_fields))
108+
return False, {"error": f"Missing required fields: {', '.join(sorted_missing)}"}
109+
110+
# Check 2: Any extra, unexpected fields?
111+
extra_fields = payload_keys - required_fields
112+
if extra_fields:
113+
return False, {
114+
"error": f"Unexpected fields provided: {', '.join(sorted(list(extra_fields)))}"
115+
}
116+
117+
# If all checks pass:
118+
return True, None
119+
120+
121+
def replace_book_by_id(book_collection, book_id, new_data):
122+
"""
123+
Updates an ENTIRE book document in the database.
124+
Returns True on success, False if book not found.
125+
"""
126+
try:
127+
object_id_to_update = ObjectId(book_id)
128+
129+
except InvalidId:
130+
return False
131+
132+
# use $set operator to update the fields OR
133+
# create them if they don't exist
134+
result = book_collection.replace_one({"_id": object_id_to_update}, new_data)
135+
136+
return result.matched_count > 0

app/routes.py

Lines changed: 39 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
"""Flask application module for managing a collection of books."""
22

3-
import copy
4-
53
from bson.objectid import InvalidId, ObjectId
64
from flask import jsonify, request
75
from pymongo.errors import ConnectionFailure
86
from werkzeug.exceptions import HTTPException, NotFound
97

108
from app.datastore.mongo_db import get_book_collection
11-
from app.datastore.mongo_helper import delete_book_by_id, insert_book_to_mongo
9+
from app.datastore.mongo_helper import (delete_book_by_id,
10+
insert_book_to_mongo,
11+
replace_book_by_id,
12+
validate_book_put_payload)
1213
from app.services.book_service import fetch_active_books, format_books_for_api
1314
from app.utils.api_security import require_api_key
1415
from app.utils.helper import append_hostname
15-
from data import books
1616

1717

1818
def register_routes(app): # pylint: disable=too-many-statements
@@ -200,52 +200,47 @@ def delete_book(book_id):
200200
def update_book(book_id):
201201
"""
202202
Update a book by its unique ID using JSON from the request body.
203-
Returns a single dictionary with the updated book's details.
203+
Finds, replaces, and returns the single updated document.
204204
"""
205-
if not books:
205+
# VALIDATION
206+
request_body = request.get_json(silent=True)
207+
if request_body is None:
208+
return jsonify({"error": "Request must be valid JSON"}), 400
209+
210+
is_valid, error = validate_book_put_payload(request_body)
211+
if not is_valid:
212+
return jsonify(error), 400
213+
214+
# DATABASE INTERACTION
215+
book_collection = get_book_collection()
216+
if not book_collection:
206217
return jsonify({"error": "Book collection not initialized"}), 500
207218

208-
# check if request is json
209-
if not request.is_json:
210-
return jsonify({"error": "Request must be JSON"}), 415
211-
212-
# check request body is valid
213-
request_body = request.get_json()
214-
if not isinstance(request_body, dict):
215-
return jsonify({"error": "JSON payload must be a dictionary"}), 400
216-
217-
# check request body contains required fields
218-
required_fields = ["title", "synopsis", "author"]
219-
missing_fields = [
220-
field for field in required_fields if field not in request_body
221-
]
222-
if missing_fields:
223-
return {
224-
"error": f"Missing required fields: {', '.join(missing_fields)}"
225-
}, 400
219+
was_replaced = replace_book_by_id(book_collection, book_id, request_body)
220+
if not was_replaced:
221+
return jsonify({"error": f"Book with ID '{book_id}' not found"}), 404
222+
223+
# RESPONSE HANDLING: Format API response
224+
# replace_one doesn't return the document, so we must fetch it to return it.
225+
updated_book = book_collection.find_one({"_id": ObjectId(book_id)})
226+
# Convert ObjectId to string for the JSON response
227+
updated_book["id"] = str(updated_book["_id"])
228+
229+
# Add HATEOAS links
230+
host = request.host_url.strip("/") # Use rstrip to avoid double slashes
231+
book_obj_id = updated_book["id"]
232+
updated_book["links"] = {
233+
"self": f"/books/{book_obj_id}",
234+
"reservations": f"/books/{book_obj_id}/reservations",
235+
"reviews": f"/books/{book_obj_id}/reviews",
236+
}
226237

227-
host = request.host_url
238+
updated_book_with_hostname = append_hostname(updated_book, host)
228239

229-
# now that we have a book object that is valid, loop through books
230-
for book in books:
231-
if book.get("id") == book_id:
232-
# update the book values to what is in the request
233-
book["title"] = request.json.get("title")
234-
book["synopsis"] = request.json.get("synopsis")
235-
book["author"] = request.json.get("author")
236-
237-
# Add links exists as paths only
238-
book["links"] = {
239-
"self": f"/books/{book_id}",
240-
"reservations": f"/books/{book_id}/reservations",
241-
"reviews": f"/books/{book_id}/reviews",
242-
}
243-
# make a deepcopy of the modified book
244-
book_copy = copy.deepcopy(book)
245-
book_with_hostname = append_hostname(book_copy, host)
246-
return jsonify(book_with_hostname), 200
240+
# Remove internal '_id' field
241+
del updated_book_with_hostname["_id"]
247242

248-
return jsonify({"error": "Book not found"}), 404
243+
return jsonify(updated_book_with_hostname), 200
249244

250245
# ----------- CUSTOM ERROR HANDLERS ------------------
251246

openapi.yml

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,10 @@ paths:
303303
put:
304304
tags:
305305
- Books
306-
summary: Update a book
306+
summary: Update a book by replacement
307307
security:
308308
- ApiKeyAuth: []
309-
description: Updates an existing book by its unique ID. The entire book object must be provided in the request body.
309+
description: Updates an existing book by its unique ID. This is a full replacement operation (HTTP PUT). The request body must contain all required fields (`title`, `synopsis`, `author`) and no extra fields.
310310
operationId: updateBook
311311
parameters:
312312
- name: book_id
@@ -318,7 +318,7 @@ paths:
318318
pattern: '^[a-f\d]{24}$'
319319
example: "635f3a7e3a8e3bcfc8e6a1e0"
320320
requestBody:
321-
description: Book object that needs to be updated.
321+
description: A complete Book object to replace the existing one.
322322
required: true
323323
content:
324324
application/json:
@@ -332,15 +332,55 @@ paths:
332332
schema:
333333
$ref: '#/components/schemas/BookOutput'
334334
'400':
335-
$ref: '#/components/responses/BadRequest'
335+
description: |-
336+
Bad Request. The request is invalid for one of the following reasons:
337+
- The request body is not valid JSON.
338+
- The JSON payload is missing required fields.
339+
- The JSON payload contains unexpected fields.
340+
content:
341+
application/json:
342+
schema:
343+
$ref: '#/components/schemas/SimpleError'
344+
examples:
345+
missingFields:
346+
summary: Missing Fields Error
347+
value:
348+
error: "Missing required fields: author, synopsis"
349+
extraFields:
350+
summary: Extra Fields Error
351+
value:
352+
error: "Unexpected fields provided: publication_year"
353+
invalidJsonBody:
354+
summary: Invalid JSON
355+
value:
356+
error: "Request must be valid JSON"
336357
'401':
337358
$ref: '#/components/responses/Unauthorized'
338359
'404':
339-
$ref: '#/components/responses/NotFound'
360+
description: The book with the specified ID was not found.
361+
content:
362+
application/json:
363+
schema:
364+
$ref: '#/components/schemas/SimpleError'
365+
example:
366+
error: "Book with ID '635f3a7e3a8e3bcfc8e6a1e1' not found"
340367
'415':
341-
$ref: '#/components/responses/UnsupportedMediaType'
368+
description: |-
369+
Unsupported Media Type. The client sent a request with a Content-Type header other than `application/json`.
370+
content:
371+
application/json:
372+
schema:
373+
$ref: '#/components/schemas/SimpleError'
374+
example:
375+
error: "Request must have Content-Type: application/json"
342376
'500':
343-
$ref: '#/components/responses/InternalServerError'
377+
description: An internal server error occurred, likely related to the database connection.
378+
content:
379+
application/json:
380+
schema:
381+
$ref: '#/components/schemas/SimpleError'
382+
example:
383+
error: "Book collection not initialized"
344384

345385
# --------------------------------------------
346386

tests/test_api_security.py

Lines changed: 45 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,8 @@
88
import pytest
99
from bson.objectid import ObjectId
1010

11-
# A dictionary for headers to keep things clean
12-
HEADERS = {
13-
"VALID": {"X-API-KEY": "test-key-123"},
14-
"INVALID": {"X-API-KEY": "This-is-the-wrong-key-12345"},
15-
"MISSING": {},
16-
}
17-
18-
# A sample payload for POST/PUT requests
19-
DUMMY_PAYLOAD = {
20-
"title": "A Test Book",
21-
"synopsis": "A test synopsis.",
22-
"author": "Tester McTestFace",
23-
}
11+
from tests.test_data import HEADERS, DUMMY_PAYLOAD
12+
2413

2514
# -------------- LOGGING --------------------------
2615

@@ -125,42 +114,54 @@ def test_add_book_fails_if_api_key_not_configured_on_the_server(client, test_app
125114
# -------------- PUT --------------------------
126115
def test_update_book_succeeds_with_valid_api_key(monkeypatch, client):
127116
"""Test successful book update with valid API key."""
117+
# Arrange
118+
test_book_id = "65a9a4b3f3a2c4a8c2b3d4e5"
128119

129-
# 1. Patch the books list
130-
test_books = [
131-
{
132-
"id": "abc123",
133-
"title": "Old Title",
134-
"synopsis": "Old Synopsis",
135-
"author": "Old Author",
136-
}
137-
]
138-
monkeypatch.setattr("app.routes.books", test_books)
139-
140-
# 2. Patch append_hostname to just return the book
141-
monkeypatch.setattr("app.routes.append_hostname", lambda book, host: book)
120+
mock_collection = MagicMock()
121+
mock_collection.replace_one.return_value.matched_count = 1
122+
123+
expected_book_from_db = {
124+
"_id": ObjectId(test_book_id),
125+
# Use dictionary unpacking to merge our payload
126+
**DUMMY_PAYLOAD,
127+
}
128+
mock_collection.find_one.return_value = expected_book_from_db
142129

143-
# 3. Call the endpoint with request details
144-
response = client.put("/books/abc123", json=DUMMY_PAYLOAD, headers=HEADERS["VALID"])
130+
# monkeypatcj to replace the real get_book_collection with a function
131+
monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection)
132+
133+
# ACT
134+
response = client.put(
135+
f"/books/{test_book_id}", json=DUMMY_PAYLOAD, headers=HEADERS["VALID"]
136+
)
145137

146-
# 4. Assert response
138+
# ASSERT 1
147139
assert response.status_code == 200
148-
assert response.json["title"] == "A Test Book"
140+
response_data = response.json
141+
assert response_data["id"] == test_book_id
142+
assert response_data["title"] == DUMMY_PAYLOAD["title"]
143+
assert "links" in response_data
144+
assert response_data["links"]["self"].endswith(f"/books/{test_book_id}")
149145

146+
# Assert 2
147+
mock_collection.replace_one.assert_called_once()
148+
mock_collection.replace_one.assert_called_with(
149+
{"_id": ObjectId(test_book_id)}, DUMMY_PAYLOAD
150+
)
151+
mock_collection.find_one.assert_called_once()
152+
mock_collection.find_one.assert_called_with({"_id": ObjectId(test_book_id)})
150153

151-
def test_update_book_fails_with_missing_api_key(monkeypatch, client):
152-
"""Should return 401 if no API key is provided."""
153154

154-
monkeypatch.setattr("app.routes.books", [])
155+
def test_update_book_fails_with_missing_api_key(client):
156+
"""Should return 401 if no API key is provided."""
155157

156158
response = client.put("/books/abc123", json=DUMMY_PAYLOAD)
157159

158160
assert response.status_code == 401
159161
assert "API key is missing." in response.json["error"]["message"]
160162

161163

162-
def test_update_book_fails_with_invalid_api_key(client, monkeypatch):
163-
monkeypatch.setattr("app.routes.books", [])
164+
def test_update_book_fails_with_invalid_api_key(client):
164165

165166
response = client.put(
166167
"/books/abc123", json=DUMMY_PAYLOAD, headers=HEADERS["INVALID"]
@@ -170,17 +171,17 @@ def test_update_book_fails_with_invalid_api_key(client, monkeypatch):
170171
assert "Invalid API key." in response.json["error"]["message"]
171172

172173

173-
def test_update_book_fails_if_api_key_not_configured_on_the_server(
174-
client, test_app, monkeypatch
175-
):
176-
# ARRANGE: Remove API_KEY from the test_app config
177-
monkeypatch.setattr("app.routes.books", [])
178-
test_app.config.pop("API_KEY", None)
174+
# def test_update_book_fails_if_api_key_not_configured_on_the_server(
175+
# client, test_app, monkeypatch
176+
# ):
177+
# # ARRANGE: Remove API_KEY from the test_app config
178+
# monkeypatch.setattr("app.routes.books", [])
179+
# test_app.config.pop("API_KEY", None)
179180

180-
response = client.put("/books/abc123", json=DUMMY_PAYLOAD)
181+
# response = client.put("/books/abc123", json=DUMMY_PAYLOAD)
181182

182-
assert response.status_code == 500
183-
assert "API key not configured on the server." in response.json["error"]["message"]
183+
# assert response.status_code == 500
184+
# assert "API key not configured on the server." in response.json["error"]["message"]
184185

185186

186187
# -------------- DELETE --------------------------

0 commit comments

Comments
 (0)