Merge pull request #309 from nitehu/fix/new_server_id

Fixed issues with new Server Settings

ultrasonic/src/main/kotlin/org/moire/ultrasonic/activity/EditServerActivity.kt

@@ -13,6 +13,7 @@ import com.google.android.material.textfield.TextInputLayout
 import java.io.IOException
 import java.net.MalformedURLException
 import java.net.URL
+import org.koin.android.ext.android.inject
 import org.koin.android.viewmodel.ext.android.viewModel
 import org.moire.ultrasonic.BuildConfig
 import org.moire.ultrasonic.R
@@ -20,7 +21,9 @@ import org.moire.ultrasonic.api.subsonic.SubsonicAPIClient
 import org.moire.ultrasonic.api.subsonic.SubsonicAPIVersions
 import org.moire.ultrasonic.api.subsonic.SubsonicClientConfiguration
 import org.moire.ultrasonic.api.subsonic.response.SubsonicResponse
+import org.moire.ultrasonic.data.ActiveServerProvider
 import org.moire.ultrasonic.data.ServerSetting
+import org.moire.ultrasonic.service.MusicServiceFactory
 import org.moire.ultrasonic.service.SubsonicRESTException
 import org.moire.ultrasonic.util.Constants
 import org.moire.ultrasonic.util.ErrorDialog
@@ -41,6 +44,8 @@ internal class EditServerActivity : AppCompatActivity() {
     }
 
     private val serverSettingsModel: ServerSettingsModel by viewModel()
+    private val activeServerProvider: ActiveServerProvider by inject()
+
     private var currentServerSetting: ServerSetting? = null
 
     private var serverNameEditText: TextInputLayout? = null
