Skip to content

Commit

Permalink
Merge pull request #645 from kagemomiji/issue401-ordering-playlist
Browse files Browse the repository at this point in the history
#401 Fix random ordering issue of playlist
  • Loading branch information
kagemomiji authored Nov 26, 2024
2 parents b640a84 + 11c65e0 commit bf829c6
Show file tree
Hide file tree
Showing 14 changed files with 621 additions and 30 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/trivy_scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
uses: actions/checkout@v4

- name: Run Trivy vulnerability scanner
uses: aquasecurity/trivy-action@0.28.0
uses: aquasecurity/trivy-action@0.29.0
with:
scan-type: 'fs'
format: 'sarif'
Expand Down
2 changes: 1 addition & 1 deletion airsonic-main/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@
<dependency>
<groupId>com.mattbertolini</groupId>
<artifactId>liquibase-slf4j</artifactId>
<version>5.0.0</version>
<version>5.1.0</version>
<scope>runtime</scope>
</dependency>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.fasterxml.jackson.annotation.JsonIgnore;

import jakarta.annotation.Nonnull;
import jakarta.persistence.*;

import java.time.Instant;
Expand Down Expand Up @@ -63,11 +64,9 @@ public class Playlist {
inverseJoinColumns = @JoinColumn(name = "username"))
private List<User> sharedUsers;

@ManyToMany
@JoinTable(name = "playlist_file",
joinColumns = @JoinColumn(name = "playlist_id", referencedColumnName = "id"),
inverseJoinColumns = @JoinColumn(name = "media_file_id", referencedColumnName = "id"))
private List<MediaFile> mediaFiles;
@OrderBy("orderIndex ASC")
@OneToMany(mappedBy = "playlist", cascade = CascadeType.ALL, orphanRemoval = true)
private List<PlaylistMediaFile> mediaFiles;

public Playlist() {
}
Expand Down Expand Up @@ -197,11 +196,15 @@ public void removeSharedUserByUsername(String username) {
}

