diff --git a/app/routes.py b/app/routes.py index d59bb47..e56f6da 100644 --- a/app/routes.py +++ b/app/routes.py @@ -2,6 +2,7 @@ import copy +from bson.objectid import ObjectId from flask import jsonify, request from pymongo.errors import ConnectionFailure from werkzeug.exceptions import HTTPException, NotFound @@ -88,7 +89,6 @@ def add_book(): host = request.host_url # Send the host and new book_id to the helper function to generate links final_book_for_api = append_hostname(book_from_db, host) - print("final_book_for_api", final_book_for_api) # Transform _id to id and remove the internal _id final_book_for_api["id"] = str(final_book_for_api["_id"]) @@ -141,20 +141,36 @@ def get_book(book_id): """ Retrieve a specific book by its unique ID. """ - if not books: - return jsonify({"error": "Book collection not initialized"}), 500 + # get the collection + collection = get_book_collection() - # extract host from the request + if collection is None: + return jsonify({"error": "Book collection not found"}), 500 + + # sanity check book_id + if not ObjectId.is_valid(book_id): + return jsonify({"error": "Invalid book ID format"}), 400 + obj_id = ObjectId(book_id) + + # Query db for a non-deleted book + query = {"_id": obj_id, "state": {"$ne": "deleted"}} + # look it up in MongoDB + book = collection.find_one(query) + # also equivalent to Key version + # book = raw_books.find_one(_id=obj_id, state={"$ne": "deleted"}) + + if book is None: + return jsonify({"error": "Book not found"}), 404 + + # Format for API response host = request.host_url + formatted_book = append_hostname(book, host) - for book in books: - if book.get("id") == book_id and book.get("state") != "deleted": - # copy the book - book_copy = copy.deepcopy(book) - book_copy.pop("state", None) - # Add the hostname to the book_copy object and return it - return jsonify(append_hostname(book_copy, host)), 200 - return jsonify({"error": "Book not found"}), 404 + formatted_book["id"] = str(formatted_book["_id"]) + formatted_book.pop("state", None) + formatted_book.pop("_id", None) + + return jsonify(formatted_book), 200 # ----------- DELETE section ------------------ @app.route("/books/", methods=["DELETE"]) diff --git a/openapi.yml b/openapi.yml index edd8d69..901fa91 100644 --- a/openapi.yml +++ b/openapi.yml @@ -135,6 +135,15 @@ components: required: - error + SimpleError: + type: object + properties: + error: + type: string + description: A brief error message. + required: + - error + # API Error: Reusable responses for common errors responses: BadRequest: @@ -263,10 +272,32 @@ paths: application/json: schema: $ref: '#/components/schemas/BookOutput' + '400': + description: |- + Bad Request. The provided book_id is not a valid 24-character hexadecimal string. + content: + application/json: + schema: + $ref: '#/components/schemas/SimpleError' + example: + error: "Invalid book ID format" '404': - $ref: '#/components/responses/NotFound' - '500': - $ref: '#/components/responses/InternalServerError' + description: A book with the specified ID was not found. + content: + application/json: + schema: + $ref: '#/components/schemas/SimpleError' + example: + error: "Book not found" + '500': + description: |- + Service Unavailable. The server cannot connect to the database. + content: + application/json: + schema: + $ref: '#/components/schemas/SimpleError' + example: + error: "Book collection not found" # -------------------------------------------- put: diff --git a/tests/conftest.py b/tests/conftest.py index 9d80751..4982b70 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,6 +10,7 @@ import pytest from app import create_app +from app.datastore.mongo_db import get_book_collection @pytest.fixture(name="_insert_book_to_db") @@ -84,3 +85,23 @@ def test_app(): def client(test_app): # pylint: disable=redefined-outer-name """A test client for the app.""" return test_app.test_client() + + +@pytest.fixture(scope="function") +def db_setup(test_app): # pylint: disable=redefined-outer-name + """ + Sets up and tears down the database for a test. + Scope is "function" to ensure a clean DB for each test. + """ + # Use app_context to access the database + with test_app.app_context(): + collection = get_book_collection() + + collection.delete_many({}) + # Pass control to the test function + yield + + # Teardown: code runs after the test is finished + with test_app.app_context(): + collection = get_book_collection() + collection.delete_many({}) diff --git a/tests/test_app.py b/tests/test_app.py index 7b907fc..b0160ad 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -6,7 +6,7 @@ from bson.objectid import ObjectId from pymongo.errors import ConnectionFailure, ServerSelectionTimeoutError -from app import create_app +from app import create_app, routes from app.datastore.mongo_db import get_book_collection # Mock book database object @@ -362,47 +362,150 @@ def test_get_books_handles_database_connection_error(mock_get_collection, client # -------- Tests for GET a single resource ---------------- -def test_get_book_returns_specified_book(client): - # Test GET request using the book ID - get_response = client.get("/books/1") +def test_get_book_happy_path_unit_test(client, monkeypatch): + # Arrange: + fake_book_id = ObjectId() + fake_book_id_str = str(fake_book_id) + fake_book_from_db = { + "_id": fake_book_id, + "title": "A Mocked Book", + "author": "The Mockist", + "synopsis": "A tale of fakes and stubs.", + "state": "active", + "links": {}, # This will be populated by the append_hostname helper + } + + # intercept the call to the service function + mock_collection = MagicMock() + mock_collection.find_one.return_value = fake_book_from_db + + # use monkeypatch to replace the get_book_collection + monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) + + # ACT + get_response = client.get(f"/books/{fake_book_id_str}") + + # ASSERT assert get_response.status_code == 200 - assert get_response.content_type == "application/json" - returned_book = get_response.get_json() - assert returned_book["id"] == "1" - assert returned_book["title"] == "The Great Adventure" + response_data = get_response.get_json() + assert response_data["title"] == "A Mocked Book" + assert response_data["id"] == fake_book_id_str + mock_collection.find_one.assert_called_once() + mock_collection.find_one.assert_called_once_with( + {"_id": fake_book_id, "state": {"$ne": "deleted"}} + ) # pylint: disable=line-too-long + + +def test_get_book_returns_specified_book( + client, db_setup +): # pylint: disable=unused-argument + """This is an INTEGRATION test""" + # The 'db_setup' fixture has already cleaned the db + # and provided the app.context + + with client.application.app_context(): + # GIVEN: Setup the db + collection = get_book_collection() + + sample_book = { + "_id": ObjectId(), # Generate a new valid ObjectId + "title": "Test Driven Development", + "author": "Kent Beck", + "synopsis": "A guide to TDD.", + "state": "active", + "links": {}, # can be empty for this test + } + collection.insert_one(sample_book) + book_id_str = str(sample_book["_id"]) + # ACT + get_response = client.get(f"/books/{book_id_str}") + assert get_response.status_code == 200 + assert get_response.content_type == "application/json" + response_data = get_response.get_json() + assert response_data["id"] == book_id_str + assert response_data["title"] == "Test Driven Development" + assert response_data["author"] == "Kent Beck" -def test_get_book_not_found_returns_404(client): - # Test GET request using invalid book ID - response = client.get("/books/12341234") - assert response.status_code == 404 + +def test_get_book_with_invalid_id_format_returns_400(client, db_setup): # pylint: disable=unused-argument + # Arrange + # an ID that is clearly not a valid MongoDB ObjectId + invalid_book_id = "this-is-not-a-valid-id" + + # Act + response = client.get(f"/books/{invalid_book_id}") + + # Assert + assert response.status_code == 400 assert response.content_type == "application/json" - assert "Book not found" in response.get_json()["error"] + # Check that the JSON error message is exactly what the code returns + expected_error = {"error": "Invalid book ID format"} + assert response.get_json() == expected_error + +def test_get_book_not_found_returns_404(client, monkeypatch): + """ + WHEN given a well-formed but non-existent ObjectId, + Returns a 404 error + """ + # Arrange + valid_but_missing_id = ObjectId() + valid_id_str = str(valid_but_missing_id) + + # Mock the collection to return None (book not in DB) + mock_collection = MagicMock() + mock_collection.find_one.return_value = None + monkeypatch.setattr(routes, "get_book_collection", lambda: mock_collection) + + # ACT: Test GET request using invalid book ID + response = client.get(f"/books/{valid_id_str}") -def test_invalid_urls_return_404(client): - # Test invalid URL - response = client.get("/books/") assert response.status_code == 404 assert response.content_type == "application/json" - assert "404 Not Found" in response.get_json()["error"] + assert "Book not found" in response.get_json()["error"] -def test_book_database_is_initialized_for_specific_book_route(client): - with patch("app.routes.books", None): - response = client.get("/books/1") - assert response.status_code == 500 - assert "Book collection not initialized" in response.get_json()["error"] +def test_book_database_is_initialized_for_specific_book_route(client, monkeypatch): + """ + WHEN get_book_collection() returns None + THEN the /books/ route should return HTTP 500 + """ + # Arrange + valid_id = ObjectId() + valid_id_str = str(valid_id) + + monkeypatch.setattr(routes, "get_book_collection", lambda: None) + + response = client.get(f"/books/{valid_id_str}") + assert response.status_code == 500 + assert "Book collection not found" in response.get_json()["error"] + + +def test_get_book_returns_404_if_state_equals_deleted(client, monkeypatch): + # Arrange + valid_id = ObjectId() + valid_id_str = str(valid_id) + # Mock the collection to return None (book state deleted) + mock_collection = MagicMock() + mock_collection.find_one.return_value = None + monkeypatch.setattr(routes, "get_book_collection", lambda: mock_collection) -def test_get_book_returns_404_if_state_equals_deleted(client): - book_id = "3" - response = client.get(f"/books/{book_id}") + response = client.get(f"/books/{valid_id_str}") assert response.status_code == 404 assert response.content_type == "application/json" assert "Book not found" in response.get_json()["error"] +def test_invalid_urls_return_404(client): + # Test invalid URL + response = client.get("/books/") + assert response.status_code == 404 + assert response.content_type == "application/json" + assert "404 Not Found" in response.get_json()["error"] + + # ------------------------ Tests for DELETE -------------------------------------------- @@ -685,33 +788,77 @@ def test_append_host_to_links_in_get(mock_find_books, client): assert book["links"]["self"].endswith(f"books/{new_book_id}") -def test_append_host_to_links_in_get_book(client): +def test_append_host_to_links_in_get_book(client, monkeypatch): + """ + GIVEN a request for a book ID + WHEN the database and helper functions are mocked + THEN the route handler correctly calls its dependencies and formats the final JSON response. + """ + # Arrange: define constants and mock return values + book_id = ObjectId() + book_id_str = str(book_id) + book_from_db = { + "_id": book_id, + "title": "Some Title", + "author": "Some Author", + "synopsis": "Foo bar.", + "state": "active", + "links": {}, + } + # Mock the external dependencies + fake_collection = MagicMock() + fake_collection.find_one.return_value = book_from_db + # Patch the function that the route uses to get the collection + monkeypatch.setattr(routes, "get_book_collection", lambda: fake_collection) + + # mock append_hostname helper, define its output + mock_appender = MagicMock( + return_value={ + **book_from_db, + "links": { + "self": f"http://localhost/books/{book_id_str}", + "reservations": f"http://localhost/books/{book_id_str}", + "reviews": f"http://localhost/books/{book_id_str}", + }, + } + ) + monkeypatch.setattr(routes, "append_hostname", mock_appender) - response = client.get("/books/1") + # ACT + response = client.get(f"/books/{book_id_str}") + # Assert assert response.status_code == 200 - assert response.headers["content-type"] == "application/json" - - # Get the response data, the ID and links response_data = response.get_json() - book_id = response_data.get("id") - links = response_data.get("links") - - assert book_id is not None, "Response JSON must contain an 'id'" - assert links is not None, "Response JSON must contain a 'links' object" + assert response_data["id"] == book_id_str + assert "links" in response_data + # Assert the final JSON is correct (based on the mock_appender's return value) + assert ( + response_data.get("links", {}).get("self") + == f"http://localhost/books/{book_id_str}" + ) - self_link = links.get("self") - assert self_link is not None, "'links' object must contain a 'self' link" + # Now specifically check the 'self' link + self_link = response_data["links"]["self"] + reservations_link = response_data["links"]["reservations"] + reviews_link = response_data["links"]["reviews"] + assert self_link, "Expected a 'self' link in the response" + assert reservations_link + assert reviews_link - expected_link_start = "http://localhost" + # By default Flask's test_client serves on http://localhost/ assert self_link.startswith( - expected_link_start - ), f"Link should start with the test server's hostname '{expected_link_start}'" + "http://localhost" + ), f"Expected the link to start with 'http://localhost', got {self_link!r}" - expected_path = f"/books/{book_id}" + # And it must end with the resource path assert self_link.endswith( - expected_path - ), f"Link should end with the resource path '{expected_path}'" + f"/books/{book_id_str}" + ), f"Expected link to end with '/books/{book_id_str}', got {self_link!r}" + # Assert the internal behavior was correct + expected_query = {"_id": book_id, "state": {"$ne": "deleted"}} + fake_collection.find_one.assert_called_once_with(expected_query) + mock_appender.assert_called_once_with(book_from_db, ANY) def test_append_host_to_links_in_put(client):