Handle player id as an Integer instead of String

2018-06-04 20:19:12 +01:00
parent b3f432a545
commit 6a44c5c815
21 changed files with 102 additions and 95 deletions
@@ -26,7 +26,7 @@ package org.airsonic.player.ajax;
 */
 public class NowPlayingInfo {

-    private final String playerId;
+    private final Integer playerId;
    private final String username;
    private final String artist;
    private final String title;
@@ -38,7 +38,7 @@ public class NowPlayingInfo {
    private final String avatarUrl;
    private final int minutesAgo;

-    public NowPlayingInfo(String playerId, String user, String artist, String title, String tooltip, String streamUrl, String albumUrl,
+    public NowPlayingInfo(Integer playerId, String user, String artist, String title, String tooltip, String streamUrl, String albumUrl,
                          String lyricsUrl, String coverArtUrl, String avatarUrl, int minutesAgo) {
        this.playerId = playerId;
        this.username = user;
@@ -53,7 +53,7 @@ public class NowPlayingInfo {
        this.minutesAgo = minutesAgo;
    }

-    public String getPlayerId() {
+    public Integer getPlayerId() {
        return playerId;
    }

@@ -149,7 +149,7 @@ public class PlayQueueService {
        return convert(request, player, false);
    }

-    public void savePlayQueue(int currentSongIndex, long positionMillis) {
+    public void savePlayQueue(int currentSongIndex, long positionMillis) throws Exception {
        HttpServletRequest request = WebContextFactory.get().getHttpServletRequest();
        HttpServletResponse response = WebContextFactory.get().getHttpServletResponse();

@@ -710,11 +710,11 @@ public class PlayQueueService {
        return mediaFile.getBitRate() + " Kbps";
    }

-    private Player getCurrentPlayer(HttpServletRequest request, HttpServletResponse response) {
+    private Player getCurrentPlayer(HttpServletRequest request, HttpServletResponse response) throws Exception {
        return playerService.getPlayer(request, response);
    }

-    private Player resolvePlayer() {
+    private Player resolvePlayer() throws Exception {
        return getCurrentPlayer(resolveHttpServletRequest(), resolveHttpServletResponse());
    }

@@ -732,7 +732,7 @@ public class PlayQueueService {
    // Methods dedicated to jukebox
    //

-    public void setGain(float gain) {
+    public void setGain(float gain) throws Exception {
        HttpServletRequest request = WebContextFactory.get().getHttpServletRequest();
        HttpServletResponse response = WebContextFactory.get().getHttpServletResponse();
        Player player = getCurrentPlayer(request, response);
@@ -741,7 +741,7 @@ public class PlayQueueService {
        }
    }

-    public void setJukeboxPosition(int positionInSeconds) {
+    public void setJukeboxPosition(int positionInSeconds) throws Exception {
        Player player = resolvePlayer();
        jukeboxService.setPosition(player,positionInSeconds);
    }
@@ -112,7 +112,7 @@ public class PlaylistService {
        return getReadablePlaylists();
    }

-    public int createPlaylistForPlayQueue() {
+    public int createPlaylistForPlayQueue() throws Exception {
        HttpServletRequest request = WebContextFactory.get().getHttpServletRequest();
        HttpServletResponse response = WebContextFactory.get().getHttpServletResponse();
        Player player = playerService.getPlayer(request, response);
@@ -34,7 +34,7 @@ import java.util.List;
 * @author Sindre Mehus
 */
 public class PlayerSettingsCommand {
-    private String playerId;
+    private Integer playerId;
    private String name;
    private String description;
    private String type;
@@ -55,11 +55,11 @@ public class PlayerSettingsCommand {
    private String javaJukeboxMixer;
    private String[] javaJukeboxMixers;

-    public String getPlayerId() {
+    public Integer getPlayerId() {
        return playerId;
    }

-    public void setPlayerId(String playerId) {
+    public void setPlayerId(Integer playerId) {
        this.playerId = playerId;
    }

@@ -99,7 +99,7 @@ public class DownloadController implements LastModified {
            MediaFile mediaFile = getMediaFile(request);

            Integer playlistId = ServletRequestUtils.getIntParameter(request, "playlist");
-            String playerId = request.getParameter("player");
+            Integer playerId = ServletRequestUtils.getIntParameter(request, "player");
            int[] indexes = request.getParameter("i") == null ? null : ServletRequestUtils.getIntParameters(request, "i");

            if (mediaFile != null) {
@@ -68,7 +68,7 @@ public class LeftController  {
     * Note: This class intentionally does not implement org.springframework.web.servlet.mvc.LastModified
     * as we don't need browser-side caching of left.jsp.  This method is only used by RESTController.
     */
-    long getLastModified(HttpServletRequest request) {
+    long getLastModified(HttpServletRequest request) throws Exception {
        saveSelectedMusicFolder(request);

        if (mediaScannerService.isScanning()) {
@@ -157,12 +157,11 @@ public class LeftController  {
        return new ModelAndView("left","model",map);
    }

-    private boolean saveSelectedMusicFolder(HttpServletRequest request) {
-        if (request.getParameter("musicFolderId") == null) {
+    private boolean saveSelectedMusicFolder(HttpServletRequest request) throws Exception {
+        Integer musicFolderId = ServletRequestUtils.getIntParameter(request, "musicFolderId");
+        if (musicFolderId == null) {
            return false;
        }
-        int musicFolderId = Integer.parseInt(request.getParameter("musicFolderId"));
-
        // Note: UserSettings.setChanged() is intentionally not called. This would break browser caching
        // of the left frame.
        UserSettings settings = settingsService.getUserSettings(securityService.getCurrentUsername(request));
@@ -29,6 +29,7 @@ import org.apache.commons.lang.StringUtils;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Controller;
 import org.springframework.ui.Model;
+import org.springframework.web.bind.ServletRequestUtils;
 import org.springframework.web.bind.annotation.ModelAttribute;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
@@ -70,7 +71,7 @@ public class PlayerSettingsController  {
        User user = securityService.getCurrentUser(request);
        PlayerSettingsCommand command = new PlayerSettingsCommand();
        Player player = null;
-        String playerId = request.getParameter("id");
+        Integer playerId = ServletRequestUtils.getIntParameter(request, "id");
        if (playerId != null) {
            player = playerService.getPlayerById(playerId);
        } else if (!players.isEmpty()) {
@@ -113,7 +114,7 @@ public class PlayerSettingsController  {
    @RequestMapping(method = RequestMethod.POST)
    protected String doSubmitAction(@ModelAttribute("command") PlayerSettingsCommand command, RedirectAttributes redirectAttributes) throws Exception {
        Player player = playerService.getPlayerById(command.getPlayerId());
-
+        if (player != null) {
            player.setAutoControlEnabled(command.isAutoControlEnabled());
            player.setM3uBomEnabled(command.isM3uBomEnabled());
            player.setDynamicIp(command.isDynamicIp());
@@ -128,6 +129,9 @@ public class PlayerSettingsController  {
            redirectAttributes.addFlashAttribute("settings_reload", true);
            redirectAttributes.addFlashAttribute("settings_toast", true);
            return "redirect:playerSettings.view";
+        } else {
+            return "redirect:notFound";
+        }
    }

    private List<Player> getPlayers(HttpServletRequest request) {
@@ -145,11 +149,11 @@ public class PlayerSettingsController  {
        return authorizedPlayers;
    }

-    private void handleRequestParameters(HttpServletRequest request) {
+    private void handleRequestParameters(HttpServletRequest request) throws Exception {
        if (request.getParameter("delete") != null) {
-            playerService.removePlayerById(request.getParameter("delete"));
+            playerService.removePlayerById(ServletRequestUtils.getIntParameter(request, "delete"));
        } else if (request.getParameter("clone") != null) {
-            playerService.clonePlayer(request.getParameter("clone"));
+            playerService.clonePlayer(ServletRequestUtils.getIntParameter(request, "clone"));
        }
    }

@@ -94,7 +94,7 @@ public class ShareManagementController  {

    private List<MediaFile> getMediaFiles(HttpServletRequest request) throws Exception {
        Integer id = ServletRequestUtils.getIntParameter(request, "id");
-        String playerId = request.getParameter("player");
+        Integer playerId = ServletRequestUtils.getIntParameter(request, "player");
        Integer playlistId = ServletRequestUtils.getIntParameter(request, "playlist");

        List<MediaFile> result = new ArrayList<>();
@@ -32,6 +32,7 @@ import org.jfree.data.Range;
 import org.jfree.data.time.*;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Controller;
+import org.springframework.web.bind.ServletRequestUtils;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.servlet.ModelAndView;
@@ -62,7 +63,7 @@ public class StatusChartController extends AbstractChartController {
    @RequestMapping(method = RequestMethod.GET)
    public synchronized ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception {
        String type = request.getParameter("type");
-        int index = Integer.parseInt(request.getParameter("index"));
+        int index = ServletRequestUtils.getIntParameter(request, "index");

        List<TransferStatus> statuses = Collections.emptyList();
        if ("stream".equals(type)) {
@@ -1225,7 +1225,7 @@ public class SubsonicRESTController {
            if (minutesAgo < 60) {
                NowPlayingEntry entry = new NowPlayingEntry();
                entry.setUsername(username);
-                entry.setPlayerId(Integer.parseInt(player.getId()));
+                entry.setPlayerId(player.getId());
                entry.setPlayerName(player.getName());
                entry.setMinutesAgo((int) minutesAgo);
                result.getEntry().add(createJaxbChild(entry, player, mediaFile, username));
@@ -2399,7 +2399,7 @@ public class SubsonicRESTController {
        }

        // Return the player ID.
-        return !players.isEmpty() ? players.get(0).getId() : null;
+        return !players.isEmpty() ? String.valueOf(players.get(0).getId()) : null;
    }

    private Locale getUserLocale(HttpServletRequest request) {
@@ -69,7 +69,7 @@ public class VideoPlayerController {
        mediaFileService.populateStarredDate(file, user.getUsername());

        Integer duration = file.getDurationSeconds();
-        String playerId = playerService.getPlayer(request, response).getId();
+        Integer playerId = playerService.getPlayer(request, response).getId();
        String url = NetworkService.getBaseUrl(request);
        String streamUrl = url + "stream?id=" + file.getId() + "&player=" + playerId + "&format=mp4";
        String coverArtUrl = url + "coverArt.view?id=" + file.getId();
@@ -20,7 +20,6 @@
 package org.airsonic.player.dao;

 import org.airsonic.player.domain.*;
-import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.jdbc.core.RowMapper;
@@ -45,7 +44,7 @@ public class PlayerDao extends AbstractDao {
    private static final String QUERY_COLUMNS = "id, " + INSERT_COLUMNS;

    private PlayerRowMapper rowMapper = new PlayerRowMapper();
-    private Map<String, PlayQueue> playlists = Collections.synchronizedMap(new HashMap<String, PlayQueue>());
+    private Map<Integer, PlayQueue> playlists = Collections.synchronizedMap(new HashMap<Integer, PlayQueue>());

    /**
     * Returns all players.
@@ -81,14 +80,10 @@ public class PlayerDao extends AbstractDao {
     * @param id The unique player ID.
     * @return The player with the given ID, or <code>null</code> if no such player exists.
     */
-    public Player getPlayerById(String id) {
-        if (StringUtils.isBlank(id)) {
-            return null;
-        } else {
+    public Player getPlayerById(int id) {
        String sql = "select " + QUERY_COLUMNS + " from player where id=?";
        return queryOne(sql, rowMapper, id);
    }
-    }

    /**
     * Creates a new player.
@@ -102,7 +97,7 @@ public class PlayerDao extends AbstractDao {
            existingMax = 0;
        }
        int id = existingMax + 1;
-        player.setId(String.valueOf(id));
+        player.setId(id);
        String sql = "insert into player (" + QUERY_COLUMNS + ") values (" + questionMarks(QUERY_COLUMNS) + ")";
        update(sql, player.getId(), player.getName(), player.getType(), player.getUsername(),
               player.getIpAddress(), player.isAutoControlEnabled(), player.isM3uBomEnabled(),
@@ -119,7 +114,7 @@ public class PlayerDao extends AbstractDao {
     *
     * @param id The player ID.
     */
-    public void deletePlayer(String id) {
+    public void deletePlayer(Integer id) {
        String sql = "delete from player where id=?";
        update(sql, id);
        playlists.remove(id);
@@ -181,7 +176,7 @@ public class PlayerDao extends AbstractDao {
        public Player mapRow(ResultSet rs, int rowNum) throws SQLException {
            Player player = new Player();
            int col = 1;
-            player.setId(rs.getString(col++));
+            player.setId(rs.getInt(col++));
            player.setName(rs.getString(col++));
            player.setType(rs.getString(col++));
            player.setUsername(rs.getString(col++));
@@ -59,7 +59,7 @@ public class TranscodingDao extends AbstractDao {
     * @param playerId The player ID.
     * @return All active transcodings for the player.
     */
-    public List<Transcoding> getTranscodingsForPlayer(String playerId) {
+    public List<Transcoding> getTranscodingsForPlayer(Integer playerId) {
        String sql = "select " + QUERY_COLUMNS + " from transcoding2, player_transcoding2 " +
                     "where player_transcoding2.player_id = ? " +
                     "and   player_transcoding2.transcoding_id = transcoding2.id";
@@ -72,7 +72,7 @@ public class TranscodingDao extends AbstractDao {
     * @param playerId       The player ID.
     * @param transcodingIds ID's of the active transcodings.
     */
-    public void setTranscodingsForPlayer(String playerId, int[] transcodingIds) {
+    public void setTranscodingsForPlayer(Integer playerId, int[] transcodingIds) {
        update("delete from player_transcoding2 where player_id = ?", playerId);
        String sql = "insert into player_transcoding2(player_id, transcoding_id) values (?, ?)";
        for (int transcodingId : transcodingIds) {
@@ -31,7 +31,7 @@ import java.util.Date;
 */
 public class Player {

-    private String id;
+    private Integer id;
    private String name;
    private PlayerTechnology technology = PlayerTechnology.WEB;
    private String clientId;
@@ -51,7 +51,7 @@ public class Player {
     *
     * @return The player ID.
     */
-    public String getId() {
+    public Integer getId() {
        return id;
    }

@@ -60,7 +60,7 @@ public class Player {
     *
     * @param id The player ID.
     */
-    public void setId(String id) {
+    public void setId(Integer id) {
        this.id = id;
    }

@@ -74,7 +74,7 @@ public class Transcoding {
        return id;
    }

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

@@ -39,7 +39,7 @@ public class JukeboxJavaService {


    private TransferStatus status;
-    private Map<String, com.github.biconou.AudioPlayer.api.Player> activeAudioPlayers = new Hashtable<>();
+    private Map<Integer, com.github.biconou.AudioPlayer.api.Player> activeAudioPlayers = new Hashtable<>();
    private Map<String, List<com.github.biconou.AudioPlayer.api.Player>> activeAudioPlayersPerMixer = new Hashtable<>();
    private final static String DEFAULT_MIXER_ENTRY_KEY = "_default";

@@ -30,6 +30,7 @@ import org.apache.commons.lang.StringUtils;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.DependsOn;
 import org.springframework.stereotype.Service;
+import org.springframework.web.bind.ServletRequestUtils;

 import javax.annotation.PostConstruct;
 import javax.servlet.http.Cookie;
@@ -70,7 +71,7 @@ public class PlayerService {
    /**
     * Equivalent to <code>getPlayer(request, response, true)</code> .
     */
-    public Player getPlayer(HttpServletRequest request, HttpServletResponse response) {
+    public Player getPlayer(HttpServletRequest request, HttpServletResponse response) throws Exception {
        return getPlayer(request, response, true, false);
    }

@@ -85,14 +86,14 @@ public class PlayerService {
     * @return The player associated with the given HTTP request.
     */
    public synchronized Player getPlayer(HttpServletRequest request, HttpServletResponse response,
-                                         boolean remoteControlEnabled, boolean isStreamRequest) {
+                                         boolean remoteControlEnabled, boolean isStreamRequest) throws Exception {

        // Find by 'player' request parameter.
-        Player player = getPlayerById(request.getParameter("player"));
+        Player player = getPlayerById(ServletRequestUtils.getIntParameter(request, "player"));

        // Find in session context.
        if (player == null && remoteControlEnabled) {
-            String playerId = (String) request.getSession().getAttribute("player");
+            Integer playerId = (Integer) request.getSession().getAttribute("player");
            if (playerId != null) {
                player = getPlayerById(playerId);
            }
@@ -148,7 +149,7 @@ public class PlayerService {
        // Set cookie in response.
        if (response != null) {
            String cookieName = COOKIE_NAME + "-" + StringUtil.utf8HexEncode(username);
-            Cookie cookie = new Cookie(cookieName, player.getId());
+            Cookie cookie = new Cookie(cookieName, String.valueOf(player.getId()));
            cookie.setMaxAge(COOKIE_EXPIRY);
            String path = request.getContextPath();
            if (StringUtils.isEmpty(path)) {
@@ -181,8 +182,8 @@ public class PlayerService {
     * @param id The unique player ID.
     * @return The player with the given ID, or <code>null</code> if no such player exists.
     */
-    public Player getPlayerById(String id) {
-        if (StringUtils.isBlank(id)) {
+    public Player getPlayerById(Integer id) {
+        if (id == null) {
            return null;
        } else {
            return playerDao.getPlayerById(id);
@@ -234,7 +235,7 @@ public class PlayerService {
     * @param username The name of the current user.
     * @return The player ID embedded in the cookie, or <code>null</code> if cookie is not present.
     */
-    private String getPlayerIdFromCookie(HttpServletRequest request, String username) {
+    private Integer getPlayerIdFromCookie(HttpServletRequest request, String username) {
        Cookie[] cookies = request.getCookies();
        if (cookies == null) {
            return null;
@@ -242,7 +243,11 @@ public class PlayerService {
        String cookieName = COOKIE_NAME + "-" + StringUtil.utf8HexEncode(username);
        for (Cookie cookie : cookies) {
            if (cookieName.equals(cookie.getName())) {
-                return StringUtils.trimToNull(cookie.getValue());
+                try {
+                    return Integer.valueOf(cookie.getValue());
+                } catch (NumberFormatException e) {
+                    return null;
+                }
            }
        }
        return null;
@@ -274,7 +279,7 @@ public class PlayerService {
     *
     * @param id The unique player ID.
     */
-    public synchronized void removePlayerById(String id) {
+    public synchronized void removePlayerById(int id) {
        playerDao.deletePlayer(id);
    }

@@ -284,7 +289,7 @@ public class PlayerService {
     * @param playerId The ID of the player to clone.
     * @return The cloned player.
     */
-    public Player clonePlayer(String playerId) {
+    public Player clonePlayer(int playerId) {
        Player player = getPlayerById(playerId);
        if (player.getName() != null) {
            player.setName(player.getName() + " (copy)");
@@ -309,9 +314,10 @@ public class PlayerService {
                defaultActiveTranscodings.add(transcoding);
            }
        }
-
+        if (player != null) {
            transcodingService.setTranscodingsForPlayer(player, defaultActiveTranscodings);
        }
+    }

    /**
     * Returns a player associated to the special "guest" user, creating it if necessary.
@@ -49,7 +49,7 @@ public class StatusService {
    private final List<PlayStatus> remotePlays = new ArrayList<PlayStatus>();

    // Maps from player ID to latest inactive stream status.
-    private final Map<String, TransferStatus> inactiveStreamStatuses = new LinkedHashMap<String, TransferStatus>();
+    private final Map<Integer, TransferStatus> inactiveStreamStatuses = new LinkedHashMap<Integer, TransferStatus>();

    public synchronized TransferStatus createStreamStatus(Player player) {
        // Reuse existing status, if possible.
@@ -74,12 +74,12 @@ public class StatusService {
        List<TransferStatus> result = new ArrayList<TransferStatus>(streamStatuses);

        // Add inactive status for those players that have no active status.
-        Set<String> activePlayers = new HashSet<String>();
+        Set<Integer> activePlayers = new HashSet<Integer>();
        for (TransferStatus status : streamStatuses) {
            activePlayers.add(status.getPlayer().getId());
        }

-        for (Map.Entry<String, TransferStatus> entry : inactiveStreamStatuses.entrySet()) {
+        for (Map.Entry<Integer, TransferStatus> entry : inactiveStreamStatuses.entrySet()) {
            if (!activePlayers.contains(entry.getKey())) {
                result.add(entry.getValue());
            }
@@ -142,7 +142,7 @@ public class StatusService {
    }

    public synchronized List<PlayStatus> getPlayStatuses() {
-        Map<String, PlayStatus> result = new LinkedHashMap<String, PlayStatus>();
+        Map<Integer, PlayStatus> result = new LinkedHashMap<Integer, PlayStatus>();
        for (PlayStatus remotePlay : remotePlays) {
            if (!remotePlay.isExpired()) {
                result.put(remotePlay.getPlayer().getId(), remotePlay);
@@ -118,12 +118,14 @@ public class TranscodingService {
        // Activate this transcoding for all players?
        if (transcoding.isDefaultActive()) {
            for (Player player : playerService.getAllPlayers()) {
+                if (player != null) {
                    List<Transcoding> transcodings = getTranscodingsForPlayer(player);
                    transcodings.add(transcoding);
                    setTranscodingsForPlayer(player, transcodings);
                }
            }
        }
+    }

    /**
     * Deletes the transcoding with the given ID.
@@ -65,25 +65,25 @@ public class PlayerDaoTestCase extends DaoTestCaseBean2 {
        Player player = new Player();

        playerDao.createPlayer(player);
-        assertEquals("Wrong ID", "1", player.getId());
+        assertEquals("Wrong ID", (Integer)1, player.getId());
        assertEquals("Wrong number of players.", 1, playerDao.getAllPlayers().size());

        playerDao.createPlayer(player);
-        assertEquals("Wrong ID", "2", player.getId());
+        assertEquals("Wrong ID", (Integer)2, player.getId());
        assertEquals("Wrong number of players.", 2, playerDao.getAllPlayers().size());

        playerDao.createPlayer(player);
-        assertEquals("Wrong ID", "3", player.getId());
+        assertEquals("Wrong ID", (Integer)3, player.getId());
        assertEquals("Wrong number of players.", 3, playerDao.getAllPlayers().size());

-        playerDao.deletePlayer("3");
+        playerDao.deletePlayer(3);
        playerDao.createPlayer(player);
-        assertEquals("Wrong ID", "3", player.getId());
+        assertEquals("Wrong ID", (Integer)3, player.getId());
        assertEquals("Wrong number of players.", 3, playerDao.getAllPlayers().size());

-        playerDao.deletePlayer("2");
+        playerDao.deletePlayer(2);
        playerDao.createPlayer(player);
-        assertEquals("Wrong ID", "4", player.getId());
+        assertEquals("Wrong ID", (Integer)4, player.getId());
        assertEquals("Wrong number of players.", 3, playerDao.getAllPlayers().size());
    }

@@ -153,10 +153,10 @@ public class PlayerDaoTestCase extends DaoTestCaseBean2 {
        playerDao.createPlayer(new Player());
        assertEquals("Wrong number of players.", 2, playerDao.getAllPlayers().size());

-        playerDao.deletePlayer("1");
+        playerDao.deletePlayer(1);
        assertEquals("Wrong number of players.", 1, playerDao.getAllPlayers().size());

-        playerDao.deletePlayer("2");
+        playerDao.deletePlayer(2);
        assertEquals("Wrong number of players.", 0, playerDao.getAllPlayers().size());
    }

@@ -40,7 +40,7 @@ public class StatusServiceTestCase extends TestCase {
        super.setUp();
        service = new StatusService();
        player1 = new Player();
-        player1.setId("1");
+        player1.setId(1);
    }

    public void testSimpleAddRemove() {