jukebox code quality and unit tests

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

@@ -28,15 +28,10 @@ public class JukeboxJavaService {
 
     private static final float DEFAULT_GAIN = 0.75f;
 
-    @Autowired
     private AudioScrobblerService audioScrobblerService;
-    @Autowired
     private StatusService statusService;
-    @Autowired
     private SecurityService securityService;
-    @Autowired
     private MediaFileService mediaFileService;
-    @Autowired
     private JavaPlayerFactory javaPlayerFactory;
 
 
@@ -45,6 +40,19 @@ public class JukeboxJavaService {
     private Map<String, List<com.github.biconou.AudioPlayer.api.Player>> activeAudioPlayersPerMixer = new Hashtable<>();
     private final static String DEFAULT_MIXER_ENTRY_KEY = "_default";
 
+
+    public JukeboxJavaService(AudioScrobblerService audioScrobblerService,
+                              StatusService statusService,
+                              SecurityService securityService,
+                              MediaFileService mediaFileService,
+                              JavaPlayerFactory javaPlayerFactory) {
+        this.audioScrobblerService = audioScrobblerService;
+        this.statusService = statusService;
+        this.securityService = securityService;
+        this.mediaFileService = mediaFileService;
+        this.javaPlayerFactory = javaPlayerFactory;
+    }
+
     /**
      * Finds the corresponding active audio player for a given airsonic player.
      * If no player exists we create one.
@@ -58,9 +66,6 @@ public class JukeboxJavaService {
         if (foundPlayer == null) {
             synchronized (activeAudioPlayers) {
                 com.github.biconou.AudioPlayer.api.Player newPlayer = initAudioPlayer(airsonicPlayer);
-                if (newPlayer == null) {
-                    throw new RuntimeException("Did not initialized a player");
-                }
                 activeAudioPlayers.put(airsonicPlayer.getId(), newPlayer);
                 String mixer = airsonicPlayer.getJavaJukeboxMixer();
                 if (StringUtils.isBlank(mixer)) {
@@ -96,8 +101,8 @@ public class JukeboxJavaService {
             log.info("use default mixer");
             audioPlayer = javaPlayerFactory.createJavaPlayer();
         }
-        audioPlayer.setGain(DEFAULT_GAIN);
         if (audioPlayer != null) {
+            audioPlayer.setGain(DEFAULT_GAIN);
             audioPlayer.registerListener(new PlayerListener() {
                 @Override
                 public void onBegin(int index, File currentFile) {
@@ -123,7 +128,6 @@ public class JukeboxJavaService {
                 public void onPause() {
                     // Nothing to do here
                 }
-
             });
             log.info("New audio player {} has been initialized.", audioPlayer.toString());
         } else {
@@ -276,7 +280,7 @@ public class JukeboxJavaService {
         play(airsonicPlayer);
     }
 
-    public void stop(Player airsonicPlayer) throws Exception {
+    public void stop(Player airsonicPlayer) {
         log.debug("begin stop jukebox : player = id:{};name:{}", airsonicPlayer.getId(), airsonicPlayer.getName());
 
         com.github.biconou.AudioPlayer.api.Player audioPlayer = retrieveAudioPlayerForAirsonicPlayer(airsonicPlayer);
@@ -320,21 +324,4 @@ public class JukeboxJavaService {
             }
         }
     }
-
-    public void setAudioScrobblerService(AudioScrobblerService audioScrobblerService) {
-        this.audioScrobblerService = audioScrobblerService;
-    }
-
-    public void setStatusService(StatusService statusService) {
-        this.statusService = statusService;
-    }
-
-    public void setSecurityService(SecurityService securityService) {
-        this.securityService = securityService;
-    }
-
-    public void setMediaFileService(MediaFileService mediaFileService) {
-        this.mediaFileService = mediaFileService;
-    }
-
 }

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

@@ -19,27 +19,24 @@
  */
 package org.airsonic.player.service;
 
-import org.airsonic.player.domain.*;
+import org.airsonic.player.domain.Player;
-import org.slf4j.Logger;
+import org.airsonic.player.domain.PlayerTechnology;
-import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 
 /**
- *
+ * @author Rémi Cocula
- *
- * @author R?mi Cocula
  */
 @Service
 public class JukeboxService {
 
-    private static final Logger log = LoggerFactory.getLogger(JukeboxService.class);
-
-    @Autowired
     private JukeboxLegacySubsonicService jukeboxLegacySubsonicService;
-    @Autowired
     private JukeboxJavaService jukeboxJavaService;
 
+    public JukeboxService(JukeboxLegacySubsonicService jukeboxLegacySubsonicService,
+                          JukeboxJavaService jukeboxJavaService) {
+        this.jukeboxLegacySubsonicService = jukeboxLegacySubsonicService;
+        this.jukeboxJavaService = jukeboxJavaService;
+    }
 
     public void setGain(Player airsonicPlayer, float gain) {
         switch (airsonicPlayer.getTechnology()) {
@@ -74,10 +71,6 @@ public class JukeboxService {
 
     /**
      * This method should be removed when the jukebox is controlled only through rest api.
-     *
-     * @param airsonicPlayer
-     * @param offset
-     * @throws Exception
      */
     @Deprecated
     public void updateJukebox(Player airsonicPlayer, int offset) throws Exception {
@@ -96,32 +89,8 @@ public class JukeboxService {
         return 0;
     }
 
-    /**
-     * This method is only here due to legacy considerations and should be removed
-     * if the jukeboxLegacySubsonicService is removed.
-     * @param airsonicPlayer
-     * @return
-     */
-    @Deprecated
-    public boolean canControl(Player airsonicPlayer) {
-        switch (airsonicPlayer.getTechnology()) {
-            case JUKEBOX:
-                if (jukeboxLegacySubsonicService.getPlayer() == null) {
-                    return false;
-                } else {
-                    return jukeboxLegacySubsonicService.getPlayer().getId().equals(airsonicPlayer.getId());
-                }
-            case JAVA_JUKEBOX:
-                return true;
-        }
-        return false;
-    }
-
     /**
      * Plays the playQueue of a jukebox player starting at the first item of the queue.
-     *
-     * @param airsonicPlayer
-     * @throws Exception
      */
     public void play(Player airsonicPlayer) throws Exception {
         switch (airsonicPlayer.getTechnology()) {
@@ -134,7 +103,6 @@ public class JukeboxService {
         }
     }
 
-
     public void start(Player airsonicPlayer) throws Exception {
         switch (airsonicPlayer.getTechnology()) {
             case JUKEBOX:
@@ -168,15 +136,22 @@ public class JukeboxService {
         }
     }
 
-
+    /**
-    /* properties setters */
+     * This method is only here due to legacy considerations and should be removed
-
+     * if the jukeboxLegacySubsonicService is removed.
-    public void setJukeboxLegacySubsonicService(JukeboxLegacySubsonicService jukeboxLegacySubsonicService) {
+     */
-        this.jukeboxLegacySubsonicService = jukeboxLegacySubsonicService;
+    @Deprecated
+    public boolean canControl(Player airsonicPlayer) {
+        switch (airsonicPlayer.getTechnology()) {
+            case JUKEBOX:
+                if (jukeboxLegacySubsonicService.getPlayer() == null) {
+                    return false;
+                } else {
+                    return jukeboxLegacySubsonicService.getPlayer().getId().equals(airsonicPlayer.getId());
                 }
-
+            case JAVA_JUKEBOX:
-    public void setJukeboxJavaService(JukeboxJavaService jukeboxJavaService) {
+                return true;
-        this.jukeboxJavaService = jukeboxJavaService;
+        }
+        return false;
     }
-
 }

airsonic-main/src/main/java/org/airsonic/player/service/jukebox/AudioPlayer.java

@@ -19,7 +19,6 @@
  */
 package org.airsonic.player.service.jukebox;
 
-import org.airsonic.player.service.JukeboxService;
 import org.apache.commons.io.IOUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -28,7 +27,6 @@ import javax.sound.sampled.AudioFormat;
 import javax.sound.sampled.AudioSystem;
 import javax.sound.sampled.FloatControl;
 import javax.sound.sampled.SourceDataLine;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.concurrent.atomic.AtomicReference;
@@ -46,7 +44,7 @@ import static org.airsonic.player.service.jukebox.AudioPlayer.State.*;
 public class AudioPlayer {
 
     public static final float DEFAULT_GAIN = 0.75f;
-    private static final Logger LOG = LoggerFactory.getLogger(JukeboxService.class);
+    private static final Logger LOG = LoggerFactory.getLogger(AudioPlayer.class);
 
     private final InputStream in;
     private final Listener listener;

airsonic-main/src/test/java/org/airsonic/player/api/AirsonicRestApiIntTest.java

@@ -24,10 +24,10 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
 @AutoConfigureMockMvc
 public class AirsonicRestApiIntTest {
 
-    public static final String CLIENT_NAME = "airsonic";
+    private static final String CLIENT_NAME = "airsonic";
-    public static final String AIRSONIC_USER = "admin";
+    private static final String AIRSONIC_USER = "admin";
-    public static final String AIRSONIC_PASSWORD = "admin";
+    private static final String AIRSONIC_PASSWORD = "admin";
-    public static final String EXPECTED_FORMAT = "json";
+    private static final String EXPECTED_FORMAT = "json";
 
     private static String AIRSONIC_API_VERSION;
 

airsonic-main/src/test/java/org/airsonic/player/api/jukebox/AbstractAirsonicRestApiJukeboxIntTest.java

@@ -58,8 +58,8 @@ public abstract class AbstractAirsonicRestApiJukeboxIntTest {
         }
     }
 
-    protected static final String CLIENT_NAME = "airsonic";
+    static final String CLIENT_NAME = "airsonic";
-    protected static final String JUKEBOX_PLAYER_NAME = CLIENT_NAME + "-jukebox";
+    static final String JUKEBOX_PLAYER_NAME = CLIENT_NAME + "-jukebox";
     private static final String EXPECTED_FORMAT = "json";
     private static String AIRSONIC_API_VERSION;
 

airsonic-main/src/test/java/org/airsonic/player/api/jukebox/AirsonicRestApiJukeboxIntTest.java

@@ -27,12 +27,12 @@ public class AirsonicRestApiJukeboxIntTest extends AbstractAirsonicRestApiJukebo
 
     @Override
     protected void createTestPlayer() {
-        Player jukeBoxPlayer = new Player();
+        Player jukeboxPlayer = new Player();
-        jukeBoxPlayer.setName(JUKEBOX_PLAYER_NAME);
+        jukeboxPlayer.setName(JUKEBOX_PLAYER_NAME);
-        jukeBoxPlayer.setUsername("admin");
+        jukeboxPlayer.setUsername("admin");
-        jukeBoxPlayer.setClientId(CLIENT_NAME + "-jukebox");
+        jukeboxPlayer.setClientId(CLIENT_NAME + "-jukebox");
-        jukeBoxPlayer.setTechnology(PlayerTechnology.JAVA_JUKEBOX);
+        jukeboxPlayer.setTechnology(PlayerTechnology.JAVA_JUKEBOX);
-        playerService.createPlayer(jukeBoxPlayer);
+        playerService.createPlayer(jukeboxPlayer);
     }
 
 }

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

@@ -0,0 +1,126 @@
+package org.airsonic.player.service;
+
+import org.airsonic.player.domain.*;
+import org.airsonic.player.service.jukebox.JavaPlayerFactory;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import static org.mockito.Mockito.*;
+
+@RunWith(MockitoJUnitRunner.class)
+public class JukeboxJavaServiceUnitTest {
+
+    private static final String USER_NAME = "admin";
+
+    private JukeboxJavaService service;
+    @Mock
+    private Player airsonicPlayer;
+    @Mock
+    private AudioScrobblerService audioScrobblerService;
+    @Mock
+    private StatusService statusService;
+    @Mock
+    private SecurityService securityService;
+    @Mock
+    private MediaFileService mediaFileService;
+    @Mock
+    private JavaPlayerFactory javaPlayerFactory;
+    @Mock
+    private com.github.biconou.AudioPlayer.api.Player player;
+    @Mock
+    private User user;
+    @Mock
+    private PlayQueue playQueue;
+    @Mock
+    private MediaFile mediaFile;
+
+
+    @Before
+    public void setup() {
+        service = new JukeboxJavaService(audioScrobblerService, statusService, securityService, mediaFileService, javaPlayerFactory);
+        when(airsonicPlayer.getTechnology()).thenReturn(PlayerTechnology.JAVA_JUKEBOX);
+        when(airsonicPlayer.getUsername()).thenReturn(USER_NAME);
+        when(javaPlayerFactory.createJavaPlayer()).thenReturn(player);
+        when(securityService.getUserByName(USER_NAME)).thenReturn(user);
+        when(user.isJukeboxRole()).thenReturn(true);
+        when(airsonicPlayer.getPlayQueue()).thenReturn(playQueue);
+        when(playQueue.getCurrentFile()).thenReturn(mediaFile);
+    }
+
+    @Test
+    public void play() {
+        // When
+        service.play(airsonicPlayer);
+        // Then
+        verify(javaPlayerFactory).createJavaPlayer();
+        verify(player).play();
+    }
+
+    @Test
+    public void playForNonDefaultMixer() {
+        // Given
+        when(airsonicPlayer.getJavaJukeboxMixer()).thenReturn("mixer");
+        when(javaPlayerFactory.createJavaPlayer("mixer")).thenReturn(player);
+        // When
+        service.play(airsonicPlayer);
+        // Then
+        verify(javaPlayerFactory).createJavaPlayer("mixer");
+        verify(player).play();
+    }
+
+    @Test
+    public void playAndStop() {
+        // When
+        service.play(airsonicPlayer);
+        // Then
+        verify(javaPlayerFactory).createJavaPlayer();
+        verify(player).play();
+        // When
+        service.stop(airsonicPlayer);
+        // Then
+        verifyNoMoreInteractions(javaPlayerFactory);
+        verify(player).pause();
+
+    }
+
+    @Test
+    public void playWithNonJukeboxUser() {
+        // Given
+        when(user.isJukeboxRole()).thenReturn(false);
+        // When
+        service.play(airsonicPlayer);
+        // Then
+        verify(javaPlayerFactory).createJavaPlayer();
+        verify(player, never()).play();
+    }
+
+    @Test(expected = RuntimeException.class)
+    public void playWithNonJukeboxPlayer() {
+        // Given
+        when(airsonicPlayer.getTechnology()).thenReturn(PlayerTechnology.WEB);
+        // When
+        service.play(airsonicPlayer);
+    }
+
+    @Test
+    public void playWithNoPlayQueueEmpty() {
+        // Given
+        when(playQueue.getCurrentFile()).thenReturn(null);
+        // When
+        service.play(airsonicPlayer);
+        // Then
+        verify(javaPlayerFactory).createJavaPlayer();
+        verify(player, never()).play();
+    }
+
+    @Test(expected = RuntimeException.class)
+    public void playerInitProblem() {
+        // Given
+        when(javaPlayerFactory.createJavaPlayer()).thenReturn(null);
+        // When
+        service.play(airsonicPlayer);
+    }
+}

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

@@ -0,0 +1,233 @@
+package org.airsonic.player.service;
+
+import org.airsonic.player.domain.Player;
+import org.airsonic.player.domain.PlayerTechnology;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import static org.mockito.Mockito.verify;
+import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.when;
+
+@RunWith(value = MockitoJUnitRunner.class)
+public class JukeboxServiceUnitTest {
+
+    private JukeboxService jukeboxService;
+    @Mock
+    private JukeboxLegacySubsonicService jukeboxLegacySubsonicService;
+    @Mock
+    private JukeboxJavaService jukeboxJavaService;
+    private Player jukeboxPlayer;
+    private Player legacyJukeboxPlayer;
+    private Player nonJukeboxPlayer;
+
+    @Before
+    public void setUp() {
+        jukeboxService = new JukeboxService(jukeboxLegacySubsonicService, jukeboxJavaService);
+        jukeboxPlayer = generateJukeboxPlayer();
+        legacyJukeboxPlayer = generateLegacyJukeboxPlayer();
+        nonJukeboxPlayer = generateNonJukeboxPlayer();
+    }
+
+    private Player generateNonJukeboxPlayer() {
+        Player player = new Player();
+        player.setId(0);
+        player.setTechnology(PlayerTechnology.WEB);
+        return player;
+    }
+
+    private Player generateLegacyJukeboxPlayer() {
+        Player player = new Player();
+        player.setId(1);
+        player.setTechnology(PlayerTechnology.JUKEBOX);
+        return player;
+    }
+
+    private Player generateJukeboxPlayer() {
+        Player player = new Player();
+        player.setId(2);
+        player.setTechnology(PlayerTechnology.JAVA_JUKEBOX);
+        return player;
+    }
+
+    @Test
+    public void setPositionWithJukeboxPlayer() {
+        // When
+        jukeboxService.setPosition(jukeboxPlayer, 0);
+        // Then
+        verify(jukeboxJavaService).setPosition(jukeboxPlayer, 0);
+    }
+
+    @Test(expected = UnsupportedOperationException.class)
+    public void setPositionWithLegacyJukeboxPlayer() {
+        // When
+        jukeboxService.setPosition(legacyJukeboxPlayer, 0);
+    }
+
+    @Test
+    public void getGainWithJukeboxPlayer() {
+        // When
+        jukeboxService.getGain(jukeboxPlayer);
+        // Then
+        verify(jukeboxJavaService).getGain(jukeboxPlayer);
+    }
+
+    @Test
+    public void getGainWithLegacyJukeboxPlayer() {
+        // When
+        jukeboxService.getGain(legacyJukeboxPlayer);
+        // Then
+        verify(jukeboxLegacySubsonicService).getGain();
+    }
+
+    @Test
+    public void getGainWithNonJukeboxPlayer() {
+        // When
+        float gain = jukeboxService.getGain(nonJukeboxPlayer);
+        // Then
+        assertThat(gain).isEqualTo(0);
+    }
+
+    @Test
+    public void updateJukebox() throws Exception {
+        // When
+        jukeboxService.updateJukebox(legacyJukeboxPlayer, 0);
+        // Then
+        verify(jukeboxLegacySubsonicService).updateJukebox(legacyJukeboxPlayer, 0);
+    }
+
+    @Test
+    public void getPositionWithJukeboxPlayer() {
+        // When
+        jukeboxService.getPosition(jukeboxPlayer);
+        // Then
+        verify(jukeboxJavaService).getPosition(jukeboxPlayer);
+    }
+
+    @Test
+    public void getPositionWithLegacyJukeboxPlayer() {
+        // When
+        jukeboxService.getPosition(legacyJukeboxPlayer);
+        // Then
+        verify(jukeboxLegacySubsonicService).getPosition();
+    }
+
+    @Test
+    public void getPasitionWithNonJukeboxPlayer() {
+        // When
+        int position = jukeboxService.getPosition(nonJukeboxPlayer);
+        // Then
+        assertThat(position).isEqualTo(0);
+    }
+
+    @Test
+    public void setGainWithJukeboxPlayer() {
+        // When
+        jukeboxService.setGain(jukeboxPlayer, 0.5f);
+        // Then
+        verify(jukeboxJavaService).setGain(jukeboxPlayer, 0.5f);
+    }
+
+    @Test
+    public void setGaintWithLegacyJukeboxPlayer() {
+        // When
+        jukeboxService.setGain(legacyJukeboxPlayer, 0.5f);
+        // Then
+        verify(jukeboxLegacySubsonicService).setGain(0.5f);
+    }
+
+    @Test
+    public void startWithJukeboxPlayer() throws Exception {
+        // When
+        jukeboxService.start(jukeboxPlayer);
+        // Then
+        verify(jukeboxJavaService).start(jukeboxPlayer);
+    }
+
+    @Test
+    public void startWithLegacyJukeboxPlayer() throws Exception {
+        // When
+        jukeboxService.start(legacyJukeboxPlayer);
+
+        // Then
+        verify(jukeboxLegacySubsonicService).updateJukebox(legacyJukeboxPlayer, 0);
+    }
+
+    @Test
+    public void playWithJukeboxPlayer() throws Exception {
+        // When
+        jukeboxService.play(jukeboxPlayer);
+        // Then
+        verify(jukeboxJavaService).play(jukeboxPlayer);
+    }
+
+    @Test
+    public void playWithLegacyJukeboxPlayer() throws Exception {
+        // When
+        jukeboxService.play(legacyJukeboxPlayer);
+        // Then
+        verify(jukeboxLegacySubsonicService).updateJukebox(legacyJukeboxPlayer, 0);
+    }
+
+    @Test
+    public void stopWithJukeboxPlayer() throws Exception {
+        // When
+        jukeboxService.stop(jukeboxPlayer);
+        // Then
+        verify(jukeboxJavaService).stop(jukeboxPlayer);
+    }
+
+    @Test
+    public void stopWithLegacyJukeboxPlayer() throws Exception {
+        // When
+        jukeboxService.stop(legacyJukeboxPlayer);
+        // Then
+        verify(jukeboxLegacySubsonicService).updateJukebox(legacyJukeboxPlayer, 0);
+    }
+
+
+    @Test
+    public void skipWithJukeboxPlayer() throws Exception {
+        // When
+        jukeboxService.skip(jukeboxPlayer, 0, 1);
+        // Then
+        verify(jukeboxJavaService).skip(jukeboxPlayer, 0, 1);
+    }
+
+    @Test
+    public void skipWithLegacyJukeboxPlayer() throws Exception {
+        // When
+        jukeboxService.skip(legacyJukeboxPlayer, 0, 1);
+        // Then
+        verify(jukeboxLegacySubsonicService).updateJukebox(legacyJukeboxPlayer, 1);
+    }
+
+    @Test
+    public void canControlWithJukeboxPlayer() {
+        // When
+        boolean canControl = jukeboxService.canControl(jukeboxPlayer);
+        // Then
+        assertThat(canControl).isEqualTo(true);
+    }
+
+    @Test
+    public void canControlWithLegacyJukeboxPlayer() {
+        // When
+        when(jukeboxLegacySubsonicService.getPlayer()).thenReturn(legacyJukeboxPlayer);
+        boolean canControl = jukeboxService.canControl(legacyJukeboxPlayer);
+        // Then
+        assertThat(canControl).isEqualTo(true);
+    }
+
+    @Test
+    public void canControlWithLegacyJukeboxPlayerWrongPlayer() {
+        // When
+        when(jukeboxLegacySubsonicService.getPlayer()).thenReturn(nonJukeboxPlayer);
+        boolean canControl = jukeboxService.canControl(legacyJukeboxPlayer);
+        // Then
+        assertThat(canControl).isEqualTo(false);
+    }
+}