@JsonIgnore
public List<MediaFile> getMediaFiles() {
return mediaFiles;
@Nonnull
public List<PlaylistMediaFile> getPlaylistMediaFiles() {
if (this.mediaFiles == null) {
this.mediaFiles = new ArrayList<>();
}
return this.mediaFiles;
}

public void setMediaFiles(List<MediaFile> mediaFiles) {
public void setPlaylistMediaFiles(List<PlaylistMediaFile> mediaFiles) {
if (this.mediaFiles == null) {
this.mediaFiles = new ArrayList<>();
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package org.airsonic.player.domain;

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.Table;

import java.io.Serializable;

@Entity
@Table(name = "playlist_file")
public class PlaylistMediaFile implements Serializable {

public PlaylistMediaFile() {
}

public PlaylistMediaFile(Playlist playlist, MediaFile mediaFile, int orderIndex) {
this.playlist = playlist;
this.mediaFile = mediaFile;
this.orderIndex = orderIndex;
}

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Integer id;

@ManyToOne
@JoinColumn(name = "playlist_id")
private Playlist playlist;

@ManyToOne
@JoinColumn(name = "media_file_id")
private MediaFile mediaFile;

@Column(name = "order_index")
private int orderIndex;

public Integer getId() {
return id;
}

public void setId(Integer id) {
this.id = id;
}

public Playlist getPlaylist() {
return playlist;
}

public void setPlaylist(Playlist playlist) {
this.playlist = playlist;
}

public MediaFile getMediaFile() {
return mediaFile;
}

public void setMediaFile(MediaFile mediaFile) {
this.mediaFile = mediaFile;
}

public int getOrderIndex() {
return orderIndex;
}

public void setOrderIndex(int orderIndex) {
this.orderIndex = orderIndex;
}


@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof PlaylistMediaFile)) return false;

PlaylistMediaFile that = (PlaylistMediaFile) o;

if (orderIndex != that.orderIndex) return false;
if (id != null ? !id.equals(that.id) : that.id != null) return false;
if (playlist != null ? !playlist.equals(that.playlist) : that.playlist != null) return false;
return mediaFile != null ? mediaFile.equals(that.mediaFile) : that.mediaFile == null;
}

@Override
public int hashCode() {
int result = id != null ? id.hashCode() : 0;
result = 31 * result + (playlist != null ? playlist.hashCode() : 0);
result = 31 * result + (mediaFile != null ? mediaFile.hashCode() : 0);
result = 31 * result + orderIndex;
return result;
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.airsonic.player.repository;

import org.airsonic.player.domain.PlaylistMediaFile;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;

@Repository
public interface PlaylistMediaFileRepository extends JpaRepository<PlaylistMediaFile, Integer> {
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.airsonic.player.domain.MediaFile;
import org.airsonic.player.domain.PlayQueue;
import org.airsonic.player.domain.Playlist;
import org.airsonic.player.domain.PlaylistMediaFile;
import org.airsonic.player.domain.User;
import org.airsonic.player.repository.PlaylistRepository;
import org.airsonic.player.repository.UserRepository;
Expand Down Expand Up @@ -148,16 +149,23 @@ public List<MediaFile> getFilesInPlaylist(int id) {

@Transactional(readOnly = true)
public List<MediaFile> getFilesInPlaylist(int id, boolean includeNotPresent) {
return playlistRepository.findById(id).map(p -> p.getMediaFiles()).orElseGet(
return playlistRepository.findById(id).map(p -> {
return p.getPlaylistMediaFiles().stream()
.map(PlaylistMediaFile::getMediaFile)
.filter(Objects::nonNull)
.filter(x -> x.isPresent() || includeNotPresent)
.collect(Collectors.toList());
}).orElseGet(
() -> {
LOG.warn("Playlist {} not found", id);
return new ArrayList<>();
}
).stream().filter(x -> x.isPresent() || includeNotPresent).collect(Collectors.toList());
);
}

@Transactional
public Playlist setFilesInPlaylist(int id, List<MediaFile> files) {
playlistCache.removePlaylistById(id);
return playlistRepository.findById(id).map(p -> {
Playlist playlist = setFilesInPlaylist(p, files);
playlistRepository.saveAndFlush(playlist);
Expand All @@ -170,7 +178,25 @@ public Playlist setFilesInPlaylist(int id, List<MediaFile> files) {
}

private Playlist setFilesInPlaylist(Playlist playlist, List<MediaFile> files) {
playlist.setMediaFiles(files);

int fileCount = files.size();
List<PlaylistMediaFile> playlistMediaFiles =
playlist.getPlaylistMediaFiles().stream()
.limit(fileCount).collect(Collectors.toList());
// update order index and media file
int orderIndex = 0;
for (MediaFile file : files) {
if (orderIndex < playlistMediaFiles.size()) {
PlaylistMediaFile pmf = playlistMediaFiles.get(orderIndex);
pmf.setMediaFile(file);
pmf.setOrderIndex(orderIndex);
} else {
PlaylistMediaFile pmf = new PlaylistMediaFile(playlist, file, orderIndex);
playlistMediaFiles.add(pmf);
}
orderIndex++;
}
playlist.setPlaylistMediaFiles(playlistMediaFiles);
playlist.setFileCount(files.size());
playlist.setDuration(files.stream().mapToDouble(MediaFile::getDuration).sum());
playlist.setChanged(Instant.now());
Expand All @@ -181,11 +207,11 @@ private Playlist setFilesInPlaylist(Playlist playlist, List<MediaFile> files) {
public void removeFilesInPlaylistByIndices(Integer id, List<Integer> indices) {
playlistCache.removePlaylistById(id);
playlistRepository.findById(id).ifPresentOrElse(p -> {
List<MediaFile> files = p.getMediaFiles();
List<PlaylistMediaFile> files = p.getPlaylistMediaFiles();
List<MediaFile> newFiles = new ArrayList<>();
for (int i = 0; i < files.size(); i++) {
if (!indices.contains(i)) {
newFiles.add(files.get(i));
newFiles.add(files.get(i).getMediaFile());
}
}
Playlist playlist = setFilesInPlaylist(p, newFiles);
Expand All @@ -202,8 +228,8 @@ public void removeFilesInPlaylistByIndices(Integer id, List<Integer> indices) {
@Transactional
public List<Playlist> refreshPlaylistsStats() {
return playlistRepository.findAll().stream().map(p -> {
p.setFileCount(p.getMediaFiles().size());
p.setDuration(p.getMediaFiles().stream().mapToDouble(MediaFile::getDuration).sum());
p.setFileCount(p.getPlaylistMediaFiles().size());
p.setDuration(p.getPlaylistMediaFiles().stream().map(PlaylistMediaFile::getMediaFile).mapToDouble(MediaFile::getDuration).sum());
p.setChanged(Instant.now());
playlistRepository.save(p);
return p;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import chameleon.playlist.SpecificPlaylist;
import chameleon.playlist.SpecificPlaylistProvider;
import org.airsonic.player.domain.MediaFile;
import org.airsonic.player.domain.PlaylistMediaFile;
import org.airsonic.player.repository.PlaylistRepository;
import org.airsonic.player.util.StringUtil;
import org.springframework.beans.factory.annotation.Autowired;
Expand All @@ -15,6 +16,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

@Component
public class DefaultPlaylistExportHandler implements PlaylistExportHandler {
Expand All @@ -37,7 +39,7 @@ public SpecificPlaylist handle(int id, SpecificPlaylistProvider provider) throws
private Playlist createChameleonGenericPlaylistFromDBId(int id) {
Playlist newPlaylist = new Playlist();
List<MediaFile> files = playlistRepository.findById(id).map(playlist -> {
return playlist.getMediaFiles();
return playlist.getPlaylistMediaFiles().stream().map(PlaylistMediaFile::getMediaFile).collect(Collectors.toList());
}).orElseGet(() -> {
return new ArrayList<>();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import chameleon.playlist.xspf.Location;
import chameleon.playlist.xspf.Track;
import chameleon.playlist.xspf.XspfProvider;
import org.airsonic.player.domain.MediaFile;
import org.airsonic.player.domain.Playlist;
import org.airsonic.player.repository.PlaylistRepository;
import org.airsonic.player.service.CoverArtService;
Expand Down Expand Up @@ -40,7 +41,7 @@ public SpecificPlaylist handle(int id, SpecificPlaylistProvider provider) {
return createXsfpPlaylistFromDBId(id);
}

chameleon.playlist.xspf.Playlist createXsfpPlaylistFromDBId(int id) {
private chameleon.playlist.xspf.Playlist createXsfpPlaylistFromDBId(int id) {
chameleon.playlist.xspf.Playlist newPlaylist = new chameleon.playlist.xspf.Playlist();
Playlist playlist = playlistRepository.findById(id).orElseGet(() -> {
LOG.error("Playlist with id {} not found", id);
Expand All @@ -50,7 +51,8 @@ chameleon.playlist.xspf.Playlist createXsfpPlaylistFromDBId(int id) {
newPlaylist.setCreator("Airsonic user " + playlist.getUsername());
newPlaylist.setDate(Date.from(Instant.now())); //TODO switch to Instant upstream

playlist.getMediaFiles().stream().map(mediaFile -> {
playlist.getPlaylistMediaFiles().stream().map(pmf -> {
MediaFile mediaFile = pmf.getMediaFile();
Track track = new Track();
track.setTrackNumber(mediaFile.getTrackNumber());
track.setCreator(mediaFile.getArtist());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<databaseChangeLog
xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.1.xsd">

<!-- 1. Check if track_number column exists, if not, add it -->
<changeSet id="add-order-index-to-playlist-file" author="kagemomiji">
<preConditions onFail="MARK_RAN">
<not>
<columnExists tableName="playlist_file" columnName="order_index"/>
</not>
</preConditions>
<addColumn tableName="playlist_file">
<column name="order_index" type="int" defaultValue="-1">
<constraints nullable="false"/>
</column>
</addColumn>
<rollback>
<dropColumn tableName="playlist_file" columnName="order_index"/>
</rollback>
</changeSet>

</databaseChangeLog>
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@
<include file="add-genre-pk.xml" relativeToChangelogFile="true"/>
<include file="change-hsqldb-table-type.xml" relativeToChangelogFile="true"/>
<include file="add-locked-column-podcast-episode.xml" relativeToChangelogFile="true"/>
<include file="add-order-index-column-playlist-file.xml" relativeToChangelogFile="true"/>
</databaseChangeLog>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.airsonic.player.domain.MediaFile;
import org.airsonic.player.domain.MusicFolder;
import org.airsonic.player.domain.Playlist;
import org.airsonic.player.domain.PlaylistMediaFile;
import org.airsonic.player.repository.PlaylistRepository;
import org.airsonic.player.repository.UserRepository;
import org.airsonic.player.service.playlist.DefaultPlaylistExportHandler;
Expand Down Expand Up @@ -89,7 +90,13 @@ public void testExportToM3U() throws Exception {

Playlist playlist = new Playlist();
playlist.setId(23);
playlist.setMediaFiles(getPlaylistFiles());
int orderIndex = 0;
List<PlaylistMediaFile> playlistMediaFiles = new ArrayList<>();
for (MediaFile mediaFile : getPlaylistFiles()) {
playlistMediaFiles.add(new PlaylistMediaFile(playlist, mediaFile, orderIndex));
orderIndex++;
}
playlist.setPlaylistMediaFiles(playlistMediaFiles);
when(playlistRepository.findById(eq(23))).thenReturn(Optional.of(playlist));
when(settingsService.getPlaylistExportFormat()).thenReturn("m3u");
when(mediaFolderService.getMusicFolderById(any())).thenReturn(mockedFolder);
Expand Down
Loading

0 comments on commit bf829c6

Please sign in to comment.