fix: probe the local server address without switching onto it

app/src/main/java/com/eddyizm/tempus/App.java

@@ -11,6 +11,7 @@
 
 import com.eddyizm.tempus.github.Github;
 import com.eddyizm.tempus.helper.ThemeHelper;
+import com.eddyizm.tempus.model.Server;
 import com.eddyizm.tempus.subsonic.Subsonic;
 import com.eddyizm.tempus.subsonic.SubsonicPreferences;
 import com.eddyizm.tempus.ui.crash.CrashActivity;
@@ -80,46 +81,41 @@ public static Subsonic getSubsonicClientInstance(boolean override) {
         }
         return subsonic;
     }
-
-    public static Subsonic getSubsonicPublicClientInstance(boolean override) {
 
-        /*
-        If I do the shortcut that the IDE suggests:
-            SubsonicPreferences preferences = getSubsonicPreferences1();
-        During the chain of calls it will run the following:
-            String server = Preferences.getInUseServerAddress();
-        Which could return Local URL, causing issues like generating public shares with Local URL
-
-        To prevent this I just replicated the entire chain of functions here,
-        if you need a call to Subsonic using the Server (Public) URL use this function.
-         */
+    // Pinned to one address instead of the one in use, so an address can be reached without the
+    // app being moved onto it first.
+    public static Subsonic getSubsonicClientInstance(String serverAddress) {
+        return buildSubsonicClient(
+                serverAddress,
+                Preferences.getUser(),
+                Preferences.getPassword(),
+                Preferences.getToken(),
+                Preferences.getSalt(),
+                Preferences.isLowScurity()
+        );
+    }
 
-        String server = Preferences.getServer();
-        String username = Preferences.getUser();
-        String password = Preferences.getPassword();
-        String token = Preferences.getToken();
-        String salt = Preferences.getSalt();
-        boolean isLowSecurity = Preferences.isLowScurity();
+    // For a server the app is not signed in to, so it can be reached before anything about it is
+    // written to the preferences.
+    public static Subsonic getSubsonicClientInstance(Server server) {
+        return buildSubsonicClient(
+                server.getAddress(),
+                server.getUsername(),
+                server.getPassword(),
+                null,
+                null,
+                server.isLowSecurity()
+        );
+    }
 
+    private static Subsonic buildSubsonicClient(String serverAddress, String username,
+                                                String password, String token, String salt,
+                                                boolean isLowSecurity) {
         SubsonicPreferences preferences = new SubsonicPreferences();
-        preferences.setServerUrl(server);
+        preferences.setServerUrl(serverAddress);
         preferences.setUsername(username);
         preferences.setAuthentication(password, token, salt, isLowSecurity);
 
-        if (subsonic == null || override) {
-
-            if (preferences.getAuthentication() != null) {
-                if (preferences.getAuthentication().getPassword() != null)
-                    Preferences.setPassword(preferences.getAuthentication().getPassword());
-                if (preferences.getAuthentication().getToken() != null)
-                    Preferences.setToken(preferences.getAuthentication().getToken());
-                if (preferences.getAuthentication().getSalt() != null)
-                    Preferences.setSalt(preferences.getAuthentication().getSalt());
-            }
-
-
-        }
-
         return new Subsonic(preferences);
     }
 

app/src/main/java/com/eddyizm/tempus/interfaces/SystemCallback.java

@@ -6,4 +6,10 @@
 public interface SystemCallback {
     default void onError(Exception exception) {}
     default void onSuccess(String password, String token, String salt) {}
+
+    // The request failed at the transport. Defaults to onError, so a caller that does not care
+    // which of the two happened behaves as it did before.
+    default void onNetworkFailure(Exception exception) {
+        onError(exception);
+    }
 }

app/src/main/java/com/eddyizm/tempus/repository/SharingRepository.java

@@ -6,6 +6,7 @@
 import com.eddyizm.tempus.App;
 import com.eddyizm.tempus.subsonic.base.ApiResponse;
 import com.eddyizm.tempus.subsonic.models.Share;
+import com.eddyizm.tempus.util.Preferences;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -41,7 +42,7 @@ public void onFailure(@NonNull Call<ApiResponse> call, @NonNull Throwable t) {
     public MutableLiveData<Share> createShare(String id, String description, Long expires) {
         MutableLiveData<Share> share = new MutableLiveData<>();
 
-        App.getSubsonicPublicClientInstance(false)
+        App.getSubsonicClientInstance(Preferences.getServer())
                 .getSharingClient()
                 .createShare(id, description, expires)
                 .enqueue(new Callback<ApiResponse>() {
@@ -64,7 +65,7 @@ public void onFailure(@NonNull Call<ApiResponse> call, @NonNull Throwable t) {
     }
 
     public void updateShare(String id, String description, Long expires) {
-        App.getSubsonicPublicClientInstance(false)
+        App.getSubsonicClientInstance(Preferences.getServer())
                 .getSharingClient()
                 .updateShare(id, description, expires)
                 .enqueue(new Callback<ApiResponse>() {

app/src/main/java/com/eddyizm/tempus/repository/SystemRepository.java

@@ -8,11 +8,14 @@
 import com.eddyizm.tempus.App;
 import com.eddyizm.tempus.github.models.LatestRelease;
 import com.eddyizm.tempus.interfaces.SystemCallback;
+import com.eddyizm.tempus.subsonic.Subsonic;
 import com.eddyizm.tempus.subsonic.base.ApiResponse;
 import com.eddyizm.tempus.subsonic.models.OpenSubsonicExtension;
 import com.eddyizm.tempus.subsonic.models.ResponseStatus;
 import com.eddyizm.tempus.subsonic.models.SubsonicResponse;
+import com.eddyizm.tempus.util.Preferences;
 
+import java.io.IOException;
 import java.util.List;
 
 import retrofit2.Call;
@@ -21,7 +24,12 @@
 
 public class SystemRepository {
     public void checkUserCredential(SystemCallback callback) {
-        App.getSubsonicClientInstance(false)
+        checkUserCredential(App.getSubsonicClientInstance(false), callback);
+    }
+
+    // Takes the client so a server can be reached before anything about it is saved.
+    public void checkUserCredential(Subsonic client, SystemCallback callback) {
+        client
                 .getSystemClient()
                 .ping()
                 .enqueue(new Callback<ApiResponse>() {
@@ -46,15 +54,30 @@ public void onResponse(@NonNull Call<ApiResponse> call, @NonNull retrofit2.Respo
 
                     @Override
                     public void onFailure(@NonNull Call<ApiResponse> call, @NonNull Throwable t) {
-                        callback.onError(new Exception(t.getMessage()));
+                        // Retrofit sends a body it cannot read here too, so a server answering
+                        // something that is not Subsonic would otherwise read as a network fault.
+                        Exception failure = new Exception(t.getMessage());
+                        if (t instanceof IOException) {
+                            callback.onNetworkFailure(failure);
+                        } else {
+                            callback.onError(failure);
+                        }
                     }
                 });
     }
 
     public MutableLiveData<SubsonicResponse> ping() {
+        return ping(App.getSubsonicClientInstance(false));
+    }
+
+    public MutableLiveData<SubsonicResponse> pingLocalAddress() {
+        return ping(App.getSubsonicClientInstance(Preferences.getLocalAddress()));
+    }
+
+    private MutableLiveData<SubsonicResponse> ping(Subsonic client) {
         MutableLiveData<SubsonicResponse> pingResult = new MutableLiveData<>();
 
-        App.getSubsonicClientInstance(false)
+        client
                 .getSystemClient()
                 .ping()
                 .enqueue(new Callback<ApiResponse>() {

app/src/main/java/com/eddyizm/tempus/service/BaseMediaService.kt

@@ -238,7 +238,14 @@ open class BaseMediaService : MediaLibraryService(), MediaManager.QueueTarget {
 
         // Map off the main thread: mapMediaItems does a blocking lookup per song, so a
         // large saved queue froze the UI on launch (#600).
+        //
+        // Every URL carries the address in force when the mapping ran, so the address is checked
+        // again on the main thread, immediately before the install, and a move starts it over.
         Thread {
+            // Nothing needs the queue until playback starts, so it waits for a tested address.
+            ConnectionUtil.awaitPingsAnswered()
+
+            val addressWhenMapped = Preferences.getInUseServerAddress()
             val queueRepository = QueueRepository()
             val storedQueue = queueRepository.media
             if (storedQueue.isNullOrEmpty()) return@Thread
@@ -268,6 +275,10 @@ open class BaseMediaService : MediaLibraryService(), MediaManager.QueueTarget {
                 // the mediaItemCount check below cannot detect a released player, so bail first.
                 if (serviceDestroyed) return@post
                 if (player.mediaItemCount > 0) return@post
+                if (addressWhenMapped != Preferences.getInUseServerAddress()) {
+                    restorePlayerFromQueue(player)
+                    return@post
+                }
                 player.setMediaItems(mediaItems, lastIndex, lastPosition)
                 player.prepare()
                 updateWidget(player)

app/src/main/java/com/eddyizm/tempus/service/MediaManager.java

@@ -31,6 +31,7 @@
 import com.eddyizm.tempus.subsonic.models.PodcastEpisode;
 import com.eddyizm.tempus.util.Constants;
 import com.eddyizm.tempus.util.MappingUtil;
+import com.eddyizm.tempus.util.ConnectionUtil;
 import com.eddyizm.tempus.util.Preferences;
 import com.eddyizm.tempus.viewmodel.PlaybackViewModel;
 import com.google.common.util.concurrent.FutureCallback;
@@ -207,6 +208,10 @@ public static void check(ListenableFuture<MediaBrowser> mediaBrowserListenableFu
                 try {
                     if (mediaBrowserListenableFuture.isDone()) {
                         if (mediaBrowserListenableFuture.get().getMediaItemCount() < 1) {
+                            // The service restores this same queue and waits for the pings first,
+                            // and reaching a browser means that restore is already in flight.
+                            if (ConnectionUtil.pingsOutstanding()) return;
+
                             List<Child> media = getQueueRepository().getMedia();
                             if (media != null && media.size() >= 1) {
                                 init(mediaBrowserListenableFuture, media);

app/src/main/java/com/eddyizm/tempus/subsonic/api/system/SystemClient.java

@@ -5,6 +5,7 @@
 import com.eddyizm.tempus.subsonic.RetrofitClient;
 import com.eddyizm.tempus.subsonic.Subsonic;
 import com.eddyizm.tempus.subsonic.base.ApiResponse;
+import com.eddyizm.tempus.util.ConnectionUtil;
 import com.eddyizm.tempus.util.Preferences;
 
 import java.util.concurrent.TimeUnit;
@@ -26,7 +27,12 @@ public Call<ApiResponse> ping() {
         Log.d(TAG, "ping()");
         int timeoutSeconds = Preferences.getNetworkPingTimeout();
         Call<ApiResponse> pingCall = systemService.ping(subsonic.getParams());
-        if (Preferences.isInUseServerAddressLocal()) {
+        // Keyed to the address this client points at, since a probe runs on its own client while
+        // the in use address is still the public one.
+        boolean pingingLocalAddress = ConnectionUtil.isUnderAddress(
+                subsonic.getUrl(), Preferences.getLocalAddress());
+
+        if (pingingLocalAddress) {
             pingCall.timeout()
                     .timeout(timeoutSeconds, TimeUnit.SECONDS);
         } else {

app/src/main/java/com/eddyizm/tempus/subsonic/utils/CacheUtil.java

@@ -39,7 +39,9 @@ public CacheUtil(int maxAge, int maxStale) {
     };
 
 
-    private boolean isConnected() {
+    // Public so a caller can tell an unreachable server from a phone with no network at all. The
+    // offline branch above answers a synthetic 504 in that case, which says nothing on its own.
+    public static boolean isConnected() {
         ConnectivityManager connectivityManager = (ConnectivityManager) App.getContext().getSystemService(Context.CONNECTIVITY_SERVICE);
         if (connectivityManager == null) {
             return false;