diff --git a/airsonic-main/pom.xml b/airsonic-main/pom.xml index d5f5d73a0..8dc79ac13 100644 --- a/airsonic-main/pom.xml +++ b/airsonic-main/pom.xml @@ -657,7 +657,6 @@ org.codehaus.mojo buildnumber-maven-plugin - 3.2.0 generate-resources diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/CoverArtController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/CoverArtController.java index 8b81addd8..a8aea6634 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/controller/CoverArtController.java +++ b/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()); } diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/MusicFolderSettingsController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/MusicFolderSettingsController.java index 367096458..e6563ddfa 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/controller/MusicFolderSettingsController.java +++ b/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..."); diff --git a/airsonic-main/src/main/java/org/airsonic/player/domain/CoverArt.java b/airsonic-main/src/main/java/org/airsonic/player/domain/CoverArt.java index 14e41f95f..fa44c551d 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/domain/CoverArt.java +++ b/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; diff --git a/airsonic-main/src/main/java/org/airsonic/player/domain/MediaFile.java b/airsonic-main/src/main/java/org/airsonic/player/domain/MediaFile.java index 831df8c73..cfe4d8037 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/domain/MediaFile.java +++ b/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; } diff --git a/airsonic-main/src/main/java/org/airsonic/player/domain/dto/CoverArtRequest.java b/airsonic-main/src/main/java/org/airsonic/player/domain/dto/CoverArtRequest.java index ccd8f9605..3293c458c 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/domain/dto/CoverArtRequest.java +++ b/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); } diff --git a/airsonic-main/src/main/java/org/airsonic/player/service/CoverArtCreateService.java b/airsonic-main/src/main/java/org/airsonic/player/service/CoverArtCreateService.java index 5bc3c7130..9c9167efd 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/service/CoverArtCreateService.java +++ b/airsonic-main/src/main/java/org/airsonic/player/service/CoverArtCreateService.java @@ -191,7 +191,7 @@ 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; } @@ -199,6 +199,9 @@ public CoverArtRequest createMediaFileCoverArtRequest(MediaFile mediaFile, int o 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 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) diff --git a/airsonic-main/src/main/java/org/airsonic/player/service/CoverArtService.java b/airsonic-main/src/main/java/org/airsonic/player/service/CoverArtService.java index cbfc5440e..42148856d 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/service/CoverArtService.java +++ b/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 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); } diff --git a/airsonic-main/src/main/java/org/airsonic/player/service/MediaFileService.java b/airsonic-main/src/main/java/org/airsonic/player/service/MediaFileService.java index d51ca2b98..cd851e02c 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/service/MediaFileService.java +++ b/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; } diff --git a/airsonic-main/src/main/java/org/airsonic/player/service/PlayQueueService.java b/airsonic-main/src/main/java/org/airsonic/player/service/PlayQueueService.java index dd09daa66..0585b02f0 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/service/PlayQueueService.java +++ b/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 mediaFileIds, Integer currentFileId, Long position, String changedBy) { SavedPlayQueue savedPlayQueue = savedPlayQueueRepository.findByUsername(username).orElse(new SavedPlayQueue(username)); - List mediaFiles = mediaFileIds.stream().map(mediaFileService::getMediaFile).collect(Collectors.toList()); + + List 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); diff --git a/airsonic-main/src/test/java/org/airsonic/player/service/CoverArtServiceTest.java b/airsonic-main/src/test/java/org/airsonic/player/service/CoverArtServiceTest.java index 47561c433..b9eaf5d97 100644 --- a/airsonic-main/src/test/java/org/airsonic/player/service/CoverArtServiceTest.java +++ b/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 deletedCoverArt = coverArtListCaptor.getValue(); - assertEquals(3, deletedCoverArt.size()); + assertEquals(6, deletedCoverArt.size()); assertTrue(deletedCoverArt.stream().allMatch(art -> art.getEntityId() > 3)); } diff --git a/pom.xml b/pom.xml index e4d800ebc..c035638ff 100644 --- a/pom.xml +++ b/pom.xml @@ -151,7 +151,7 @@ org.checkerframework checker-qual - 3.42.0 + 3.43.0 com.google.j2objc @@ -161,7 +161,7 @@ com.google.errorprone error_prone_annotations - 2.26.1 + 2.27.1 net.java.dev.jna @@ -197,7 +197,7 @@ org.codehaus.mojo buildnumber-maven-plugin - 1.4 + 3.2.0 org.apache.cxf