From b2c702b534fca8fc70ac77bc9e174f5d81ad9fb2 Mon Sep 17 00:00:00 2001 From: Jesse Brault Date: Tue, 23 Jul 2024 16:23:44 -0500 Subject: [PATCH] Refactoring of S3ImageService and related. --- .../api/image/S3ImageServiceTests.java | 15 +++-- .../app/mealsmadeeasy/api/image/Image.java | 2 - .../api/image/ImageException.java | 2 +- .../api/image/ImageSecurityImpl.java | 6 +- .../mealsmadeeasy/api/image/ImageService.java | 6 +- .../{ImageEntity.java => S3ImageEntity.java} | 17 +----- ...Repository.java => S3ImageRepository.java} | 8 ++- .../api/image/S3ImageService.java | 55 ++++++++++--------- 8 files changed, 54 insertions(+), 57 deletions(-) rename src/main/java/app/mealsmadeeasy/api/image/{ImageEntity.java => S3ImageEntity.java} (88%) rename src/main/java/app/mealsmadeeasy/api/image/{ImageRepository.java => S3ImageRepository.java} (57%) diff --git a/src/integrationTest/java/app/mealsmadeeasy/api/image/S3ImageServiceTests.java b/src/integrationTest/java/app/mealsmadeeasy/api/image/S3ImageServiceTests.java index 1f8e21f..dadec3b 100644 --- a/src/integrationTest/java/app/mealsmadeeasy/api/image/S3ImageServiceTests.java +++ b/src/integrationTest/java/app/mealsmadeeasy/api/image/S3ImageServiceTests.java @@ -30,6 +30,8 @@ import static org.hamcrest.Matchers.notNullValue; @SpringBootTest public class S3ImageServiceTests { + private static final String USER_FILENAME = "HAL9000.svg"; + @Container private static final MinIOContainer container = new MinIOContainer( DockerImageName.parse("minio/minio:latest") @@ -65,7 +67,7 @@ public class S3ImageServiceTests { try (final InputStream hal9000 = getHal9000()) { return this.imageService.create( owner, - "HAL9000.svg", + USER_FILENAME, hal9000, "image/svg+xml", 27881L @@ -88,7 +90,6 @@ public class S3ImageServiceTests { assertThat(image.getMimeType(), is("image/svg+xml")); assertThat(image.getAlt(), is(nullValue())); assertThat(image.getCaption(), is(nullValue())); - assertThat(image.getInternalUrl(), is(notNullValue())); assertThat(image.isPublic(), is(false)); assertThat(image.getViewers(), is(empty())); } @@ -98,18 +99,21 @@ public class S3ImageServiceTests { public void loadImageWithOwner() throws ImageException, IOException { final User owner = this.createTestUser("imageOwner"); final Image image = this.createHal9000(owner); - try (final InputStream stored = this.imageService.getImageContentById(image.getId(), owner)) { + try (final InputStream stored = + this.imageService.getImageContentByOwnerAndFilename(owner, owner, image.getUserFilename())) { final byte[] storedBytes = stored.readAllBytes(); assertThat(storedBytes.length, is(27881)); } } @Test + @DirtiesContext public void loadPublicImage() throws ImageException, IOException { final User owner = this.createTestUser("imageOwner"); Image image = this.createHal9000(owner); image = this.imageService.setPublic(image, owner, true); - try (final InputStream stored = this.imageService.getImageContentById(image.getId())) { + try (final InputStream stored = + this.imageService.getImageContentByOwnerAndFilename(owner, image.getUserFilename())) { final byte[] storedBytes = stored.readAllBytes(); assertThat(storedBytes.length, is(27881)); } @@ -122,7 +126,8 @@ public class S3ImageServiceTests { final User viewer = this.createTestUser("imageViewer"); Image image = this.createHal9000(owner); image = this.imageService.addViewer(image, owner, viewer); - try (final InputStream stored = this.imageService.getImageContentById(image.getId(), viewer)) { + try (final InputStream stored = + this.imageService.getImageContentByOwnerAndFilename(viewer, owner, image.getUserFilename())) { final byte[] storedBytes = stored.readAllBytes(); assertThat(storedBytes.length, is(27881)); } diff --git a/src/main/java/app/mealsmadeeasy/api/image/Image.java b/src/main/java/app/mealsmadeeasy/api/image/Image.java index ccc7c64..c231c03 100644 --- a/src/main/java/app/mealsmadeeasy/api/image/Image.java +++ b/src/main/java/app/mealsmadeeasy/api/image/Image.java @@ -15,8 +15,6 @@ public interface Image { String getMimeType(); @Nullable String getAlt(); @Nullable String getCaption(); - String getObjectName(); - String getInternalUrl(); User getOwner(); boolean isPublic(); Set getViewers(); diff --git a/src/main/java/app/mealsmadeeasy/api/image/ImageException.java b/src/main/java/app/mealsmadeeasy/api/image/ImageException.java index 9a000d2..9e0844c 100644 --- a/src/main/java/app/mealsmadeeasy/api/image/ImageException.java +++ b/src/main/java/app/mealsmadeeasy/api/image/ImageException.java @@ -3,7 +3,7 @@ package app.mealsmadeeasy.api.image; public class ImageException extends Exception { public enum Type { - INVALID_ID, UNKNOWN_MIME_TYPE + INVALID_ID, IMAGE_NOT_FOUND, UNKNOWN_MIME_TYPE } private final Type type; diff --git a/src/main/java/app/mealsmadeeasy/api/image/ImageSecurityImpl.java b/src/main/java/app/mealsmadeeasy/api/image/ImageSecurityImpl.java index 3d08d7f..a9adfa2 100644 --- a/src/main/java/app/mealsmadeeasy/api/image/ImageSecurityImpl.java +++ b/src/main/java/app/mealsmadeeasy/api/image/ImageSecurityImpl.java @@ -9,9 +9,9 @@ import java.util.Objects; @Component("imageSecurity") public class ImageSecurityImpl implements ImageSecurity { - private final ImageRepository imageRepository; + private final S3ImageRepository imageRepository; - public ImageSecurityImpl(ImageRepository imageRepository) { + public ImageSecurityImpl(S3ImageRepository imageRepository) { this.imageRepository = imageRepository; } @@ -28,7 +28,7 @@ public class ImageSecurityImpl implements ImageSecurity { return true; } else { // check if viewer - final ImageEntity withViewers = this.imageRepository.getByIdWithViewers(image.getId()); + final S3ImageEntity withViewers = this.imageRepository.getByIdWithViewers(image.getId()); for (final User user : withViewers.getViewers()) { if (user.getId() != null && user.getId().equals(viewer.getId())) { return true; diff --git a/src/main/java/app/mealsmadeeasy/api/image/ImageService.java b/src/main/java/app/mealsmadeeasy/api/image/ImageService.java index 856e63b..adfe5c5 100644 --- a/src/main/java/app/mealsmadeeasy/api/image/ImageService.java +++ b/src/main/java/app/mealsmadeeasy/api/image/ImageService.java @@ -13,9 +13,10 @@ public interface ImageService { Image getById(long id) throws ImageException; Image getById(long id, User viewer) throws ImageException; + Image getByOwnerAndFilename(User viewer, User owner, String filename) throws ImageException; - InputStream getImageContentById(long id) throws IOException, ImageException; - InputStream getImageContentById(long id, User viewer) throws IOException, ImageException; + InputStream getImageContentByOwnerAndFilename(User owner, String filename) throws ImageException, IOException; + InputStream getImageContentByOwnerAndFilename(User viewer, User owner, String filename) throws ImageException, IOException; List getImagesOwnedBy(User user); @@ -30,6 +31,5 @@ public interface ImageService { Image clearViewers(Image image, User owner); void deleteImage(Image image, User owner) throws IOException; - void deleteById(long id, User owner) throws IOException; } diff --git a/src/main/java/app/mealsmadeeasy/api/image/ImageEntity.java b/src/main/java/app/mealsmadeeasy/api/image/S3ImageEntity.java similarity index 88% rename from src/main/java/app/mealsmadeeasy/api/image/ImageEntity.java rename to src/main/java/app/mealsmadeeasy/api/image/S3ImageEntity.java index a852279..e1fb283 100644 --- a/src/main/java/app/mealsmadeeasy/api/image/ImageEntity.java +++ b/src/main/java/app/mealsmadeeasy/api/image/S3ImageEntity.java @@ -10,7 +10,7 @@ import java.util.HashSet; import java.util.Set; @Entity(name = "Image") -public class ImageEntity implements Image { +public class S3ImageEntity implements Image { @Id @GeneratedValue(strategy = GenerationType.AUTO) @@ -35,9 +35,6 @@ public class ImageEntity implements Image { @Column(nullable = false) private String objectName; - @Column(nullable = false) - private String internalUrl; - @ManyToOne(optional = false) @JoinColumn(name = "owner_id", nullable = false) private UserEntity owner; @@ -111,7 +108,6 @@ public class ImageEntity implements Image { this.caption = caption; } - @Override public String getObjectName() { return this.objectName; } @@ -120,15 +116,6 @@ public class ImageEntity implements Image { this.objectName = objectName; } - @Override - public String getInternalUrl() { - return this.internalUrl; - } - - public void setInternalUrl(String internalUrl) { - this.internalUrl = internalUrl; - } - @Override public User getOwner() { return this.owner; @@ -158,7 +145,7 @@ public class ImageEntity implements Image { @Override public String toString() { - return "ImageEntity(" + this.id + ", " + this.internalUrl + ")"; + return "S3ImageEntity(" + this.id + ", " + this.userFilename + "," + this.objectName + ")"; } } diff --git a/src/main/java/app/mealsmadeeasy/api/image/ImageRepository.java b/src/main/java/app/mealsmadeeasy/api/image/S3ImageRepository.java similarity index 57% rename from src/main/java/app/mealsmadeeasy/api/image/ImageRepository.java rename to src/main/java/app/mealsmadeeasy/api/image/S3ImageRepository.java index 864694c..e79cd23 100644 --- a/src/main/java/app/mealsmadeeasy/api/image/ImageRepository.java +++ b/src/main/java/app/mealsmadeeasy/api/image/S3ImageRepository.java @@ -6,13 +6,15 @@ import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; import java.util.List; +import java.util.Optional; -public interface ImageRepository extends JpaRepository { +public interface S3ImageRepository extends JpaRepository { @Query("SELECT image FROM Image image WHERE image.id = ?1") @EntityGraph(attributePaths = { "viewers" }) - ImageEntity getByIdWithViewers(long id); + S3ImageEntity getByIdWithViewers(long id); - List findAllByOwner(UserEntity owner); + List findAllByOwner(UserEntity owner); + Optional findByOwnerAndUserFilename(UserEntity owner, String filename); } diff --git a/src/main/java/app/mealsmadeeasy/api/image/S3ImageService.java b/src/main/java/app/mealsmadeeasy/api/image/S3ImageService.java index d3ebdfb..e922dce 100644 --- a/src/main/java/app/mealsmadeeasy/api/image/S3ImageService.java +++ b/src/main/java/app/mealsmadeeasy/api/image/S3ImageService.java @@ -18,12 +18,12 @@ import java.util.UUID; public class S3ImageService implements ImageService { private final S3Manager s3Manager; - private final ImageRepository imageRepository; + private final S3ImageRepository imageRepository; private final String imageBucketName; public S3ImageService( S3Manager s3Manager, - ImageRepository imageRepository, + S3ImageRepository imageRepository, @Value("${app.mealsmadeeasy.api.images.bucketName}") String imageBucketName ) { this.s3Manager = s3Manager; @@ -51,12 +51,11 @@ public class S3ImageService implements ImageService { this.imageBucketName, filename, mimeType, inputStream, objectSize ); - final ImageEntity draft = new ImageEntity(); + final S3ImageEntity draft = new S3ImageEntity(); draft.setOwner((UserEntity) owner); draft.setUserFilename(userFilename); draft.setMimeType(mimeType); draft.setObjectName(objectName); - draft.setInternalUrl(this.s3Manager.getUrl(this.imageBucketName, objectName)); return this.imageRepository.save(draft); } @@ -77,15 +76,26 @@ public class S3ImageService implements ImageService { } @Override - public InputStream getImageContentById(long id) throws IOException, ImageException { - final Image image = this.getById(id); - return this.s3Manager.load(this.imageBucketName, image.getObjectName()); + @PostAuthorize("@imageSecurity.isViewableBy(returnObject, #viewer)") + public Image getByOwnerAndFilename(User viewer, User owner, String filename) throws ImageException { + return this.imageRepository.findByOwnerAndUserFilename((UserEntity) owner, filename) + .orElseThrow(() -> new ImageException( + ImageException.Type.IMAGE_NOT_FOUND, + "No such image for owner " + owner + " with filename " + filename + )); } @Override - public InputStream getImageContentById(long id, User viewer) throws IOException, ImageException { - final Image image = this.getById(id, viewer); - return this.s3Manager.load(this.imageBucketName, image.getObjectName()); + public InputStream getImageContentByOwnerAndFilename(User viewer, User owner, String filename) + throws ImageException, IOException { + final S3ImageEntity imageEntity = (S3ImageEntity) this.getByOwnerAndFilename(viewer, owner, filename); + return this.s3Manager.load(this.imageBucketName, imageEntity.getObjectName()); + } + + @Override + public InputStream getImageContentByOwnerAndFilename(User owner, String filename) throws ImageException, IOException { + final S3ImageEntity imageEntity = (S3ImageEntity) this.getByOwnerAndFilename(null, owner, filename); + return this.s3Manager.load(this.imageBucketName, imageEntity.getObjectName()); } @Override @@ -96,7 +106,7 @@ public class S3ImageService implements ImageService { @Override @PreAuthorize("@imageSecurity.isOwner(#image, #oldOwner)") public Image updateOwner(Image image, User oldOwner, User newOwner) { - final ImageEntity imageEntity = (ImageEntity) image; + final S3ImageEntity imageEntity = (S3ImageEntity) image; imageEntity.setOwner((UserEntity) newOwner); return this.imageRepository.save(imageEntity); } @@ -104,7 +114,7 @@ public class S3ImageService implements ImageService { @Override @PreAuthorize("@imageSecurity.isOwner(#image, #owner)") public Image setAlt(Image image, User owner, String alt) { - final ImageEntity imageEntity = (ImageEntity) image; + final S3ImageEntity imageEntity = (S3ImageEntity) image; imageEntity.setAlt(alt); return this.imageRepository.save(imageEntity); } @@ -112,7 +122,7 @@ public class S3ImageService implements ImageService { @Override @PreAuthorize("@imageSecurity.isOwner(#image, #owner)") public Image setCaption(Image image, User owner, String caption) { - final ImageEntity imageEntity = (ImageEntity) image; + final S3ImageEntity imageEntity = (S3ImageEntity) image; imageEntity.setCaption(caption); return this.imageRepository.save(imageEntity); } @@ -120,7 +130,7 @@ public class S3ImageService implements ImageService { @Override @PreAuthorize("@imageSecurity.isOwner(#image, #owner)") public Image setPublic(Image image, User owner, boolean isPublic) { - final ImageEntity imageEntity = (ImageEntity) image; + final S3ImageEntity imageEntity = (S3ImageEntity) image; imageEntity.setPublic(isPublic); return this.imageRepository.save(imageEntity); } @@ -128,7 +138,7 @@ public class S3ImageService implements ImageService { @Override @PreAuthorize("@imageSecurity.isOwner(#image, #owner)") public Image addViewer(Image image, User owner, User viewer) { - final ImageEntity withViewers = this.imageRepository.getByIdWithViewers(image.getId()); + final S3ImageEntity withViewers = this.imageRepository.getByIdWithViewers(image.getId()); withViewers.getViewers().add((UserEntity) viewer); return this.imageRepository.save(withViewers); } @@ -136,7 +146,7 @@ public class S3ImageService implements ImageService { @Override @PreAuthorize("@imageSecurity.isOwner(#image, #owner)") public Image removeViewer(Image image, User owner, User viewer) { - final ImageEntity withViewers = this.imageRepository.getByIdWithViewers(image.getId()); + final S3ImageEntity withViewers = this.imageRepository.getByIdWithViewers(image.getId()); withViewers.getViewers().remove((UserEntity) viewer); return this.imageRepository.save(withViewers); } @@ -144,7 +154,7 @@ public class S3ImageService implements ImageService { @Override @PreAuthorize("@imageSecurity.isOwner(#image, #owner)") public Image clearViewers(Image image, User owner) { - final ImageEntity withViewers = this.imageRepository.getByIdWithViewers(image.getId()); + final S3ImageEntity withViewers = this.imageRepository.getByIdWithViewers(image.getId()); withViewers.getViewers().clear(); return this.imageRepository.save(withViewers); } @@ -152,14 +162,9 @@ public class S3ImageService implements ImageService { @Override @PreAuthorize("@imageSecurity.isOwner(#image, #owner)") public void deleteImage(Image image, User owner) throws IOException { - this.imageRepository.delete((ImageEntity) image); - this.s3Manager.delete("images", image.getObjectName()); - } - - @Override - public void deleteById(long id, User owner) throws IOException { - final ImageEntity toDelete = this.imageRepository.getReferenceById(id); - this.deleteImage(toDelete, owner); + final S3ImageEntity imageEntity = (S3ImageEntity) image; + this.imageRepository.delete(imageEntity); + this.s3Manager.delete("images", imageEntity.getObjectName()); } }