From de19fa64f05056571ff8130b7cc4fadeee283e23 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Tue, 30 Jul 2024 10:34:45 -0700 Subject: [PATCH 1/6] Add route to fetch artifacts in bulk and return only images --- store/app/routers/image.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/store/app/routers/image.py b/store/app/routers/image.py index 86875b00..9f53254d 100644 --- a/store/app/routers/image.py +++ b/store/app/routers/image.py @@ -3,14 +3,14 @@ import logging from typing import Annotated -from fastapi import APIRouter, Depends, HTTPException, UploadFile, status +from fastapi import APIRouter, Depends, HTTPException, Query, UploadFile, status from fastapi.responses import RedirectResponse from PIL import Image from pydantic.main import BaseModel from store.app.crud.artifacts import get_image_name from store.app.db import Crud -from store.app.model import ArtifactSize, User +from store.app.model import Artifact, ArtifactSize, User from store.app.routers.users import get_session_user_with_write_permission from store.settings import settings @@ -64,3 +64,21 @@ async def image_url(image_id: str, size: ArtifactSize) -> RedirectResponse: # TODO: Use CloudFront API to return a signed CloudFront URL. image_url = f"{settings.site.artifact_base_url}/{get_image_name(image_id, size)}" return RedirectResponse(url=image_url) + + +class ImageInfoResponse(BaseModel): + id: str + caption: str | None + + +@image_router.get("/batch") +async def batch( + crud: Annotated[Crud, Depends(Crud.get)], + ids: list[str] = Query(description="List of part ids"), +) -> list[ImageInfoResponse]: + artifacts = await crud._get_item_batch(ids, Artifact) + return [ + ImageInfoResponse(id=artifact.id, caption=artifact.description) + for artifact in artifacts + if artifact.artifact_type == "image" + ] From 5318f03ecc44dcf97e839e547e6cd4acc075e3b7 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Tue, 30 Jul 2024 10:35:03 -0700 Subject: [PATCH 2/6] In Listings, fetch images from artifact_ids This fixes captions being just "caption" (which was a placeholder waiting for this commit) and also fixes trying to render URDF tar gunzips. --- frontend/src/components/files/ViewImage.tsx | 2 +- frontend/src/hooks/api.tsx | 9 ++ frontend/src/pages/ListingDetails.tsx | 142 ++++++++++---------- 3 files changed, 84 insertions(+), 69 deletions(-) diff --git a/frontend/src/components/files/ViewImage.tsx b/frontend/src/components/files/ViewImage.tsx index 590cf033..09c24448 100644 --- a/frontend/src/components/files/ViewImage.tsx +++ b/frontend/src/components/files/ViewImage.tsx @@ -4,7 +4,7 @@ import React from "react"; interface ImageProps { imageId: string; size: "small" | "large"; - caption: string; + caption?: string; } const ImageComponent: React.FC = ({ imageId, size, caption }) => { diff --git a/frontend/src/hooks/api.tsx b/frontend/src/hooks/api.tsx index 65bf7e95..57f6b5b3 100644 --- a/frontend/src/hooks/api.tsx +++ b/frontend/src/hooks/api.tsx @@ -191,6 +191,15 @@ export class api { }); } + public async getImages(ids: string[]): Promise { + return this.callWrapper(async () => { + const response = await this.api.get("/images/batch", { + params: { ids: ids.join(",") }, + }); + return response.data; + }); + } + public async uploadImage(formData: FormData): Promise { return this.callWrapper(async () => { const res = await this.api.post( diff --git a/frontend/src/pages/ListingDetails.tsx b/frontend/src/pages/ListingDetails.tsx index f3cc8522..95156842 100644 --- a/frontend/src/pages/ListingDetails.tsx +++ b/frontend/src/pages/ListingDetails.tsx @@ -2,7 +2,7 @@ import TCButton from "components/files/TCButton"; import ImageComponent from "components/files/ViewImage"; import { humanReadableError } from "constants/backend"; import { useAlertQueue } from "hooks/alerts"; -import { api } from "hooks/api"; +import { api, Artifact } from "hooks/api"; import { useAuthentication } from "hooks/auth"; import { useEffect, useState } from "react"; import { @@ -47,9 +47,10 @@ const RenderListing = ({ const [userId, setUserId] = useState(null); const [show, setShow] = useState(false); const [ownerEmail, setOwnerEmail] = useState(null); - const [imageIndex, setArtifactIndex] = useState(0); + const [imageIndex, setArtifactIndex] = useState(0); const [showDelete, setShowDelete] = useState(false); const [children, setChildren] = useState(null); + const [images, setImages] = useState([]); const handleClose = () => setShow(false); const handleShow = () => setShow(true); @@ -77,6 +78,8 @@ const RenderListing = ({ try { const ownerEmail = await auth_api.getUserById(user_id); setOwnerEmail(ownerEmail); + const images = await auth_api.getImages(artifact_ids); + setImages(images); } catch (err) { addAlert(humanReadableError(err), "error"); } @@ -229,9 +232,9 @@ const RenderListing = ({ data-bs-theme="dark" style={{ border: "1px solid #ccc" }} interval={null} - controls={artifact_ids.length > 1} + controls={images.length > 1} > - {artifact_ids.map((id, key) => ( + {images.map((image, key) => (
- - {"caption"} - + {image.caption && ( + + {image.caption} + + )} ))} - - - - {/* TO-DO: This supposed to be the caption */} - {artifact_ids[imageIndex]} ({imageIndex + 1} of{" "} - {artifact_ids.length}) - - - -
- -
- - - {artifact_ids.length > 1 && ( - - { - setArtifactIndex( - (imageIndex - 1 + artifact_ids.length) % - artifact_ids.length, - ); - }} - > - Previous - - { - setArtifactIndex((imageIndex + 1) % artifact_ids.length); - }} - > - Next - - - )} - - + {images.length > 0 && ( + + + + {images[imageIndex].caption} ({imageIndex + 1} of {images.length}) + + + +
+ +
+ + + {images.length > 1 && ( + + { + setArtifactIndex( + (imageIndex - 1 + images.length) % images.length, + ); + }} + > + Previous + + { + setArtifactIndex((imageIndex + 1) % images.length); + }} + > + Next + + + )} + + + )} ); }; From e09c7a64f864a98a065553a19f178344d6b1adba Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Tue, 30 Jul 2024 10:51:20 -0700 Subject: [PATCH 3/6] Add 'name' field to Artifacts, which stores filename This is so people have a good identifier for URDFs, etc. --- store/app/crud/artifacts.py | 6 ++++++ store/app/model.py | 3 +++ store/app/routers/image.py | 1 + store/app/routers/urdf.py | 1 + 4 files changed, 11 insertions(+) diff --git a/store/app/crud/artifacts.py b/store/app/crud/artifacts.py index 4c30eabf..a0cff0ad 100644 --- a/store/app/crud/artifacts.py +++ b/store/app/crud/artifacts.py @@ -65,12 +65,14 @@ async def _upload_cropped_image(self, image: Image.Image, image_id: str, size: A async def upload_image( self, + name: str, image: Image.Image, user_id: str, description: str | None = None, ) -> Artifact: artifact = Artifact.create( user_id=user_id, + name=name, artifact_type="image", sizes=list(SizeMapping.keys()), description=description, @@ -90,12 +92,14 @@ async def upload_image( async def upload_urdf( self, + name: str, file: io.BytesIO | BinaryIO, user_id: str, description: str | None = None, ) -> Artifact: artifact = Artifact.create( user_id=user_id, + name=name, artifact_type="urdf", description=description, ) @@ -107,12 +111,14 @@ async def upload_urdf( async def upload_mjcf( self, + name: str, file: io.BytesIO | BinaryIO, user_id: str, description: str | None = None, ) -> Artifact: artifact = Artifact.create( user_id=user_id, + name=name, artifact_type="mjcf", description=description, ) diff --git a/store/app/model.py b/store/app/model.py index 7afb58e8..828132ef 100644 --- a/store/app/model.py +++ b/store/app/model.py @@ -99,6 +99,7 @@ class Artifact(RobolistBaseModel): """ user_id: str + name: str artifact_type: ArtifactType sizes: list[ArtifactSize] | None = None description: str | None = None @@ -107,12 +108,14 @@ class Artifact(RobolistBaseModel): def create( cls, user_id: str, + name: str, artifact_type: ArtifactType, sizes: list[ArtifactSize] | None = None, description: str | None = None, ) -> Self: return cls( id=str(new_uuid()), + name=name, user_id=user_id, artifact_type=artifact_type, sizes=sizes, diff --git a/store/app/routers/image.py b/store/app/routers/image.py index 9f53254d..c8ed7a13 100644 --- a/store/app/routers/image.py +++ b/store/app/routers/image.py @@ -50,6 +50,7 @@ async def upload_image( image = Image.open(file.file) artifact = await crud.upload_image( image=image, + name=file.filename, user_id=user.id, description=description, ) diff --git a/store/app/routers/urdf.py b/store/app/routers/urdf.py index f4341519..825c2da5 100644 --- a/store/app/routers/urdf.py +++ b/store/app/routers/urdf.py @@ -35,6 +35,7 @@ async def upload_urdf( ) artifact = await crud.upload_urdf( file=file.file, + name=file.filename, user_id=user.id, ) return UserInfoResponse(urdf_id=artifact.id) From f27f80b40d53ce3e037e1b4f30c91d69f25cedaa Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Tue, 30 Jul 2024 10:58:54 -0700 Subject: [PATCH 4/6] Take in a listing for upload_urdf - Check whether user actually owns that listing - Update listing artifact_ids --- store/app/routers/urdf.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/store/app/routers/urdf.py b/store/app/routers/urdf.py index 825c2da5..42669ee8 100644 --- a/store/app/routers/urdf.py +++ b/store/app/routers/urdf.py @@ -9,7 +9,7 @@ from store.app.crud.artifacts import get_urdf_name from store.app.db import Crud -from store.app.model import User +from store.app.model import Listing, User from store.app.routers.users import get_session_user_with_write_permission from store.settings import settings @@ -26,8 +26,20 @@ class UserInfoResponse(BaseModel): async def upload_urdf( user: Annotated[User, Depends(get_session_user_with_write_permission)], crud: Annotated[Crud, Depends(Crud.get)], + listing_id: str, file: UploadFile, ) -> UserInfoResponse: + listing = await crud.get_listing(listing_id) + if listing is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="Could not find listing associated with the given id", + ) + if listing.user_id is not user.id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="You do not own this listing.", + ) if file.content_type != "application/gzip": raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, @@ -38,6 +50,7 @@ async def upload_urdf( name=file.filename, user_id=user.id, ) + await crud._update_item(listing_id, Listing, {"artifact_ids": listing.artifact_ids.append(artifact.id)}) return UserInfoResponse(urdf_id=artifact.id) From 60b97e5aebd57a763af4035f77d144b9faa88599 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Tue, 30 Jul 2024 11:18:28 -0700 Subject: [PATCH 5/6] rename batch to get_batch --- store/app/routers/listings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/store/app/routers/listings.py b/store/app/routers/listings.py index da1fbf00..456ea304 100644 --- a/store/app/routers/listings.py +++ b/store/app/routers/listings.py @@ -36,7 +36,7 @@ async def list_listings( @listings_router.get("/batch") -async def batch( +async def get_batch( crud: Annotated[Crud, Depends(Crud.get)], ids: list[str] = Query(description="List of part ids"), ) -> list[Listing]: From 6941104883dcc996a04766c5284042bce3f6c06f Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Tue, 30 Jul 2024 11:32:11 -0700 Subject: [PATCH 6/6] Change route that gets unique urdf to route that gets latest --- store/app/crud/listings.py | 11 +++-------- store/app/model.py | 3 +++ store/app/routers/urdf.py | 4 ++-- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/store/app/crud/listings.py b/store/app/crud/listings.py index ad8e8f19..7ed477aa 100644 --- a/store/app/crud/listings.py +++ b/store/app/crud/listings.py @@ -3,7 +3,7 @@ import asyncio import logging -from store.app.crud.base import BaseCrud, InternalError, ItemNotFoundError +from store.app.crud.base import BaseCrud, ItemNotFoundError from store.app.model import Artifact, Listing, ListingTag logger = logging.getLogger(__name__) @@ -54,7 +54,7 @@ async def delete_tag(self, listing_id: str, tag: str) -> None: listing_tag = ListingTag.create(listing_id=listing_id, tag=tag) return await self._delete_item(listing_tag) - async def get_urdf_id( + async def get_latest_urdf_id( self, listing_id: str, ) -> str | None: @@ -63,9 +63,4 @@ async def get_urdf_id( urdfs = [artifact for artifact in artifacts if artifact.artifact_type == "urdf"] if len(urdfs) == 0: return None - if len(urdfs) > 1: - raise InternalError( - f"""More than one URDF found for listing {listing_id}. - This is due to incorrect data in the database, likely caused by a bug.""" - ) - return urdfs[0].id + return max(urdfs, key=lambda urdf: urdf.timestamp) diff --git a/store/app/model.py b/store/app/model.py index 828132ef..8ac6a070 100644 --- a/store/app/model.py +++ b/store/app/model.py @@ -5,6 +5,7 @@ expects (for example, converting a UUID into a string). """ +import time from datetime import datetime, timedelta from typing import Literal, Self @@ -103,6 +104,7 @@ class Artifact(RobolistBaseModel): artifact_type: ArtifactType sizes: list[ArtifactSize] | None = None description: str | None = None + timestamp: int @classmethod def create( @@ -120,6 +122,7 @@ def create( artifact_type=artifact_type, sizes=sizes, description=description, + timestamp=int(time.time()), ) diff --git a/store/app/routers/urdf.py b/store/app/routers/urdf.py index 42669ee8..73869804 100644 --- a/store/app/routers/urdf.py +++ b/store/app/routers/urdf.py @@ -54,12 +54,12 @@ async def upload_urdf( return UserInfoResponse(urdf_id=artifact.id) -@urdf_router.get("/{listing_id}") +@urdf_router.get("/latest/{listing_id}") async def listing_urdf( listing_id: str, crud: Annotated[Crud, Depends(Crud.get)], ) -> RedirectResponse: - urdf_id = await crud.get_urdf_id(listing_id) + urdf_id = await crud.get_latest_urdf_id(listing_id) if urdf_id is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) urdf_url = f"{settings.site.artifact_base_url}/{get_urdf_name(urdf_id)}"