Merge pull request #456 from kagemomiji/issue432-fix-cover-art-not-fo…

…und-error

#432 check media file existence before getting cover art

airsonic-main/pom.xml

@@ -657,7 +657,6 @@
             <plugin>
               <groupId>org.codehaus.mojo</groupId>
               <artifactId>buildnumber-maven-plugin</artifactId>
-              <version>3.2.0</version>
               <executions>
                 <execution>
                   <phase>generate-resources</phase>

airsonic-main/src/main/java/org/airsonic/player/controller/CoverArtController.java

@@ -189,6 +189,9 @@ private void sendUnscaled(CoverArtRequest coverArtRequest, HttpServletResponse r
                 coverArtRequest.getCoverArt().getFullPath());
 
         try (InputStream in = imageInputStreamWithType.getLeft()) {
+            if (in == null) {
+                throw new FileNotFoundException("Cover art not found");
+            }
             response.setContentType(imageInputStreamWithType.getRight());
             IOUtils.copy(in, response.getOutputStream());
         }

airsonic-main/src/main/java/org/airsonic/player/controller/MusicFolderSettingsController.java

@@ -135,14 +135,14 @@ private void expunge() {
         }
 
         LOG.debug("Cleaning database...");
+        LOG.debug("Deleting non-present cover art...");
+        coverArtService.expunge();
         LOG.debug("Deleting non-present artists...");
         artistService.expunge();
         LOG.debug("Deleting non-present albums...");
         albumService.expunge();
         LOG.debug("Deleting non-present media files...");
         mediaFileService.expunge();
-        LOG.debug("Deleting non-present cover art...");
-        coverArtService.expunge();
         LOG.debug("Deleting non-present media folders...");
         mediaFolderService.expunge();
         LOG.debug("Refreshing playlist stats...");

airsonic-main/src/main/java/org/airsonic/player/domain/CoverArt.java

