model: Use ConfigStore.rename() to avoid recreating tunnels

Rename all of the functions to be in line with setConfig/setState

Signed-off-by: Samuel Holland <samuel@sholland.org>

3 files changed, 51 insertions, 61 deletions

diff --git a/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java b/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java
index aa4321e8..fdb5fc20 100644
--- a/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java
+++ b/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java
@@ -154,7 +154,7 @@ public class TunnelEditorFragment extends BaseFragment {
                             .whenComplete(this::onTunnelCreated);
                 } else if (!selectedTunnel.getName().equals(localName.get())) {
                     Log.d(TAG, "Attempting to rename tunnel to " + localName.get());
-                    selectedTunnel.rename(localName.get())
+                    selectedTunnel.setName(localName.get())
                             .whenComplete(this::onTunnelRenamed);
                 } else if (localConfig != null) {
                     Log.d(TAG, "Attempting to save config of " + selectedTunnel.getName());
@@ -212,16 +212,14 @@ public class TunnelEditorFragment extends BaseFragment {
         }
     }
 
-    private void onTunnelRenamed(final Tunnel tunnel, final Throwable throwable) {
+    private void onTunnelRenamed(final String name, final Throwable throwable) {
         final String message;
         if (throwable == null) {
-            message = getString(R.string.tunnel_rename_success, localTunnel.getName(),
-                    tunnel.getName());
+            message = getString(R.string.tunnel_rename_success, localTunnel.getName(), name);
             Log.d(TAG, message);
-            localTunnel = tunnel;
             // Now save the rest of configuration changes.
-            Log.d(TAG, "Attempting to save config of renamed tunnel " + tunnel.getName());
-            tunnel.setConfig(localConfig).whenComplete(this::onConfigSaved);
+            Log.d(TAG, "Attempting to save config of renamed tunnel " + localTunnel.getName());
+            localTunnel.setConfig(localConfig).whenComplete(this::onConfigSaved);
         } else {
             final String error = ExceptionLoggers.unwrap(throwable).getMessage();
             message = getString(R.string.tunnel_rename_error, error);
diff --git a/app/src/main/java/com/wireguard/android/model/Tunnel.java b/app/src/main/java/com/wireguard/android/model/Tunnel.java
index 024f27a6..09efc70c 100644
--- a/app/src/main/java/com/wireguard/android/model/Tunnel.java
+++ b/app/src/main/java/com/wireguard/android/model/Tunnel.java
@@ -24,8 +24,8 @@ public class Tunnel extends BaseObservable implements Keyed<String> {
     private static final Pattern NAME_PATTERN = Pattern.compile("[a-zA-Z0-9_=+.-]{1,15}");
 
     private final TunnelManager manager;
-    private final String name;
     private Config config;
+    private String name;
     private State state;
     private Statistics statistics;
 
@@ -63,6 +63,7 @@ public class Tunnel extends BaseObservable implements Keyed<String> {
         return name;
     }
 
+    @Bindable
     public String getName() {
         return name;
     }
@@ -97,6 +98,12 @@ public class Tunnel extends BaseObservable implements Keyed<String> {
         return config;
     }
 
+    public String onNameChanged(final String name) {
+        this.name = name;
+        notifyPropertyChanged(BR.name);
+        return name;
+    }
+
     State onStateChanged(final State state) {
         if (state != State.UP)
             onStatisticsChanged(null);
@@ -111,18 +118,18 @@ public class Tunnel extends BaseObservable implements Keyed<String> {
         return statistics;
     }
 
-    public CompletionStage<Tunnel> rename(@NonNull final String name) {
-        if (!name.equals(this.name))
-            return manager.rename(this, name);
-        return CompletableFuture.completedFuture(this);
-    }
-
     public CompletionStage<Config> setConfig(@NonNull final Config config) {
         if (!config.equals(this.config))
             return manager.setTunnelConfig(this, config);
         return CompletableFuture.completedFuture(this.config);
     }
 
+    public CompletionStage<String> setName(@NonNull final String name) {
+        if (!name.equals(this.name))
+            return manager.setTunnelName(this, name);
+        return CompletableFuture.completedFuture(this.name);
+    }
+
     public CompletionStage<State> setState(@NonNull final State state) {
         if (state != this.state)
             return manager.setTunnelState(this, state);
diff --git a/app/src/main/java/com/wireguard/android/model/TunnelManager.java b/app/src/main/java/com/wireguard/android/model/TunnelManager.java
index 4c053e9a..9e03cc39 100644
--- a/app/src/main/java/com/wireguard/android/model/TunnelManager.java
+++ b/app/src/main/java/com/wireguard/android/model/TunnelManager.java
@@ -142,53 +142,6 @@ public final class TunnelManager extends BaseObservable {
             setLastUsedTunnel(tunnels.get(lastUsedName));
     }
 
-    CompletionStage<Tunnel> rename(final Tunnel tunnel, final String name) {
-        if (!Tunnel.isNameValid(name))
-            return CompletableFuture.failedFuture(new IllegalArgumentException("Invalid name"));
-        if (tunnels.containsKey(name)) {
-            final String message = "Tunnel " + name + " already exists";
-            return CompletableFuture.failedFuture(new IllegalArgumentException(message));
-        }
-        final State originalState = tunnel.getState();
-        final boolean wasLastUsed = tunnel == lastUsedTunnel;
-        // Make sure nothing touches the tunnel.
-        if (wasLastUsed)
-            setLastUsedTunnel(null);
-        tunnels.remove(tunnel);
-        return asyncWorker.supplyAsync(() -> {
-            if (originalState == State.UP)
-                backend.setState(tunnel, State.DOWN);
-            final Config newConfig = configStore.create(name, tunnel.getConfig());
-            final Tunnel newTunnel = new Tunnel(this, name, newConfig, State.DOWN);
-            try {
-                if (originalState == State.UP)
-                    backend.setState(newTunnel, originalState);
-                configStore.delete(tunnel.getName());
-            } catch (final Exception e) {
-                // Clean up.
-                configStore.delete(name);
-                if (originalState == State.UP)
-                    backend.setState(tunnel, originalState);
-                // Re-throw the exception to fail the completion.
-                throw e;
-            }
-            return newTunnel;
-        }).whenComplete((newTunnel, e) -> {
-            if (e == null) {
-                // Success, add the new tunnel.
-                newTunnel.onStateChanged(originalState);
-                tunnels.add(newTunnel);
-                if (wasLastUsed)
-                    setLastUsedTunnel(newTunnel);
-            } else {
-                // Failure, put the old tunnel back.
-                tunnels.add(tunnel);
-                if (wasLastUsed)
-                    setLastUsedTunnel(tunnel);
-            }
-        });
-    }
-
     public CompletionStage<Void> restoreState() {
         if (!preferences.getBoolean(KEY_RESTORE_ON_BOOT, false))
             return CompletableFuture.completedFuture(null);
@@ -227,6 +180,38 @@ public final class TunnelManager extends BaseObservable {
         }).thenApply(tunnel::onConfigChanged);
     }
 
+    CompletionStage<String> setTunnelName(final Tunnel tunnel, final String name) {
+        if (!Tunnel.isNameValid(name))
+            return CompletableFuture.failedFuture(new IllegalArgumentException("Invalid name"));
+        if (tunnels.containsKey(name)) {
+            final String message = "Tunnel " + name + " already exists";
+            return CompletableFuture.failedFuture(new IllegalArgumentException(message));
+        }
+        final State originalState = tunnel.getState();
+        final boolean wasLastUsed = tunnel == lastUsedTunnel;
+        // Make sure nothing touches the tunnel.
+        if (wasLastUsed)
+            setLastUsedTunnel(null);
+        tunnels.remove(tunnel);
+        return asyncWorker.supplyAsync(() -> {
+            if (originalState == State.UP)
+                backend.setState(tunnel, State.DOWN);
+            configStore.rename(tunnel.getName(), name);
+            final String newName = tunnel.onNameChanged(name);
+            if (originalState == State.UP)
+                backend.setState(tunnel, originalState);
+            return newName;
+        }).whenComplete((newName, e) -> {
+            // On failure, we don't know what state the tunnel might be in. Fix that.
+            if (e != null)
+                getTunnelState(tunnel);
+            // Add the tunnel back to the manager, under whatever name it thinks it has.
+            tunnels.add(tunnel);
+            if (wasLastUsed)
+                setLastUsedTunnel(tunnel);
+        });
+    }
+
     CompletionStage<State> setTunnelState(final Tunnel tunnel, final State state) {
         // Ensure the configuration is loaded before trying to use it.
         return tunnel.getConfigAsync().thenCompose(x ->