Merge branch '#8858_persist_custom_providers' into '0.9.8'

#8858 persist custom providers

See merge request leap/bitmask_android!47

4 files changed, 60 insertions, 39 deletions

diff --git a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java
index a84432e9..a21a9601 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java
@@ -27,6 +27,8 @@ import org.spongycastle.util.encoders.Base64;
 
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -47,9 +49,11 @@ import java.security.interfaces.RSAPrivateKey;
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.PKCS8EncodedKeySpec;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 
 import static android.R.attr.name;
 import static se.leap.bitmaskclient.Constants.PREFERENCES_APP_VERSION;
@@ -125,36 +129,14 @@ public class ConfigHelper {
         return (X509Certificate) certificate;
     }
 
+    public static String loadInputStreamAsString(java.io.InputStream is) {
+        java.util.Scanner s = new java.util.Scanner(is).useDelimiter("\\A");
+        return s.hasNext() ? s.next() : "";
+    }
 
-    public static String loadInputStreamAsString(InputStream inputStream) {
-        BufferedReader in = null;
-        try {
-            StringBuilder buf = new StringBuilder();
-            in = new BufferedReader(new InputStreamReader(inputStream));
-
-            String str;
-            boolean isFirst = true;
-            while ( (str = in.readLine()) != null ) {
-                if (isFirst)
-                    isFirst = false;
-                else
-                    buf.append('\n');
-                buf.append(str);
-            }
-            return buf.toString();
-        } catch (IOException e) {
-            Log.e(TAG, "Error opening asset " + name);
-        } finally {
-            if (in != null) {
-                try {
-                    in.close();
-                } catch (IOException e) {
-                    Log.e(TAG, "Error closing asset " + name);
-                }
-            }
-        }
-
-        return null;
+    //allows us to mock FileInputStream
+    public static InputStream getInputStreamFrom(String filePath) throws FileNotFoundException {
+        return new FileInputStream(filePath);
     }
 
     protected static RSAPrivateKey parseRsaKeyFromString(String rsaKeyString) {
diff --git a/app/src/main/java/se/leap/bitmaskclient/Provider.java b/app/src/main/java/se/leap/bitmaskclient/Provider.java
index 98662783..fd067bf9 100644
--- a/app/src/main/java/se/leap/bitmaskclient/Provider.java
+++ b/app/src/main/java/se/leap/bitmaskclient/Provider.java
@@ -278,8 +278,6 @@ public final class Provider implements Parcelable {
         try {
             json.put(Provider.MAIN_URL, mainUrl);
             //TODO: add other fields here?
-            //this is used to save custom providers as json. I guess this doesn't work correctly
-            //TODO 2: verify that
         } catch (JSONException e) {
             e.printStackTrace();
         }
@@ -428,5 +426,4 @@ public final class Provider implements Parcelable {
         allowRegistered = false;
         allowAnonymous = false;
     }
-
 }
diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java b/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java
index e961b0a2..3bf51a8c 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java
@@ -207,7 +207,8 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
 
     void handleProviderSetUp(Provider handledProvider) {
         this.provider = handledProvider;
-
+        adapter.add(provider);
+        adapter.saveProviders();
         if (provider.allowsAnonymous()) {
             mConfigState.putExtra(SERVICES_RETRIEVED, true);
             downloadVpnCertificate();
diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderManager.java b/app/src/main/java/se/leap/bitmaskclient/ProviderManager.java
index ed41be67..97ba3b98 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ProviderManager.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ProviderManager.java
@@ -31,6 +31,8 @@ public class ProviderManager implements AdapteeCollection<Provider> {
     private File externalFilesDir;
     private Set<Provider> defaultProviders;
     private Set<Provider> customProviders;
+    private Set<URL> defaultProviderURLs;
+    private Set<URL> customProviderURLs;
 
     private static ProviderManager instance;
 
@@ -52,11 +54,20 @@ public class ProviderManager implements AdapteeCollection<Provider> {
     private void addDefaultProviders(AssetManager assets_manager) {
         try {
             defaultProviders = providersFromAssets(URLS, assets_manager.list(URLS));
+            defaultProviderURLs = getProviderUrlSetFromProviderSet(defaultProviders);
         } catch (IOException e) {
             e.printStackTrace();
         }
     }
 
+    private Set<URL> getProviderUrlSetFromProviderSet(Set<Provider> providers) {
+        HashSet<URL> providerUrls = new HashSet<>();
+        for (Provider provider : providers) {
+            providerUrls.add(provider.getMainUrl().getUrl());
+        }
+        return providerUrls;
+    }
+
     private Set<Provider> providersFromAssets(String directory, String[] relativeFilePaths) {
         Set<Provider> providers = new HashSet<>();
 
@@ -89,13 +100,14 @@ public class ProviderManager implements AdapteeCollection<Provider> {
         customProviders = externalFilesDir != null && externalFilesDir.isDirectory() ?
                 providersFromFiles(externalFilesDir.list()) :
                 new HashSet<Provider>();
+        customProviderURLs = getProviderUrlSetFromProviderSet(customProviders);
     }
 
     private Set<Provider> providersFromFiles(String[] files) {
         Set<Provider> providers = new HashSet<>();
         try {
             for (String file : files) {
-                String mainUrl = extractMainUrlFromInputStream(new FileInputStream(externalFilesDir.getAbsolutePath() + "/" + file));
+                String mainUrl = extractMainUrlFromInputStream(ConfigHelper.getInputStreamFrom(externalFilesDir.getAbsolutePath() + "/" + file));
                 providers.add(new Provider(new URL(mainUrl)));
             }
         } catch (MalformedURLException | FileNotFoundException e) {
@@ -132,6 +144,8 @@ public class ProviderManager implements AdapteeCollection<Provider> {
         allProviders.addAll(defaultProviders);
         if(customProviders != null)
             allProviders.addAll(customProviders);
+        //add an option to add a custom provider
+        //TODO: refactor me?
         allProviders.add(new Provider());
         return allProviders;
     }
@@ -153,32 +167,59 @@ public class ProviderManager implements AdapteeCollection<Provider> {
 
     @Override
     public boolean add(Provider element) {
-        return !defaultProviders.contains(element) || customProviders.add(element);
+        return element != null &&
+                !defaultProviderURLs.contains(element.getMainUrl().getUrl()) &&
+                customProviders.add(element) &&
+                customProviderURLs.add(element.getMainUrl().getUrl());
     }
 
     @Override
     public boolean remove(Object element) {
-        return customProviders.remove(element);
+        return element instanceof Provider &&
+                customProviders.remove(element) &&
+                customProviderURLs.remove(((Provider) element).getMainUrl().getUrl());
     }
 
     @Override
     public boolean addAll(Collection<? extends Provider> elements) {
-        return customProviders.addAll(elements);
+        Iterator iterator = elements.iterator();
+        boolean addedAll = true;
+        while (iterator.hasNext()) {
+            Provider p = (Provider) iterator.next();
+            addedAll = customProviders.add(p) &&
+                    customProviderURLs.add(p.getMainUrl().getUrl()) &&
+                    addedAll;
+        }
+        return addedAll;
     }
 
     @Override
     public boolean removeAll(Collection<?> elements) {
-        if(!elements.getClass().equals(Provider.class))
+        Iterator iterator = elements.iterator();
+        boolean removedAll = true;
+        try {
+            while (iterator.hasNext()) {
+                Provider p = (Provider) iterator.next();
+                removedAll = ((defaultProviders.remove(p) && defaultProviderURLs.remove(p.getMainUrl().getUrl())) ||
+                        (customProviders.remove(p) && customProviderURLs.remove(p.getMainUrl().getUrl()))) &&
+                        removedAll;
+            }
+        } catch (ClassCastException e) {
             return false;
-        return defaultProviders.removeAll(elements) || customProviders.removeAll(elements);
+        }
+
+        return removedAll;
     }
 
     @Override
     public void clear() {
         defaultProviders.clear();
         customProviders.clear();
+        customProviderURLs.clear();
+        defaultProviderURLs.clear();
     }
 
+    //FIXME: removed custom providers should be deleted here as well
     void saveCustomProvidersToFile() {
         try {
             for (Provider provider : customProviders) {