diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 6c57759..a5bfe87 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -29,12 +29,11 @@ jobs: S3_SECRET_KEY: ${{ secrets.S3_SECRET_KEY }} S3_REGION: ${{ secrets.S3_REGION }} S3_ENDPOINT_URL: ${{ secrets.S3_ENDPOINT_URL }} - run: docker-compose up -d --build - - name: Docker sanity check - run: sleep 20 && nc -vz localhost 8080 - - name: Ping server - run: curl http://localhost:8080/docs - + run: | + docker compose up -d --build --wait + docker compose logs backend + nc -vz localhost 8080 + curl http://localhost:8080/docs client: runs-on: ${{ matrix.os }} strategy: diff --git a/client/docs/source/conf.py b/client/docs/source/conf.py index 12b3ad3..c94bd2c 100644 --- a/client/docs/source/conf.py +++ b/client/docs/source/conf.py @@ -104,6 +104,7 @@ # so a file named "default.css" will overwrite the builtin "default.css". html_static_path = ["_static"] + # Add googleanalytics id # ref: https://github.com/orenhecht/googleanalytics/blob/master/sphinxcontrib/googleanalytics.py def add_ga_javascript(app, pagename, templatename, context, doctree): diff --git a/client/pyrostorage/client.py b/client/pyrostorage/client.py index 10d784e..5f6a1b1 100644 --- a/client/pyrostorage/client.py +++ b/client/pyrostorage/client.py @@ -5,7 +5,7 @@ import io import logging -from typing import Dict +from typing import Dict, List from urllib.parse import urljoin import requests @@ -29,8 +29,7 @@ # ANNOTATIONS ################# "create-annotation": "/annotations", - "upload-annotation": "/annotations/{annotation_id}/upload", - "get-annotation-url": "/annotations/{annotation_id}/url", + "update-annotation": "/annotations/{annotation_id}", } @@ -129,7 +128,7 @@ def get_media_url(self, media_id: int) -> Response: return requests.get(self.routes["get-media-url"].format(media_id=media_id), headers=self.headers) - def create_annotation(self, media_id: int) -> Response: + def create_annotation(self, media_id: int, observations: List[str]) -> Response: """Create an annotation entry Example:: @@ -139,49 +138,35 @@ def create_annotation(self, media_id: int) -> Response: Args: media_id: the identifier of the media entry + observations: list of observations Returns: HTTP response containing the created annotation """ - return requests.post(self.routes["create-annotation"], headers=self.headers, json={"media_id": media_id}) - - def upload_annotation(self, annotation_id: int, annotation_data: bytes) -> Response: - """Upload the annotation content - - Example:: - >>> from pyrostorage import client - >>> api_client = client.Client("http://pyro-storage.herokuapp.com", "MY_LOGIN", "MY_PWD") - >>> with open("path/to/my/file.ext", "rb") as f: data = f.read() - >>> response = api_client.upload_annotation(annotation_id=1, annotation_data=data) - - Args: - annotation_id: ID of the associated annotation entry - annotation_data: byte data - - Returns: - HTTP response containing the updated annotation - """ - return requests.post( - self.routes["upload-annotation"].format(annotation_id=annotation_id), + self.routes["create-annotation"], headers=self.headers, - files={"file": io.BytesIO(annotation_data)}, + json={"media_id": media_id, "observations": observations}, ) - def get_annotation_url(self, annotation_id: int) -> Response: - """Get the image as a URL + def update_annotation(self, annotation_id: int, observations: List[str]) -> Response: + """Update an annotation entry Example:: >>> from pyrostorage import client >>> api_client = client.Client("http://pyro-storage.herokuapp.com", "MY_LOGIN", "MY_PWD") - >>> response = api_client.get_annotation_url(1) + >>> response = api_client.update_annotation(media_id=1) Args: annotation_id: the identifier of the annotation entry + observations: list of observations Returns: - HTTP response containing the URL to the annotation content + HTTP response containing the updated annotation """ - - return requests.get(self.routes["get-annotation-url"].format(annotation_id=annotation_id), headers=self.headers) + return requests.post( + f'self.routes["update-annotation"]/{annotation_id}', + headers=self.headers, + json={"observations": observations}, + ) diff --git a/client/tests/test_client.py b/client/tests/test_client.py index 99e0bf4..26e2f5b 100644 --- a/client/tests/test_client.py +++ b/client/tests/test_client.py @@ -30,7 +30,7 @@ def test_client(): # Media media_id = _test_route_return(api_client.create_media(media_type="image"), dict, 201)["id"] # Annotation - _test_route_return(api_client.create_annotation(media_id=media_id), dict, 201)["id"] + _test_route_return(api_client.create_annotation(media_id=media_id, observations=["smoke", "fire"]), dict, 201)["id"] # Check token refresh prev_headers = deepcopy(api_client.headers) diff --git a/docker-compose-dev.yml b/docker-compose-dev.yml index fabea73..61f3347 100644 --- a/docker-compose-dev.yml +++ b/docker-compose-dev.yml @@ -23,6 +23,11 @@ services: - S3_ENDPOINT_URL=${S3_ENDPOINT_URL} depends_on: - db + healthcheck: + test: ['CMD-SHELL', 'nc -vz localhost 8080'] + interval: 10s + timeout: 3s + retries: 3 db: image: postgres:15-alpine volumes: @@ -33,6 +38,11 @@ services: - POSTGRES_USER=dummy_pg_user - POSTGRES_PASSWORD=dummy_pg_pwd - POSTGRES_DB=dummy_pg_db + healthcheck: + test: ['CMD-SHELL', "sh -c 'pg_isready -U dummy_pg_user -d dummy_pg_db'"] + interval: 10s + timeout: 3s + retries: 3 proxy: build: nginx ports: diff --git a/docker-compose.yml b/docker-compose.yml index d1f192e..8fc17d9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,8 +18,14 @@ services: - S3_SECRET_KEY=${S3_SECRET_KEY} - S3_REGION=${S3_REGION} - S3_ENDPOINT_URL=${S3_ENDPOINT_URL} + healthcheck: + test: ['CMD-SHELL', 'nc -vz localhost 8080'] + interval: 10s + timeout: 3s + retries: 3 depends_on: - - db + db: + condition: service_healthy db: image: postgres:15-alpine volumes: @@ -30,13 +36,11 @@ services: - POSTGRES_USER=dummy_pg_user - POSTGRES_PASSWORD=dummy_pg_pwd - POSTGRES_DB=dummy_pg_db - nginx: - build: nginx - ports: - - 80:80 - - 443:443 - depends_on: - - backend + healthcheck: + test: ['CMD-SHELL', "sh -c 'pg_isready -U dummy_pg_user -d dummy_pg_db'"] + interval: 10s + timeout: 3s + retries: 3 volumes: postgres_data: diff --git a/src/Dockerfile-dev b/src/Dockerfile-dev index fa1c961..51e97b1 100644 --- a/src/Dockerfile-dev +++ b/src/Dockerfile-dev @@ -1,4 +1,4 @@ -FROM pyrostorage:python3.8-alpine3.10 +FROM pyronear/storage-api:python3.8-alpine3.10 # copy requirements file COPY requirements-dev.txt requirements-dev.txt diff --git a/src/app/api/routes/annotations.py b/src/app/api/routes/annotations.py index 53bec20..a60dcba 100644 --- a/src/app/api/routes/annotations.py +++ b/src/app/api/routes/annotations.py @@ -5,15 +5,13 @@ from typing import Any, Dict, List -from fastapi import APIRouter, BackgroundTasks, File, HTTPException, Path, Security, UploadFile, status +from fastapi import APIRouter, Path, Security, status from app.api import crud from app.api.crud.authorizations import check_access_read, is_admin_access from app.api.deps import get_current_access -from app.api.schemas import AccessType, AnnotationCreation, AnnotationIn, AnnotationOut, AnnotationUrl -from app.api.security import hash_content_file +from app.api.schemas import AccessType, AnnotationIn, AnnotationOut, AnnotationUpdateIn from app.db import annotations -from app.services import resolve_bucket_key, s3_bucket router = APIRouter() @@ -33,7 +31,7 @@ async def create_annotation( payload: AnnotationIn, _=Security(get_current_access, scopes=[AccessType.admin, AccessType.user]) ): """ - Creates an annotation related to specific media, based on media_id as argument + Creates an annotation related to specific media, based on media_id as argument, and with as many observations as needed Below, click on "Schema" for more detailed information about arguments or "Example Value" to get a concrete idea of arguments @@ -66,9 +64,9 @@ async def fetch_annotations( return [] -@router.put("/{annotation_id}/", response_model=AnnotationOut, summary="Update information about a specific annotation") +@router.put("/{annotation_id}/", response_model=AnnotationOut, summary="Update observations on a specific annotation") async def update_annotation( - payload: AnnotationIn, + payload: AnnotationUpdateIn, annotation_id: int = Path(..., gt=0), _=Security(get_current_access, scopes=[AccessType.admin]), ): @@ -86,69 +84,3 @@ async def delete_annotation( Based on a annotation_id, deletes the specified annotation """ return await crud.delete_entry(annotations, annotation_id) - - -@router.post("/{annotation_id}/upload", response_model=AnnotationOut, status_code=200) -async def upload_annotation( - background_tasks: BackgroundTasks, - annotation_id: int = Path(..., gt=0), - file: UploadFile = File(...), -): - """ - Upload a annotation (image or video) linked to an existing annotation object in the DB - """ - - # Check in DB - entry = await check_annotation_registration(annotation_id) - - # Concatenate the first 32 chars (to avoid system interactions issues) of SHA256 hash with file extension - file_hash = hash_content_file(file.file.read()) - file_name = f"{file_hash[:32]}.{file.filename.rpartition('.')[-1]}" - # Reset byte position of the file (cf. https://fastapi.tiangolo.com/tutorial/request-files/#uploadfile) - await file.seek(0) - # Use MD5 to verify upload - md5_hash = hash_content_file(file.file.read(), use_md5=True) - await file.seek(0) - # If files are in a subfolder of the bucket, prepend the folder path - bucket_key = resolve_bucket_key(file_name, "annotations") - - # Upload if bucket_key is different (otherwise the content is the exact same) - if isinstance(entry["bucket_key"], str) and entry["bucket_key"] == bucket_key: - return await crud.get_entry(annotations, annotation_id) - else: - # Failed upload - if not await s3_bucket.upload_file(bucket_key=bucket_key, file_binary=file.file): - raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Failed upload") - # Data integrity check - file_meta = await s3_bucket.get_file_metadata(bucket_key) - # Corrupted file - if md5_hash != file_meta["ETag"].replace('"', ""): - # Delete the corrupted upload - await s3_bucket.delete_file(bucket_key) - # Raise the exception - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Data was corrupted during upload", - ) - # If a file was previously uploaded, delete it - if isinstance(entry["bucket_key"], str): - await s3_bucket.delete_file(entry["bucket_key"]) - - entry_dict = dict(**entry) - entry_dict["bucket_key"] = bucket_key - return await crud.update_entry(annotations, AnnotationCreation(**entry_dict), annotation_id) - - -@router.get("/{annotation_id}/url", response_model=AnnotationUrl, status_code=200) -async def get_annotation_url( - annotation_id: int = Path(..., gt=0), - requester=Security(get_current_access, scopes=[AccessType.admin, AccessType.user]), -): - """Resolve the temporary media image URL""" - await check_access_read(requester.id) - - # Check in DB - annotation_instance = await check_annotation_registration(annotation_id) - # Check in bucket - temp_public_url = await s3_bucket.get_public_url(annotation_instance["bucket_key"]) - return AnnotationUrl(url=temp_public_url) diff --git a/src/app/api/schemas.py b/src/app/api/schemas.py index cffbde6..a1b1edd 100644 --- a/src/app/api/schemas.py +++ b/src/app/api/schemas.py @@ -8,7 +8,7 @@ from pydantic import BaseModel, Field, validator -from app.db.models import AccessType, MediaType +from app.db.models import AccessType, MediaType, ObservationType # Template classes @@ -88,15 +88,12 @@ class MediaUrl(BaseModel): # Annotation class AnnotationIn(BaseModel): media_id: int = Field(..., gt=0) + observations: List[ObservationType] -class AnnotationCreation(AnnotationIn): - bucket_key: str = Field(...) +class AnnotationUpdateIn(BaseModel): + observations: List[ObservationType] class AnnotationOut(AnnotationIn, _CreatedAt, _Id): pass - - -class AnnotationUrl(BaseModel): - url: str diff --git a/src/app/db/models.py b/src/app/db/models.py index fe9d278..e6a97de 100644 --- a/src/app/db/models.py +++ b/src/app/db/models.py @@ -5,7 +5,7 @@ import enum -from sqlalchemy import Column, DateTime, Enum, ForeignKey, Integer, String +from sqlalchemy import ARRAY, Column, DateTime, Enum, ForeignKey, Integer, String from sqlalchemy.orm import relationship from sqlalchemy.sql import func @@ -46,15 +46,23 @@ def __repr__(self): return f"" +class ObservationType(str, enum.Enum): + clouds: str = "clouds" + fire: str = "fire" + fog: str = "fog" + sky: str = "sky" + smoke: str = "smoke" + + class Annotations(Base): __tablename__ = "annotations" id = Column(Integer, primary_key=True) media_id = Column(Integer, ForeignKey("media.id")) - bucket_key = Column(String(100), nullable=True) + observations = Column(ARRAY(Enum(ObservationType)), nullable=False) created_at = Column(DateTime, default=func.now()) media = relationship("Media", uselist=False, back_populates="annotations") def __repr__(self): - return f"" + return f"" diff --git a/src/tests/crud/test_authorizations.py b/src/tests/crud/test_authorizations.py index 9f0e2c4..a7b0bc9 100644 --- a/src/tests/crud/test_authorizations.py +++ b/src/tests/crud/test_authorizations.py @@ -19,7 +19,7 @@ ANNOTATIONS_TABLE = [ - {"id": 1, "media_id": 1, "created_at": "2020-10-13T08:18:45.447773"}, + {"id": 1, "media_id": 1, "observations": [], "created_at": "2020-10-13T08:18:45.447773"}, ] diff --git a/src/tests/db_utils.py b/src/tests/db_utils.py index 86d4e8e..ff32608 100644 --- a/src/tests/db_utils.py +++ b/src/tests/db_utils.py @@ -34,7 +34,7 @@ async def fill_table(test_db: Database, table: Table, entries: List[Dict[str, An are not incremented if the "id" field is included """ if remove_ids: - entries = [{k: v for k, v in x.items() if k != "id"} for x in entries] + entries = [{k: v for k, v in entry.items() if k != "id"} for entry in entries] query = table.insert().values(entries) await test_db.execute(query=query) diff --git a/src/tests/routes/test_annotations.py b/src/tests/routes/test_annotations.py index 71f00e1..365bfbb 100644 --- a/src/tests/routes/test_annotations.py +++ b/src/tests/routes/test_annotations.py @@ -1,6 +1,4 @@ import json -import os -import tempfile from datetime import datetime import pytest @@ -8,8 +6,7 @@ from app import db from app.api import crud -from app.api.security import hash_content_file -from app.services import s3_bucket +from app.db.models import ObservationType from tests.db_utils import TestSessionLocal, fill_table, get_entry from tests.utils import update_only_datetime @@ -24,7 +21,13 @@ ] ANNOTATIONS_TABLE = [ - {"id": 1, "media_id": 1, "bucket_key": "dummy_key", "created_at": "2020-10-13T08:18:45.447773"}, + { + "id": 1, + "media_id": 1, + "observations": [ObservationType.fire, ObservationType.smoke, ObservationType.clouds], + "created_at": "2020-10-13T08:18:45.447773", + }, + {"id": 2, "media_id": 2, "observations": [], "created_at": "2022-10-13T08:18:45.447773"}, ] @@ -65,7 +68,7 @@ async def test_get_annotation(test_app_asyncio, init_test_db, access_idx, annota assert response.json()["detail"] == status_details if response.status_code // 100 == 2: - assert response.json() == {k: v for k, v in ANNOTATIONS_TABLE[annotation_id - 1].items() if k != "bucket_key"} + assert response.json() == ANNOTATIONS_TABLE[annotation_id - 1] @pytest.mark.parametrize( @@ -73,7 +76,7 @@ async def test_get_annotation(test_app_asyncio, init_test_db, access_idx, annota [ [None, 401, "Not authenticated", None], [0, 200, None, []], - [1, 200, None, [{k: v for k, v in elt.items() if k != "bucket_key"} for elt in ANNOTATIONS_TABLE]], + [1, 200, None, ANNOTATIONS_TABLE], ], ) @pytest.mark.asyncio @@ -98,25 +101,31 @@ async def test_fetch_annotations( @pytest.mark.parametrize( "access_idx, payload, status_code, status_details", [ - [None, {"media_id": 1}, 401, "Not authenticated"], - [0, {"media_id": 1}, 201, None], - [1, {"media_id": 1}, 201, None], - [1, {"media_id": "alpha"}, 422, None], + [None, {"media_id": 1, "observations": []}, 401, "Not authenticated"], + [0, {"media_id": 1, "observations": []}, 201, None], + [1, {"media_id": 1, "observations": []}, 201, None], + [1, {"media_id": 1, "observations": ["clouds"]}, 201, None], + [1, {"media_id": 1, "observations": ["clouds", "fire", "smoke"]}, 201, None], + [1, {"media_id": 1, "observations": ["clouds", "fire", "puppy"]}, 422, None], + [1, {"media_id": 1, "observations": [1337]}, 422, None], + [1, {"media_id": 1, "observations": "smoke"}, 422, None], + [1, {"media_id": "alpha", "observations": []}, 422, None], [1, {}, 422, None], + [1, {"media_id": 1}, 422, None], + [1, {"observations": []}, 422, None], ], ) @pytest.mark.asyncio async def test_create_annotation( test_app_asyncio, init_test_db, test_db, access_idx, payload, status_code, status_details ): - # Create a custom access token auth = None if isinstance(access_idx, int): auth = await pytest.get_token(ACCESS_TABLE[access_idx]["id"], ACCESS_TABLE[access_idx]["scope"].split()) utc_dt = datetime.utcnow() - response = await test_app_asyncio.post("/annotations/", data=json.dumps(payload), headers=auth) + response = await test_app_asyncio.post("/annotations/", json=payload, headers=auth) assert response.status_code == status_code if isinstance(status_details, str): assert response.json()["detail"] == status_details @@ -136,13 +145,16 @@ async def test_create_annotation( @pytest.mark.parametrize( "access_idx, payload, annotation_id, status_code, status_details", [ - [None, {"media_id": 1}, 1, 401, "Not authenticated"], - [0, {"media_id": 1}, 1, 403, "Your access scope is not compatible with this operation."], - [1, {"media_id": 1}, 1, 200, None], + [None, {"observations": []}, 1, 401, "Not authenticated"], + [0, {"observations": []}, 1, 403, "Your access scope is not compatible with this operation."], + [1, {"observations": []}, 1, 200, None], + [1, {"observations": [1337]}, 1, 422, None], + [1, {"observations": ["smoke"]}, 1, 200, None], + [1, {"observations": ["smoke", "fire", "puppy"]}, 1, 422, None], [1, {}, 1, 422, None], - [1, {"media_id": "alpha"}, 1, 422, None], - [1, {"media_id": 1}, 999, 404, "Table annotations has no entry with id=999"], - [1, {"media_id": 1}, 0, 422, None], + [1, {"observations": "smoke"}, 1, 422, None], + [1, {"observations": []}, 999, 404, "Table annotations has no entry with id=999"], + [1, {"observations": []}, 0, 422, None], ], ) @pytest.mark.asyncio @@ -164,8 +176,7 @@ async def test_update_annotation( updated_annotation = await get_entry(test_db, db.annotations, annotation_id) updated_annotation = dict(**updated_annotation) for k, v in updated_annotation.items(): - if k != "bucket_key": - assert v == payload.get(k, ANNOTATIONS_TABLE_FOR_DB[annotation_id - 1][k]) + assert v == payload.get(k, ANNOTATIONS_TABLE_FOR_DB[annotation_id - 1][k]) @pytest.mark.parametrize( @@ -194,71 +205,6 @@ async def test_delete_annotation( assert response.json()["detail"] == status_details if response.status_code // 100 == 2: - assert response.json() == {k: v for k, v in ANNOTATIONS_TABLE[annotation_id - 1].items() if k != "bucket_key"} + assert response.json() == ANNOTATIONS_TABLE[annotation_id - 1] remaining_annotation = await test_app_asyncio.get("/annotations/", headers=auth) assert all(entry["id"] != annotation_id for entry in remaining_annotation.json()) - - -@pytest.mark.asyncio -async def test_upload_annotation(test_app_asyncio, init_test_db, test_db, monkeypatch): - - admin_idx = 1 - # Create a custom access token - admin_auth = await pytest.get_token(ACCESS_TABLE[admin_idx]["id"], ACCESS_TABLE[admin_idx]["scope"].split()) - - # 1 - Create a annotation that will have an upload - payload = {"media_id": 2} - new_annotation_id = len(ANNOTATIONS_TABLE_FOR_DB) + 1 - response = await test_app_asyncio.post("/annotations/", data=json.dumps(payload), headers=admin_auth) - assert response.status_code == 201 - - # 2 - Upload something - async def mock_upload_file(bucket_key, file_binary): - return True - - monkeypatch.setattr(s3_bucket, "upload_file", mock_upload_file) - - # Download and save a temporary file - local_tmp_path = os.path.join(tempfile.gettempdir(), "my_temp_annotation.json") - data = {"label": "fire"} - with open(local_tmp_path, "w") as f: - json.dump(data, f) - - with open(local_tmp_path, "rb") as f: - md5_hash = hash_content_file(f.read(), use_md5=True) - - async def mock_get_file_metadata(bucket_key): - return {"ETag": md5_hash} - - monkeypatch.setattr(s3_bucket, "get_file_metadata", mock_get_file_metadata) - - async def mock_delete_file(filename): - return True - - monkeypatch.setattr(s3_bucket, "delete_file", mock_delete_file) - - # Switch content-type from JSON to multipart - del admin_auth["Content-Type"] - - with open(local_tmp_path, "rb") as content: - response = await test_app_asyncio.post( - f"/annotations/{new_annotation_id}/upload", files=dict(file=content), headers=admin_auth - ) - - assert response.status_code == 200, print(response.json()["detail"]) - response_json = response.json() - updated_annotation = await get_entry(test_db, db.annotations, response_json["id"]) - updated_annotation = dict(**updated_annotation) - response_json.pop("created_at") - assert {k: v for k, v in updated_annotation.items() if k not in ("created_at", "bucket_key")} == response_json - assert updated_annotation["bucket_key"] is not None - - # 2b - Upload failing - async def failing_upload(bucket_key, file_binary): - return False - - monkeypatch.setattr(s3_bucket, "upload_file", failing_upload) - response = await test_app_asyncio.post( - f"/annotations/{new_annotation_id}/upload", files=dict(file="bar"), headers=admin_auth - ) - assert response.status_code == 500