fix download document version endpoint

papermerge/app.py

@@ -11,6 +11,9 @@
 from papermerge.core.features.nodes.router_thumbnails import router as thumbnails_router
 from papermerge.core.features.document.router import router as document_router
 from papermerge.core.features.document.router_pages import router as pages_router
+from papermerge.core.features.document.router_document_version import (
+    router as document_versions_router,
+)
 from papermerge.search.routers.search import router as search_router
 
 from papermerge.core.version import __version__
@@ -33,6 +36,7 @@
 app.include_router(folders_router, prefix=prefix)
 app.include_router(thumbnails_router, prefix=prefix)
 app.include_router(document_router, prefix=prefix)
+app.include_router(document_versions_router, prefix=prefix)
 app.include_router(pages_router, prefix=prefix)
 app.include_router(dt_router, prefix=prefix)
 app.include_router(cf_router, prefix=prefix)

papermerge/core/dbapi.py

@@ -1,8 +1,9 @@
 from .features.page_mngm.db.api import move_pages
-from .features.document.db.api import get_last_doc_ver, upload
+from .features.document.db.api import get_last_doc_ver, upload, get_doc_ver
 
 __all__ = [
     "move_pages",
     "get_last_doc_ver",
+    "get_doc_ver",
     "upload"
 ]

papermerge/core/features/conftest.py

@@ -27,6 +27,9 @@
 from papermerge.core.features.nodes import router_folders as folders_router
 from papermerge.core.features.document import router as docs_router
 from papermerge.core.features.document import router_pages as pages_router
+from papermerge.core.features.document import (
+    router_document_version as document_versions_router,
+)
 from papermerge.core.features.nodes import router_thumbnails as thumbnails_router
 from papermerge.core.features.custom_fields.schema import CustomFieldType
 from papermerge.core.features.document_types import router as document_types_router
@@ -271,6 +274,7 @@ def get_app_with_routes():
     app.include_router(folders_router.router, prefix="")
     app.include_router(docs_router.router, prefix="")
     app.include_router(pages_router.router, prefix="")
+    app.include_router(document_versions_router.router, prefix="")
     app.include_router(thumbnails_router.router, prefix="")
     app.include_router(usr_router.router, prefix="")
     app.include_router(tags_router.router, prefix="")

papermerge/core/features/document/db/api.py

@@ -13,6 +13,7 @@
 
 from sqlalchemy import delete, func, insert, select, text, update, Select
 from sqlalchemy.exc import IntegrityError
+from sqlalchemy.orm import joinedload
 
 from papermerge.core.db.engine import Session
 from papermerge.core.constants import ContentType
