Remodel the Model

- The configuration and crypto model is now entirely independent
of Android classes other than Nullable and TextUtils.
- Model classes are immutable and use builders that enforce the
appropriate optional/required attributes.
- The Android config proxies (for Parcelable and databinding) are
moved to the Android side of the codebase, and are designed to be
safe for two-way databinding. This allows proper observability in
TunnelDetailFragment.
- Various robustness fixes and documentation updates to helper classes.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

5 files changed, 53 insertions, 109 deletions

diff --git a/app/src/main/java/com/wireguard/android/fragment/AppListDialogFragment.java b/app/src/main/java/com/wireguard/android/fragment/AppListDialogFragment.java
index 8bf5a22d..20633c3e 100644
--- a/app/src/main/java/com/wireguard/android/fragment/AppListDialogFragment.java
+++ b/app/src/main/java/com/wireguard/android/fragment/AppListDialogFragment.java
@@ -27,19 +27,21 @@ import com.wireguard.android.util.ObservableKeyedArrayList;
 import com.wireguard.android.util.ObservableKeyedList;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
+import java9.util.Comparators;
+
 public class AppListDialogFragment extends DialogFragment {
 
     private static final String KEY_EXCLUDED_APPS = "excludedApps";
     private final ObservableKeyedList<String, ApplicationData> appData = new ObservableKeyedArrayList<>();
-    private List<String> currentlyExcludedApps;
+    @Nullable private List<String> currentlyExcludedApps;
 
-    public static <T extends Fragment & AppExclusionListener> AppListDialogFragment newInstance(final String[] excludedApps, final T target) {
+    public static <T extends Fragment & AppExclusionListener>
+    AppListDialogFragment newInstance(final ArrayList<String> excludedApps, final T target) {
         final Bundle extras = new Bundle();
-        extras.putStringArray(KEY_EXCLUDED_APPS, excludedApps);
+        extras.putStringArrayList(KEY_EXCLUDED_APPS, excludedApps);
         final AppListDialogFragment fragment = new AppListDialogFragment();
         fragment.setTargetFragment(target, 0);
         fragment.setArguments(extras);
@@ -64,7 +66,7 @@ public class AppListDialogFragment extends DialogFragment {
                 appData.add(new ApplicationData(resolveInfo.loadIcon(pm), resolveInfo.loadLabel(pm).toString(), packageName, currentlyExcludedApps.contains(packageName)));
             }
 
-            Collections.sort(appData, (lhs, rhs) -> lhs.getName().toLowerCase().compareTo(rhs.getName().toLowerCase()));
+            Collections.sort(appData, Comparators.comparing(ApplicationData::getName, String.CASE_INSENSITIVE_ORDER));
             return appData;
         }).whenComplete(((data, throwable) -> {
             if (data != null) {
@@ -82,12 +84,11 @@ public class AppListDialogFragment extends DialogFragment {
     @Override
     public void onCreate(@Nullable final Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
-
-        currentlyExcludedApps = Arrays.asList(getArguments().getStringArray(KEY_EXCLUDED_APPS));
+        currentlyExcludedApps = getArguments().getStringArrayList(KEY_EXCLUDED_APPS);
     }
 
     @Override
-    public Dialog onCreateDialog(final Bundle savedInstanceState) {
+    public Dialog onCreateDialog(@Nullable final Bundle savedInstanceState) {
         final AlertDialog.Builder alertDialogBuilder = new AlertDialog.Builder(getActivity());
         alertDialogBuilder.setTitle(R.string.excluded_applications);
 
diff --git a/app/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.java b/app/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.java
index 83799818..0931868e 100644
--- a/app/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.java
+++ b/app/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.java
@@ -19,8 +19,11 @@ import com.wireguard.android.Application;
 import com.wireguard.android.R;
 import com.wireguard.android.databinding.ConfigNamingDialogFragmentBinding;
 import com.wireguard.config.Config;
+import com.wireguard.config.ParseException;
 
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.Objects;
 
 public class ConfigNamingDialogFragment extends DialogFragment {
@@ -63,8 +66,8 @@ public class ConfigNamingDialogFragment extends DialogFragment {
         super.onCreate(savedInstanceState);
 
         try {
-            config = Config.from(getArguments().getString(KEY_CONFIG_TEXT));
-        } catch (final IOException exception) {
+            config = Config.parse(new ByteArrayInputStream(getArguments().getString(KEY_CONFIG_TEXT).getBytes(StandardCharsets.UTF_8)));
+        } catch (final IOException | ParseException exception) {
             throw new RuntimeException("Invalid config passed to " + getClass().getSimpleName(), exception);
         }
     }
diff --git a/app/src/main/java/com/wireguard/android/fragment/TunnelDetailFragment.java b/app/src/main/java/com/wireguard/android/fragment/TunnelDetailFragment.java
index fcc601f3..b4e7202f 100644
--- a/app/src/main/java/com/wireguard/android/fragment/TunnelDetailFragment.java
+++ b/app/src/main/java/com/wireguard/android/fragment/TunnelDetailFragment.java
@@ -16,7 +16,6 @@ import android.view.ViewGroup;
 import com.wireguard.android.R;
 import com.wireguard.android.databinding.TunnelDetailFragmentBinding;
 import com.wireguard.android.model.Tunnel;
-import com.wireguard.config.Config;
 
 /**
  * Fragment that shows details about a specific tunnel.
@@ -25,12 +24,6 @@ import com.wireguard.config.Config;
 public class TunnelDetailFragment extends BaseFragment {
     @Nullable private TunnelDetailFragmentBinding binding;
 
-    private void onConfigLoaded(final String name, final Config config) {
-        if (binding != null) {
-            binding.setConfig(new Config.Observable(config, name));
-        }
-    }
-
     @Override
     public void onCreate(@Nullable final Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -65,7 +58,7 @@ public class TunnelDetailFragment extends BaseFragment {
         if (newTunnel == null)
             binding.setConfig(null);
         else
-            newTunnel.getConfigAsync().thenAccept(a -> onConfigLoaded(newTunnel.getName(), a));
+            newTunnel.getConfigAsync().thenAccept(binding::setConfig);
     }
 
     @Override
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 8f319e1e..f1250e64 100644
--- a/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java
+++ b/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java
@@ -7,7 +7,6 @@ package com.wireguard.android.fragment;
 
 import android.app.Activity;
 import android.content.Context;
-import android.databinding.Observable;
 import android.databinding.ObservableList;
 import android.os.Bundle;
 import android.support.annotation.Nullable;
@@ -24,19 +23,16 @@ import android.view.inputmethod.InputMethodManager;
 import android.widget.Toast;
 
 import com.wireguard.android.Application;
-import com.wireguard.android.BR;
 import com.wireguard.android.R;
 import com.wireguard.android.databinding.TunnelEditorFragmentBinding;
 import com.wireguard.android.fragment.AppListDialogFragment.AppExclusionListener;
 import com.wireguard.android.model.Tunnel;
 import com.wireguard.android.model.TunnelManager;
 import com.wireguard.android.util.ExceptionLoggers;
-import com.wireguard.config.Attribute;
+import com.wireguard.android.viewmodel.ConfigProxy;
 import com.wireguard.config.Config;
-import com.wireguard.config.Peer;
 
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
 
@@ -48,64 +44,13 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi
     private static final String KEY_LOCAL_CONFIG = "local_config";
     private static final String KEY_ORIGINAL_NAME = "original_name";
     private static final String TAG = "WireGuard/" + TunnelEditorFragment.class.getSimpleName();
-    private final Collection<Object> breakObjectOrientedLayeringHandlerReceivers = new ArrayList<>();
-    @Nullable private TunnelEditorFragmentBinding binding;
-    private final Observable.OnPropertyChangedCallback breakObjectOrientedLayeringHandler = new Observable.OnPropertyChangedCallback() {
-        @Override
-        public void onPropertyChanged(final Observable sender, final int propertyId) {
-            if (binding == null)
-                return;
-            final Config.Observable config = binding.getConfig();
-            if (config == null)
-                return;
-            if (propertyId == BR.config) {
-                config.addOnPropertyChangedCallback(breakObjectOrientedLayeringHandler);
-                breakObjectOrientedLayeringHandlerReceivers.add(config);
-                config.getInterfaceSection().addOnPropertyChangedCallback(breakObjectOrientedLayeringHandler);
-                breakObjectOrientedLayeringHandlerReceivers.add(config.getInterfaceSection());
-                config.getPeers().addOnListChangedCallback(breakObjectListOrientedLayeringHandler);
-                breakObjectOrientedLayeringHandlerReceivers.add(config.getPeers());
-            } else if (propertyId == BR.dnses || propertyId == BR.peers)
-                ;
-            else
-                return;
-            final int numSiblings = config.getPeers().size() - 1;
-            for (final Peer.Observable peer : config.getPeers()) {
-                peer.setInterfaceDNSRoutes(config.getInterfaceSection().getDnses());
-                peer.setNumSiblings(numSiblings);
-            }
-        }
-    };
-    private final ObservableList.OnListChangedCallback<? extends ObservableList<Peer.Observable>> breakObjectListOrientedLayeringHandler = new ObservableList.OnListChangedCallback<ObservableList<Peer.Observable>>() {
-        @Override
-        public void onChanged(final ObservableList<Peer.Observable> sender) {
-        }
-
-        @Override
-        public void onItemRangeChanged(final ObservableList<Peer.Observable> sender, final int positionStart, final int itemCount) {
-        }
-
-        @Override
-        public void onItemRangeInserted(final ObservableList<Peer.Observable> sender, final int positionStart, final int itemCount) {
-            if (binding != null)
-                breakObjectOrientedLayeringHandler.onPropertyChanged(binding.getConfig(), BR.peers);
-        }
 
-        @Override
-        public void onItemRangeMoved(final ObservableList<Peer.Observable> sender, final int fromPosition, final int toPosition, final int itemCount) {
-        }
-
-        @Override
-        public void onItemRangeRemoved(final ObservableList<Peer.Observable> sender, final int positionStart, final int itemCount) {
-            if (binding != null)
-                breakObjectOrientedLayeringHandler.onPropertyChanged(binding.getConfig(), BR.peers);
-        }
-    };
+    @Nullable private TunnelEditorFragmentBinding binding;
     @Nullable private Tunnel tunnel;
 
-    private void onConfigLoaded(final String name, final Config config) {
+    private void onConfigLoaded(final Config config) {
         if (binding != null) {
-            binding.setConfig(new Config.Observable(config, name));
+            binding.setConfig(new ConfigProxy(config));
         }
     }
 
@@ -143,29 +88,23 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi
                              @Nullable final Bundle savedInstanceState) {
         super.onCreateView(inflater, container, savedInstanceState);
         binding = TunnelEditorFragmentBinding.inflate(inflater, container, false);
-        binding.addOnPropertyChangedCallback(breakObjectOrientedLayeringHandler);
-        breakObjectOrientedLayeringHandlerReceivers.add(binding);
         binding.executePendingBindings();
         return binding.getRoot();
     }
 
-    @SuppressWarnings("unchecked")
     @Override
     public void onDestroyView() {
         binding = null;
-        for (final Object o : breakObjectOrientedLayeringHandlerReceivers) {
-            if (o instanceof Observable)
-                ((Observable) o).removeOnPropertyChangedCallback(breakObjectOrientedLayeringHandler);
-            else if (o instanceof ObservableList)
-                ((ObservableList) o).removeOnListChangedCallback(breakObjectListOrientedLayeringHandler);
-        }
         super.onDestroyView();
     }
 
     @Override
     public void onExcludedAppsSelected(final List<String> excludedApps) {
         Objects.requireNonNull(binding, "Tried to set excluded apps while no view was loaded");
-        binding.getConfig().getInterfaceSection().setExcludedApplications(Attribute.iterableToString(excludedApps));
+        final ObservableList<String> excludedApplications =
+                binding.getConfig().getInterface().getExcludedApplications();
+        excludedApplications.clear();
+        excludedApplications.addAll(excludedApps);
     }
 
     private void onFinished() {
@@ -195,25 +134,27 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi
     public boolean onOptionsItemSelected(final MenuItem item) {
         switch (item.getItemId()) {
             case R.id.menu_action_save:
-                final Config newConfig = new Config();
+                if (binding == null)
+                    return false;
+                final Config newConfig;
                 try {
-                    binding.getConfig().commitData(newConfig);
+                    newConfig = binding.getConfig().resolve();
                 } catch (final Exception e) {
                     final String error = ExceptionLoggers.unwrapMessage(e);
-                    final String tunnelName = tunnel == null ? binding.getConfig().getName() : tunnel.getName();
+                    final String tunnelName = tunnel == null ? binding.getName() : tunnel.getName();
                     final String message = getString(R.string.config_save_error, tunnelName, error);
                     Log.e(TAG, message, e);
                     Snackbar.make(binding.mainContainer, error, Snackbar.LENGTH_LONG).show();
                     return false;
                 }
                 if (tunnel == null) {
-                    Log.d(TAG, "Attempting to create new tunnel " + binding.getConfig().getName());
+                    Log.d(TAG, "Attempting to create new tunnel " + binding.getName());
                     final TunnelManager manager = Application.getTunnelManager();
-                    manager.create(binding.getConfig().getName(), newConfig)
+                    manager.create(binding.getName(), newConfig)
                             .whenComplete(this::onTunnelCreated);
-                } else if (!tunnel.getName().equals(binding.getConfig().getName())) {
-                    Log.d(TAG, "Attempting to rename tunnel to " + binding.getConfig().getName());
-                    tunnel.setName(binding.getConfig().getName())
+                } else if (!tunnel.getName().equals(binding.getName())) {
+                    Log.d(TAG, "Attempting to rename tunnel to " + binding.getName());
+                    tunnel.setName(binding.getName())
                             .whenComplete((a, b) -> onTunnelRenamed(tunnel, newConfig, b));
                 } else {
                     Log.d(TAG, "Attempting to save config of " + tunnel.getName());
@@ -229,7 +170,7 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi
     public void onRequestSetExcludedApplications(@SuppressWarnings("unused") final View view) {
         final FragmentManager fragmentManager = getFragmentManager();
         if (fragmentManager != null && binding != null) {
-            final String[] excludedApps = Attribute.stringToList(binding.getConfig().getInterfaceSection().getExcludedApplications());
+            final ArrayList<String> excludedApps = new ArrayList<>(binding.getConfig().getInterface().getExcludedApplications());
             final AppListDialogFragment fragment = AppListDialogFragment.newInstance(excludedApps, this);
             fragment.show(fragmentManager, null);
         }
@@ -237,19 +178,25 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi
 
     @Override
     public void onSaveInstanceState(final Bundle outState) {
-        outState.putParcelable(KEY_LOCAL_CONFIG, binding.getConfig());
+        if (binding != null)
+            outState.putParcelable(KEY_LOCAL_CONFIG, binding.getConfig());
         outState.putString(KEY_ORIGINAL_NAME, tunnel == null ? null : tunnel.getName());
         super.onSaveInstanceState(outState);
     }
 
     @Override
-    public void onSelectedTunnelChanged(@Nullable final Tunnel oldTunnel, @Nullable final Tunnel newTunnel) {
+    public void onSelectedTunnelChanged(@Nullable final Tunnel oldTunnel,
+                                        @Nullable final Tunnel newTunnel) {
         tunnel = newTunnel;
         if (binding == null)
             return;
-        binding.setConfig(new Config.Observable(null, null));
-        if (tunnel != null)
-            tunnel.getConfigAsync().thenAccept(a -> onConfigLoaded(tunnel.getName(), a));
+        binding.setConfig(new ConfigProxy());
+        if (tunnel != null) {
+            binding.setName(tunnel.getName());
+            tunnel.getConfigAsync().thenAccept(this::onConfigLoaded);
+        } else {
+            binding.setName("");
+        }
     }
 
     private void onTunnelCreated(final Tunnel newTunnel, @Nullable final Throwable throwable) {
@@ -301,7 +248,7 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi
             onSelectedTunnelChanged(null, getSelectedTunnel());
         } else {
             tunnel = getSelectedTunnel();
-            final Config.Observable config = savedInstanceState.getParcelable(KEY_LOCAL_CONFIG);
+            final ConfigProxy config = savedInstanceState.getParcelable(KEY_LOCAL_CONFIG);
             final String originalName = savedInstanceState.getString(KEY_ORIGINAL_NAME);
             if (tunnel != null && !tunnel.getName().equals(originalName))
                 onSelectedTunnelChanged(null, tunnel);
diff --git a/app/src/main/java/com/wireguard/android/fragment/TunnelListFragment.java b/app/src/main/java/com/wireguard/android/fragment/TunnelListFragment.java
index 7509e40c..783c0a29 100644
--- a/app/src/main/java/com/wireguard/android/fragment/TunnelListFragment.java
+++ b/app/src/main/java/com/wireguard/android/fragment/TunnelListFragment.java
@@ -41,9 +41,10 @@ import com.wireguard.android.util.ExceptionLoggers;
 import com.wireguard.android.widget.MultiselectableRelativeLayout;
 import com.wireguard.android.widget.fab.FloatingActionsMenuRecyclerViewScrollListener;
 import com.wireguard.config.Config;
+import com.wireguard.config.ParseException;
 
-import java.io.BufferedReader;
-import java.io.InputStreamReader;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -79,13 +80,13 @@ public class TunnelListFragment extends BaseFragment {
     private void importTunnel(@NonNull final String configText) {
         try {
             // Ensure the config text is parseable before proceeding…
-            Config.from(configText);
+            Config.parse(new ByteArrayInputStream(configText.getBytes(StandardCharsets.UTF_8)));
 
             // Config text is valid, now create the tunnel…
             final FragmentManager fragmentManager = getFragmentManager();
             if (fragmentManager != null)
                 ConfigNamingDialogFragment.newInstance(configText).show(fragmentManager, null);
-        } catch (final Exception exception) {
+        } catch (final IllegalArgumentException | IOException | ParseException exception) {
             onTunnelImportFinished(Collections.emptyList(), Collections.singletonList(exception));
         }
     }
@@ -122,7 +123,6 @@ public class TunnelListFragment extends BaseFragment {
 
             if (isZip) {
                 try (ZipInputStream zip = new ZipInputStream(contentResolver.openInputStream(uri))) {
-                    BufferedReader reader = new BufferedReader(new InputStreamReader(zip, StandardCharsets.UTF_8));
                     ZipEntry entry;
                     while ((entry = zip.getNextEntry()) != null) {
                         if (entry.isDirectory())
@@ -140,7 +140,7 @@ public class TunnelListFragment extends BaseFragment {
                             continue;
                         Config config = null;
                         try {
-                            config = Config.from(reader);
+                            config = Config.parse(zip);
                         } catch (Exception e) {
                             throwables.add(e);
                         }
@@ -150,7 +150,7 @@ public class TunnelListFragment extends BaseFragment {
                 }
             } else {
                 futureTunnels.add(Application.getTunnelManager().create(name,
-                        Config.from(contentResolver.openInputStream(uri))).toCompletableFuture());
+                        Config.parse(contentResolver.openInputStream(uri))).toCompletableFuture());
             }
 
             if (futureTunnels.isEmpty()) {