@@ -49,15 +49,15 @@ public class CoverArt {
     @Column(name = "updated")
     private Instant updated = created;
 
-    @OneToOne(fetch = FetchType.LAZY)
+    @OneToOne(fetch = FetchType.EAGER)
     @JoinColumn(name = "entity_id", insertable = false, updatable = false)
     private Artist artist;
 
-    @OneToOne(fetch = FetchType.LAZY)
+    @OneToOne(fetch = FetchType.EAGER)
     @JoinColumn(name = "entity_id", insertable = false, updatable = false)
     private Album album;
 
-    @OneToOne(fetch = FetchType.LAZY)
+    @OneToOne(fetch = FetchType.EAGER)
     @JoinColumn(name = "entity_id", insertable = false, updatable = false)
     private MediaFile mediaFile;
 

airsonic-main/src/main/java/org/airsonic/player/domain/MediaFile.java

@@ -21,13 +21,16 @@
 package org.airsonic.player.domain;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
+import org.airsonic.player.util.FileUtil;
 import org.apache.commons.io.FilenameUtils;
 
 import jakarta.persistence.*;
 
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.time.Instant;
+import java.time.temporal.ChronoUnit;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
@@ -188,6 +191,23 @@ public Path getFullPath() {
         return folder.getPath().resolve(path);
     }
 
+    @JsonIgnore
+    public boolean isExist() {
+        return Files.exists(getFullPath());
+    }
+
+    /**
+     * Returns true if the file has been changed since the last time it was scanned.
+     *
+     * @return true if the file has been changed since the last time it was scanned. True if the file does not exist.
+     */
+    @JsonIgnore
+    public boolean isChanged() {
+        Path fullPath = this.getFullPath();
+        return !Files.exists(fullPath) || this.changed.truncatedTo(ChronoUnit.MICROS)
+                .compareTo(FileUtil.lastModified(fullPath).truncatedTo(ChronoUnit.MICROS)) < 0;
+    }
+
     public String getIndexPath() {
         return indexPath;
     }

airsonic-main/src/main/java/org/airsonic/player/domain/dto/CoverArtRequest.java

@@ -3,6 +3,7 @@
 import org.airsonic.player.domain.CoverArt;
 import org.airsonic.player.util.FileUtil;
 
+import java.nio.file.Files;
 import java.time.Instant;
 import java.util.Optional;
 import java.util.function.Supplier;
@@ -29,7 +30,7 @@ public String getKey() {
     }
 
     public Instant lastModified() {
-        return Optional.ofNullable(coverArt).map(c -> FileUtil.lastModified(c.getFullPath()))
+        return Optional.ofNullable(coverArt).filter(c -> Files.exists(c.getFullPath())).map(c -> FileUtil.lastModified(c.getFullPath()))
                 .orElseGet(lastModifiedGenerator);
     }
 

airsonic-main/src/main/java/org/airsonic/player/service/CoverArtCreateService.java

@@ -191,14 +191,17 @@ public CoverArtRequest createMediaFileCoverArtRequest(int mediaFileId, int offse
      * @return the cover art request
      */
     @Nullable
-    public CoverArtRequest createMediaFileCoverArtRequest(MediaFile mediaFile, int offset) {
+    public CoverArtRequest createMediaFileCoverArtRequest(@Nullable MediaFile mediaFile, int offset) {
         if (mediaFile == null) {
             return null;
         }
         if (mediaFile.isVideo()) {
             return new VideoCoverArtRequest(mediaFile, offset);
         }
         MediaFile dir = mediaFile.isDirectory() ? mediaFile : mediaFileService.getParentOf(mediaFile);
+        if (dir == null || !dir.isExist()) {
+            return null;
+        }
         CoverArt coverArt = coverArtService.getMediaFileArt(dir.getId());
         return new MediaFileCoverArtRequest(coverArt, dir, mediaFile.isDirectory() ? null : mediaFile.getId());
     }
@@ -300,6 +303,7 @@ public BufferedImage createVideoImage(VideoCoverArtRequest request, int size) {
      * audio file,
      * the embedded album art is returned.
      */
+    @Nullable
     private InputStream getImageInputStream(CoverArt art) throws IOException {
         return getImageInputStreamWithType(art.getFullPath()).getLeft();
     }
@@ -310,24 +314,22 @@ private InputStream getImageInputStream(CoverArt art) throws IOException {
      * the embedded album art is returned. In addition returns the mime type
      */
     public Pair<InputStream, String> getImageInputStreamWithType(Path file) throws IOException {
-        InputStream is;
-        String mimeType;
-        if (JaudiotaggerParser.isImageAvailable(file)) {
-            LOG.trace("Using Jaudio Tagger for reading artwork from {}", file);
-            try {
-                LOG.trace("Reading artwork from file {}", file);
+        try {
+            if (JaudiotaggerParser.isImageAvailable(file)) {
+                LOG.trace("Using Jaudio Tagger for reading artwork from {}", file);
                 Artwork artwork = JaudiotaggerParser.getArtwork(file);
-                is = new ByteArrayInputStream(artwork.getBinaryData());
-                mimeType = artwork.getMimeType();
-            } catch (Exception e) {
-                LOG.debug("Could not read artwork from file {}", file);
-                throw new RuntimeException(e);
+                return Pair.of(new ByteArrayInputStream(artwork.getBinaryData()), artwork.getMimeType());
+            } else {
+                LOG.trace("Reading artwork from file {}", file);
+                return Pair.of(
+                    new BufferedInputStream(Files.newInputStream(file)),
+                    StringUtil.getMimeType(FilenameUtils.getExtension(file.toString()))
+                );
             }
-        } else {
-            is = new BufferedInputStream(Files.newInputStream(file));
-            mimeType = StringUtil.getMimeType(FilenameUtils.getExtension(file.toString()));
+        } catch (Exception e) {
+            LOG.debug("Could not read artwork from file {}", file);
+            return Pair.of(null, null);
         }
-        return Pair.of(is, mimeType);
     }
 
     private InputStream getImageInputStreamForVideo(MediaFile mediaFile, int width, int height, int offset)

airsonic-main/src/main/java/org/airsonic/player/service/CoverArtService.java

@@ -22,6 +22,8 @@
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
+import jakarta.annotation.Nullable;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Files;
@@ -135,24 +137,17 @@ public CoverArt getMediaFileArt(@Param("id") int id) {
         return art;
     }
 
+    @Nullable
     public Path getMediaFileArtPath(int id) {
         CoverArt art = getMediaFileArt(id);
         return getFullPath(art);
     }
 
-    public Path getFullPath(CoverArt art) {
+    @Nullable
+    public Path getFullPath(@Nullable CoverArt art) {
         if (art != null && !CoverArt.NULL_ART.equals(art)) {
-            if (art.getFolder() == null) {
-                // null folder ids mean absolute paths
-                return art.getRelativePath();
-            } else {
-                MusicFolder folder = art.getFolder();
-                if (folder != null) {
-                    return art.getFullPath(folder.getPath());
-                }
-            }
+            return art.getFullPath();
         }
-
         return null;
     }
 
@@ -166,10 +161,18 @@ public void delete(EntityType type, Integer id) {
     public void expunge() {
         coverArtCache.clear();
         List<CoverArt> expungeCoverArts = coverArtRepository.findAll().stream()
-            .filter(art ->
-                (art.getEntityType() == EntityType.ALBUM && art.getAlbum() == null) ||
-                (art.getEntityType() == EntityType.ARTIST && art.getArtist() == null) ||
-                (art.getEntityType() == EntityType.MEDIA_FILE && art.getMediaFile() == null))
+            .filter(art -> {
+                switch (art.getEntityType()) {
+                    case ALBUM:
+                        return art.getAlbum() == null || !art.getAlbum().isPresent();
+                    case ARTIST:
+                        return art.getArtist() == null || !art.getArtist().isPresent();
+                    case MEDIA_FILE:
+                        return art.getMediaFile() == null || !art.getMediaFile().isPresent();
+                    default:
+                        return false;
+                }
+            })
             .collect(Collectors.toList());
         coverArtRepository.deleteAll(expungeCoverArts);
     }

airsonic-main/src/main/java/org/airsonic/player/service/MediaFileService.java

@@ -61,7 +61,6 @@
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Direction;
-import org.springframework.data.repository.query.Param;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Isolation;
 import org.springframework.transaction.annotation.Transactional;
@@ -213,9 +212,20 @@ public MediaFile getMediaFile(Path relativePath, MusicFolder folder, Double star
      * @param id The media file id.
      * @return mediafile for the given id.
      */
-    public MediaFile getMediaFile(@Param("id") Integer id) {
+    public MediaFile getMediaFile(Integer id) {
+        return getMediaFile(id, false);
+    }
+
+    /**
+     * Returns the media file for checking last modified.
+     *
+     * @param id The media file id.
+     * @param ignoreCache Whether to ignore the cache
+     * @return mediafile for the given id.
+     */
+    public MediaFile getMediaFile(Integer id, boolean ignoreCache) {
         if (Objects.isNull(id)) return null;
-        MediaFile result = mediaFileCache.getMediaFileById(id);
+        MediaFile result = ignoreCache ? null : mediaFileCache.getMediaFileById(id);
         if (result == null) {
             result = mediaFileRepository.findById(id).map(mediaFile -> checkLastModified(mediaFile, settingsService.isFastCacheEnabled())).orElse(null);
             mediaFileCache.putMediaFileById(id, result);
@@ -231,7 +241,15 @@ public MediaFile getParentOf(MediaFile mediaFile) {
         return getParentOf(mediaFile, settingsService.isFastCacheEnabled());
     }
 
-    public MediaFile getParentOf(MediaFile mediaFile, boolean minimizeDiskAccess) {
+    /**
+     * Returns the parent of the given media file.
+     *
+     * @param mediaFile The media file. Must not be {@code null}.
+     * @param minimizeDiskAccess Whether to refrain from checking for new or changed files
+     * @return The parent of the given media file. May be {@code null}.
+     */
+    @Nullable
+    public MediaFile getParentOf(@Nonnull MediaFile mediaFile, boolean minimizeDiskAccess) {
         if (mediaFile.getParentPath() == null) {
             return null;
         }
@@ -243,21 +261,11 @@ private boolean needsUpdate(MediaFile mediaFile, boolean minimizeDiskAccess) {
                 && !mediaFile.isIndexedTrack() // ignore virtual track
                 && (mediaFile.getVersion() < MediaFile.VERSION
                     || settingsService.getFullScan()
-                    || isFileUpdated(mediaFile)
+                    || mediaFile.isChanged()
                     || (mediaFile.hasIndex() && mediaFile.getChanged().truncatedTo(ChronoUnit.MICROS).compareTo(FileUtil.lastModified(mediaFile.getFullIndexPath()).truncatedTo(ChronoUnit.MICROS)) < 0)
                 );
     }
 
-    /**
-     * Check if the media file is updated.
-     *
-     * @param mediaFile The media file.
-     * @return
-     */
-    private boolean isFileUpdated(@Nonnull MediaFile mediaFile) {
-        return Files.exists(mediaFile.getFullPath()) && mediaFile.getChanged().truncatedTo(ChronoUnit.MICROS).compareTo(FileUtil.lastModified(mediaFile.getFullPath()).truncatedTo(ChronoUnit.MICROS)) < 0;
-    }
-
     /**
      * Return the media file for checking last modified.
      *
@@ -1662,6 +1670,7 @@ public MediaFile delete(MediaFile file) {
         file.setPresent(false);
         file.setChildrenLastUpdated(Instant.ofEpochMilli(1));
         mediaFileRepository.save(file);
+        coverArtService.delete(EntityType.MEDIA_FILE, file.getId());
         return file;
     }
 

airsonic-main/src/main/java/org/airsonic/player/service/PlayQueueService.java

@@ -32,6 +32,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Function;
@@ -148,9 +149,10 @@ public int savePlayQueue(Player player, int index, long offset) {
     @Transactional
     public int savePlayQueue(String username, List<Integer> mediaFileIds, Integer currentFileId, Long position, String changedBy) {
         SavedPlayQueue savedPlayQueue = savedPlayQueueRepository.findByUsername(username).orElse(new SavedPlayQueue(username));
-        List<MediaFile> mediaFiles = mediaFileIds.stream().map(mediaFileService::getMediaFile).collect(Collectors.toList());
+
+        List<MediaFile> mediaFiles = mediaFileIds.stream().map(mid -> mediaFileService.getMediaFile(mid, true)).filter(Objects::nonNull).collect(Collectors.toList());
         savedPlayQueue.setMediaFiles(mediaFiles);
-        savedPlayQueue.setCurrentMediaFile(mediaFileService.getMediaFile(currentFileId));
+        savedPlayQueue.setCurrentMediaFile(mediaFileService.getMediaFile(currentFileId, true));
         savedPlayQueue.setPositionMillis(position);
         savedPlayQueue.setChanged(Instant.now());
         savedPlayQueue.setChangedBy(changedBy);

airsonic-main/src/test/java/org/airsonic/player/service/CoverArtServiceTest.java

@@ -147,18 +147,27 @@ void testExpunge() {
                 new CoverArt(3, EntityType.MEDIA_FILE, "path/to/image.jpg", null, false),
                 new CoverArt(4, EntityType.ALBUM, "path/to/image.jpg", null, false),
                 new CoverArt(5, EntityType.ARTIST, "path/to/image.jpg", null, false),
-                new CoverArt(6, EntityType.MEDIA_FILE, "path/to/image.jpg", null, false)
+                new CoverArt(6, EntityType.MEDIA_FILE, "path/to/image.jpg", null, false),
+                new CoverArt(7, EntityType.ALBUM, "path/to/image.jpg", null, false),
+                new CoverArt(8, EntityType.ARTIST, "path/to/image.jpg", null, false),
+                new CoverArt(9, EntityType.MEDIA_FILE, "path/to/image.jpg", null, false)
         ).map(art -> {
-            if (art.getEntityId() < 4) {
+            if (art.getEntityId() < 7) {
                 switch (art.getEntityType()) {
                     case ALBUM:
-                        art.setAlbum(new Album());
+                        Album album = new Album();
+                        album.setPresent(art.getEntityId() <= 3); // only album with id 3 has an album present
+                        art.setAlbum(album);
                         break;
                     case ARTIST:
-                        art.setArtist(new Artist());
+                        Artist artist = new Artist();
+                        artist.setPresent(art.getEntityId() <= 3); // only artist with id 3 has an artist present
+                        art.setArtist(artist);
                         break;
                     case MEDIA_FILE:
-                        art.setMediaFile(new MediaFile());
+                        MediaFile mediaFile = new MediaFile();
+                        mediaFile.setPresent(art.getEntityId() <= 3); // only media file with id 3 has a media file present
+                        art.setMediaFile(mediaFile);
                         break;
                     case NONE:
                         break;
@@ -171,7 +180,7 @@ void testExpunge() {
         coverArtService.expunge();
         verify(coverArtRepository, times(1)).deleteAll(coverArtListCaptor.capture());
         List<CoverArt> deletedCoverArt = coverArtListCaptor.getValue();
-        assertEquals(3, deletedCoverArt.size());
+        assertEquals(6, deletedCoverArt.size());
         assertTrue(deletedCoverArt.stream().allMatch(art -> art.getEntityId() > 3));
     }