@@ -90,6 +95,13 @@ internal class EditServerActivity : AppCompatActivity() {
                 if (currentServerSetting != null) {
                     if (getFields()) {
                         serverSettingsModel.updateItem(currentServerSetting)
+                        // Apply modifications if the current server was modified
+                        if (
+                            activeServerProvider.getActiveServer().id ==
+                            currentServerSetting!!.id
+                        ) {
+                            MusicServiceFactory.resetMusicService()
+                        }
                         finish()
                     }
                 }

ultrasonic/src/main/kotlin/org/moire/ultrasonic/activity/ServerRowAdapter.kt

@@ -53,7 +53,7 @@ internal class ServerRowAdapter(
     }
 
     override fun getCount(): Int {
-        return if (manageMode) data.size - 1 else data.size
+        return if (manageMode) data.size else data.size + 1
     }
 
     override fun getItem(position: Int): Any {
@@ -81,8 +81,15 @@ internal class ServerRowAdapter(
         val image = vi?.findViewById<ImageView>(R.id.server_image)
         val serverMenu = vi?.findViewById<ImageButton>(R.id.server_menu)
 
-        text?.text = data.single { setting -> setting.index == index }.name
+        if (index == 0) {
-        description?.text = data.single { setting -> setting.index == index }.url
+            text?.text = context.getString(R.string.main_offline)
+            description?.text = ""
+        } else {
+            val setting = data.singleOrNull { t -> t.index == index }
+            text?.text = setting?.name ?: ""
+            description?.text = setting?.url ?: ""
+            if (setting == null) serverMenu?.visibility = View.INVISIBLE
+        }
 
         // Provide icons for the row
         if (index == 0) {

ultrasonic/src/main/kotlin/org/moire/ultrasonic/activity/ServerSelectorActivity.kt

@@ -144,8 +144,9 @@ internal class ServerSelectorActivity : AppCompatActivity() {
                 if (activeServerProvider.getActiveServer().index != index) {
                     service.clearIncomplete()
                     activeServerProvider.setActiveServerByIndex(index)
+                    service.isJukeboxEnabled =
+                        activeServerProvider.getActiveServer().jukeboxByDefault
                 }
-                service.isJukeboxEnabled = activeServerProvider.getActiveServer().jukeboxByDefault
             }
         }
         Log.i(TAG, "Active server was set to: $index")

ultrasonic/src/main/kotlin/org/moire/ultrasonic/activity/ServerSettingsModel.kt

@@ -5,12 +5,10 @@ import android.content.SharedPreferences
 import android.preference.PreferenceManager
 import android.util.Log
 import androidx.lifecycle.LiveData
-import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
-import org.moire.ultrasonic.R
 import org.moire.ultrasonic.data.ActiveServerProvider
 import org.moire.ultrasonic.data.ServerSetting
 import org.moire.ultrasonic.data.ServerSettingDao
@@ -23,10 +21,10 @@ class ServerSettingsModel(
     private val activeServerProvider: ActiveServerProvider,
     private val context: Context
 ) : ViewModel() {
-    private var serverList: MutableLiveData<List<ServerSetting>> = MutableLiveData()
 
     companion object {
         private val TAG = ServerSettingsModel::class.simpleName
+        private const val PREFERENCES_KEY_SERVER_MIGRATED = "serverMigrated"
         // These constants were removed from Constants.java as they are deprecated and only used here
         private const val PREFERENCES_KEY_JUKEBOX_BY_DEFAULT = "jukeboxEnabled"
         private const val PREFERENCES_KEY_SERVER_NAME = "serverName"
@@ -84,13 +82,13 @@ class ServerSettingsModel(
      * This function is asynchronous, uses LiveData to provide the Setting.
      */
     fun getServerList(): LiveData<List<ServerSetting>> {
-        viewModelScope.launch {
+        // This check should run before returning any result
-            val dbServerList = repository.loadAllServerSettings().toMutableList()
+        runBlocking {
-
+            if (areIndexesMissing()) {
-            dbServerList.add(0, ServerSetting(context.getString(R.string.main_offline), ""))
+                reindexSettings()
-            serverList.value = dbServerList
             }
-        return serverList
+        }
+        return repository.loadAllServerSettings()
     }
 
     /**
@@ -98,55 +96,47 @@ class ServerSettingsModel(
      * This function is asynchronous, uses LiveData to provide the Setting.
      */
     fun getServerSetting(index: Int): LiveData<ServerSetting?> {
-        val result = MutableLiveData<ServerSetting?>()
+        return repository.getLiveServerSettingByIndex(index)
-        viewModelScope.launch {
-            val dbServer = repository.findByIndex(index)
-            result.value = dbServer
-            Log.d(TAG, "getServerSetting($index) returning $dbServer")
-        }
-        return result
     }
 
     /**
      * Moves a Setting up in the Server List by decreasing its index
      */
     fun moveItemUp(index: Int) {
-        if (index == 1) return
+        if (index <= 1) return
-
-        val itemToBeMoved = serverList.value?.single { setting -> setting.index == index }
-        val previousItem = serverList.value?.single { setting -> setting.index == index - 1 }
-
-        itemToBeMoved?.index = previousItem!!.index
-        previousItem.index = index
 
         viewModelScope.launch {
-            repository.update(itemToBeMoved!!, previousItem)
+            val itemToBeMoved = repository.findByIndex(index)
-        }
+            val previousItem = repository.findByIndex(index - 1)
 
+            if (itemToBeMoved != null && previousItem != null) {
+                itemToBeMoved.index = previousItem.index
+                previousItem.index = index
+
+                repository.update(itemToBeMoved, previousItem)
                 activeServerProvider.invalidateCache()
-        // Notify the observers of the changed values
+            }
-        serverList.value = serverList.value
+        }
     }
 
     /**
      * Moves a Setting down in the Server List by increasing its index
      */
     fun moveItemDown(index: Int) {
-        if (index == (serverList.value!!.size - 1)) return
+        viewModelScope.launch {
+            if (index < repository.getMaxIndex() ?: 0) {
+                val itemToBeMoved = repository.findByIndex(index)
+                val nextItem = repository.findByIndex(index + 1)
 
-        val itemToBeMoved = serverList.value?.single { setting -> setting.index == index }
+                if (itemToBeMoved != null && nextItem != null) {
-        val nextItem = serverList.value?.single { setting -> setting.index == index + 1 }
+                    itemToBeMoved.index = nextItem.index
-
-        itemToBeMoved?.index = nextItem!!.index
                     nextItem.index = index
 
-        viewModelScope.launch {
+                    repository.update(itemToBeMoved, nextItem)
-            repository.update(itemToBeMoved!!, nextItem)
-        }
-
                     activeServerProvider.invalidateCache()
-        // Notify the observers of the changed values
+                }
-        serverList.value = serverList.value
+            }
+        }
     }
 
     /**
@@ -155,24 +145,15 @@ class ServerSettingsModel(
     fun deleteItem(index: Int) {
         if (index == 0) return
 
-        val newList = serverList.value!!.toMutableList()
-        val itemToBeDeleted = newList.single { setting -> setting.index == index }
-        newList.remove(itemToBeDeleted)
-
-        for (x in index + 1 until newList.size + 1) {
-            newList.single { setting -> setting.index == x }.index--
-        }
-
         viewModelScope.launch {
+            val itemToBeDeleted = repository.findByIndex(index)
+            if (itemToBeDeleted != null) {
                 repository.delete(itemToBeDeleted)
-            for (x in index until newList.size) {
-                repository.update(newList.single { setting -> setting.index == x })
-            }
-        }
-
-        activeServerProvider.invalidateCache()
-        serverList.value = newList
                 Log.d(TAG, "deleteItem deleted index: $index")
+                reindexSettings()
+                activeServerProvider.invalidateCache()
+            }
+        }
     }
 
     /**
@@ -196,7 +177,7 @@ class ServerSettingsModel(
 
         viewModelScope.launch {
             serverSetting.index = (repository.count() ?: 0) + 1
-            serverSetting.id = serverSetting.index
+            serverSetting.id = (repository.getMaxId() ?: 0) + 1
             repository.insert(serverSetting)
             Log.d(TAG, "saveNewItem saved server setting: $serverSetting")
         }
@@ -212,8 +193,10 @@ class ServerSettingsModel(
     ): ServerSetting? {
         val url = settings.getString(PREFERENCES_KEY_SERVER_URL + preferenceId, "")
         val userName = settings.getString(PREFERENCES_KEY_USERNAME + preferenceId, "")
+        val isMigrated = settings.getBoolean(PREFERENCES_KEY_SERVER_MIGRATED + preferenceId, false)
 
-        if (url.isNullOrEmpty() || userName.isNullOrEmpty()) return null
+        if (url.isNullOrEmpty() || userName.isNullOrEmpty() || isMigrated) return null
+        setServerMigrated(settings, preferenceId)
 
         return ServerSetting(
             preferenceId,
@@ -231,4 +214,47 @@ class ServerSettingsModel(
             settings.getString(PREFERENCES_KEY_MUSIC_FOLDER_ID + preferenceId, null)
         )
     }
+
+    /**
+     * Checks if there are any missing indexes in the ServerSetting list
+     * For displaying the Server Settings in a ListView, it is mandatory that their indexes
+     * are'nt missing. Ideally the indexes are continuous, but some circumstances (e.g.
+     * concurrency or migration errors) may get them out of order.
+     * This would make the List Adapter crash, so it is best to prepare and check the list.
+     */
+    private suspend fun areIndexesMissing(): Boolean {
+        for (i in 1 until getMaximumIndexToCheck() + 1) {
+            if (repository.findByIndex(i) == null) return true
+        }
+        return false
+    }
+
+    /**
+     * This function updates all the Server Settings in the DB so their indexing is continuous.
+     */
+    private suspend fun reindexSettings() {
+        var newIndex = 1
+        for (i in 1 until getMaximumIndexToCheck() + 1) {
+            val setting = repository.findByIndex(i)
+            if (setting != null) {
+                setting.index = newIndex
+                newIndex++
+                repository.update(setting)
+                Log.d(TAG, "reindexSettings saved $setting")
+            }
+        }
+    }
+
+    private suspend fun getMaximumIndexToCheck(): Int {
+        val rowsInDatabase = repository.count() ?: 0
+        val indexesInDatabase = repository.getMaxIndex() ?: 0
+        if (rowsInDatabase > indexesInDatabase) return rowsInDatabase
+        return indexesInDatabase
+    }
+
+    private fun setServerMigrated(settings: SharedPreferences, preferenceId: Int) {
+        val editor = settings.edit()
+        editor.putBoolean(PREFERENCES_KEY_SERVER_MIGRATED + preferenceId, true)
+        editor.apply()
+    }
 }

ultrasonic/src/main/kotlin/org/moire/ultrasonic/data/ActiveServerProvider.kt

@@ -75,7 +75,7 @@ class ActiveServerProvider(
         }
 
         GlobalScope.launch(Dispatchers.IO) {
-            val serverId = repository.findByIndex(index)!!.id
+            val serverId = repository.findByIndex(index)?.id ?: 0
             setActiveServerId(context, serverId)
         }
     }

ultrasonic/src/main/kotlin/org/moire/ultrasonic/data/ServerSettingDao.kt

@@ -1,5 +1,6 @@
 package org.moire.ultrasonic.data
 
+import androidx.lifecycle.LiveData
 import androidx.room.Dao
 import androidx.room.Delete
 import androidx.room.Insert
@@ -35,7 +36,7 @@ interface ServerSettingDao {
      * Loads all Server Settings from the table
      */
     @Query("SELECT * FROM serverSetting")
-    suspend fun loadAllServerSettings(): Array<ServerSetting>
+    fun loadAllServerSettings(): LiveData<List<ServerSetting>>
 
     /**
      * Finds a Server Setting by its unique Id
@@ -49,9 +50,28 @@ interface ServerSettingDao {
     @Query("SELECT * FROM serverSetting WHERE [index] = :index")
     suspend fun findByIndex(index: Int): ServerSetting?
 
+    /**
+     * Finds a Server Setting by its Index in the Select List
+     * @return LiveData of the ServerSetting
+     */
+    @Query("SELECT * FROM serverSetting WHERE [index] = :index")
+    fun getLiveServerSettingByIndex(index: Int): LiveData<ServerSetting?>
+
     /**
      * Retrieves the count of rows in the table
      */
     @Query("SELECT COUNT(*) FROM serverSetting")
     suspend fun count(): Int?
+
+    /**
+     * Retrieves the greatest value of the Id column in the table
+     */
+    @Query("SELECT MAX([id]) FROM serverSetting")
+    suspend fun getMaxId(): Int?
+
+    /**
+     * Retrieves the greatest value of the Index column in the table
+     */
+    @Query("SELECT MAX([index]) FROM serverSetting")
+    suspend fun getMaxIndex(): Int?
 }