Refactoring of S3ImageService and related.

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));
         }

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<UserEntity> getViewers();

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;

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;

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 getImageContentByOwnerAndFilename(User owner, String filename) throws ImageException, IOException;
-    InputStream getImageContentById(long id, User viewer) throws IOException, ImageException;
+    InputStream getImageContentByOwnerAndFilename(User viewer, User owner, String filename) throws ImageException, IOException;
 
     List<Image> 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;
 
 }

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 + ")";
     }
 
 }

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<ImageEntity, Long> {
+public interface S3ImageRepository extends JpaRepository<S3ImageEntity, Long> {
 
     @Query("SELECT image FROM Image image WHERE image.id = ?1")
     @EntityGraph(attributePaths = { "viewers" })
-    ImageEntity getByIdWithViewers(long id);
+    S3ImageEntity getByIdWithViewers(long id);
 
-    List<ImageEntity> findAllByOwner(UserEntity owner);
+    List<S3ImageEntity> findAllByOwner(UserEntity owner);
+    Optional<S3ImageEntity> findByOwnerAndUserFilename(UserEntity owner, String filename);
 
 }

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 {
+    @PostAuthorize("@imageSecurity.isViewableBy(returnObject, #viewer)")
-        final Image image = this.getById(id);
+    public Image getByOwnerAndFilename(User viewer, User owner, String filename) throws ImageException {
-        return this.s3Manager.load(this.imageBucketName, image.getObjectName());
+        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 {
+    public InputStream getImageContentByOwnerAndFilename(User viewer, User owner, String filename)
-        final Image image = this.getById(id, viewer);
+            throws ImageException, IOException {
-        return this.s3Manager.load(this.imageBucketName, image.getObjectName());
+        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);
+        final S3ImageEntity imageEntity = (S3ImageEntity) image;
-        this.s3Manager.delete("images", image.getObjectName());
+        this.imageRepository.delete(imageEntity);
-    }
+        this.s3Manager.delete("images", imageEntity.getObjectName());
-
-    @Override
-    public void deleteById(long id, User owner) throws IOException {
-        final ImageEntity toDelete = this.imageRepository.getReferenceById(id);
-        this.deleteImage(toDelete, owner);
     }
 
 }