@@ -817,9 +818,10 @@ def get_first_page(
 
 def get_doc_ver(
     db_session: Session,
-    id: uuid.UUID,
-    user_id: uuid.UUID,  # noqa
-) -> schema.DocumentVersion:
+    *,
+    document_version_id: uuid.UUID,
+    user_id: uuid.UUID,
+) -> orm.DocumentVersion:
     """
     Returns last version of the document
     identified by doc_id
@@ -828,12 +830,15 @@ def get_doc_ver(
     stmt = (
         select(orm.DocumentVersion)
         .join(orm.Document)
-        .where(orm.Document.user_id == user_id, orm.DocumentVersion.id == id)
+        .options(joinedload(orm.DocumentVersion.pages))
+        .where(
+            orm.Document.user_id == user_id,
+            orm.DocumentVersion.id == document_version_id,
+        )
     )
-    db_doc_ver = db_session.scalars(stmt).one()
-    model_doc_ver = schema.DocumentVersion.model_validate(db_doc_ver)
+    db_doc_ver = db_session.scalars(stmt).unique().one()
 
-    return model_doc_ver
+    return db_doc_ver
 
 
 def select_last_doc_ver(document_id: uuid.UUID, user_id: uuid.UUID) -> Select:

papermerge/core/routers/document_version.py → papermerge/core/features/document/router_document_version.py

@@ -2,15 +2,16 @@
 import uuid
 from typing import Annotated
 
+from sqlalchemy.exc import NoResultFound
+
 from fastapi import APIRouter, HTTPException, Security, Depends
 from fastapi.responses import FileResponse
 
 from papermerge.core.constants import ContentType
-from papermerge.core import schemas, utils, db
+from papermerge.core import schema, utils, db, dbapi, orm
 from papermerge.core.db import exceptions as db_exc
 from papermerge.core.features.auth import get_current_user
 from papermerge.core.features.auth import scopes
-from papermerge.core.models import DocumentVersion
 
 logger = logging.getLogger(__name__)
 
@@ -27,22 +28,27 @@ class PDFFileResponse(FileResponse):
 def download_document_version(
     document_version_id: uuid.UUID,
     user: Annotated[
-        schemas.User, Security(get_current_user, scopes=[scopes.DOCUMENT_DOWNLOAD])
+        schema.User, Security(get_current_user, scopes=[scopes.DOCUMENT_DOWNLOAD])
     ],
 ):
     """Downloads given document version
 
     Required scope: `{scope}`
     """
     try:
-        doc_ver = DocumentVersion.objects.get(
-            id=document_version_id, document__user=user.id
-        )
-    except DocumentVersion.DoesNotExist:
-        raise HTTPException(status_code=404, detail="Document version not found")
+        with db.Session() as db_session:
+            doc_ver: orm.DocumentVersion = dbapi.get_doc_ver(
+                db_session,
+                document_version_id=document_version_id,
+                user_id=user.id,
+            )
+    except NoResultFound:
+        error = schema.Error(messages=["Document version not found"])
+        raise HTTPException(status_code=404, detail=error.model_dump())
 
     if not doc_ver.file_path.exists():
-        raise HTTPException(status_code=404, detail="Document version file not found")
+        error = schema.Error(messages=["Document version file not found"])
+        raise HTTPException(status_code=404, detail=error.model_dump())
 
     return PDFFileResponse(
         doc_ver.file_path,
@@ -51,22 +57,23 @@ def download_document_version(
     )
 
 
-@router.get("/{document_version_id}", response_model=schemas.DocumentVersion)
+@router.get("/{document_version_id}", response_model=schema.DocumentVersion)
 @utils.docstring_parameter(scope=scopes.NODE_VIEW)
 def document_version_details(
     document_version_id: uuid.UUID,
-    user: Annotated[
-        schemas.User, Security(get_current_user, scopes=[scopes.NODE_VIEW])
-    ],
-    engine: db.Engine = Depends(db.get_engine),
+    user: Annotated[schema.User, Security(get_current_user, scopes=[scopes.NODE_VIEW])],
 ):
     """Get document version details
 
     Required scope: `{scope}`
     """
     try:
-        doc_ver = db.get_doc_ver(engine, id=document_version_id, user_id=user.id)
-    except db_exc.PageNotFound:
-        raise HTTPException(status_code=404, detail="Page not found")
+        with db.Session() as db_session:
+            doc_ver: orm.DocumentVersion = dbapi.get_doc_ver(
+                db_session, document_version_id=document_version_id, user_id=user.id
+            )
+    except NoResultFound:
+        error = schema.Error(messages=["Page not found"])
+        raise HTTPException(status_code=404, detail=error.model_dump())
 
-    return doc_ver
+    return schema.DocumentVersion.model_validate(doc_ver)

papermerge/core/features/document/router_pages.py

@@ -77,7 +77,9 @@ def get_page_jpg_url(
         with db.Session() as db_session:
             page = doc_dbapi.get_page(db_session, page_id=page_id, user_id=user.id)
             doc_ver = doc_dbapi.get_doc_ver(
-                db_session, id=page.document_version_id, user_id=user.id
+                db_session,
+                document_version_id=page.document_version_id,
+                user_id=user.id,
             )
     except NoResultFound:
         raise HTTPException(status_code=404, detail="Page does not exist")

papermerge/core/features/document/tests/test_document_version_router.py

@@ -0,0 +1,37 @@
+import os
+import uuid
+from pathlib import Path
+
+from papermerge.core import dbapi
+from papermerge.core.tests.resource_file import ResourceFile
+
+
+DIR_ABS_PATH = os.path.abspath(os.path.dirname(__file__))
+RESOURCES = Path(DIR_ABS_PATH) / "resources"
+
+
+def test_download_document_version(
+    auth_api_client, make_document_from_resource, user, db_session
+):
+    doc = make_document_from_resource(
+        resource=ResourceFile.THREE_PAGES, user=user, parent=user.home_folder
+    )
+
+    last_ver = dbapi.get_last_doc_ver(db_session, doc_id=doc.id, user_id=user.id)
+
+    response = auth_api_client.get(f"/document-versions/{last_ver.id}/download")
+    assert response.status_code == 200
+
+
+def test_document_version_download_request_non_existing_resource(auth_api_client):
+    non_existing_resource_id = uuid.uuid4().hex
+    response = auth_api_client.get(
+        f"/document-versions/{non_existing_resource_id}/download"
+    )
+    assert response.status_code == 404
+
+
+def test_document_version_details_request_non_existing_resource(auth_api_client):
+    non_existing_resource_id = uuid.uuid4().hex
+    response = auth_api_client.get(f"/document-versions/{non_existing_resource_id}")
+    assert response.status_code == 404

papermerge/core/features/page_mngm/test_page_mngm.py

@@ -26,7 +26,9 @@ def test_copy_text_field(db_session, make_document_version, user):
         db_session, src=doc_ver_x, dst=doc_ver_y, page_numbers=[2]
     )
 
-    doc_ver = doc_dbapi.get_doc_ver(db_session, id=doc_ver_y.id, user_id=user.id)
+    doc_ver = doc_dbapi.get_doc_ver(
+        db_session, document_version_id=doc_ver_y.id, user_id=user.id
+    )
 
     assert doc_ver.pages[0].text == "body"