From e4f4521f9d50aad80a7c689eb949e75d1b521bac Mon Sep 17 00:00:00 2001 From: cyBerta Date: Wed, 21 Feb 2018 11:12:21 +0100 Subject: #8858 add custom providers to the providers list on successful setup --- app/assets/urls/calyx.net.url | 2 +- app/assets/urls/demo.bitmask.net.url | 2 +- app/assets/urls/riseup.net.url | 2 +- .../main/java/se/leap/bitmaskclient/Provider.java | 3 -- .../bitmaskclient/ProviderListBaseActivity.java | 3 +- .../se/leap/bitmaskclient/ProviderManager.java | 50 +++++++++++++++++++--- .../leap/bitmaskclient/ProviderListActivity.java | 3 +- .../java/se/leap/bitmaskclient/ProviderTest.java | 30 +++++++++++++ 8 files changed, 81 insertions(+), 14 deletions(-) diff --git a/app/assets/urls/calyx.net.url b/app/assets/urls/calyx.net.url index 8de04fe9..807e9e18 100644 --- a/app/assets/urls/calyx.net.url +++ b/app/assets/urls/calyx.net.url @@ -1,3 +1,3 @@ { - "main_url" : "https://calyx.net/" + "main_url" : "https://calyx.net" } diff --git a/app/assets/urls/demo.bitmask.net.url b/app/assets/urls/demo.bitmask.net.url index 1a412055..0c4de648 100644 --- a/app/assets/urls/demo.bitmask.net.url +++ b/app/assets/urls/demo.bitmask.net.url @@ -1,3 +1,3 @@ { - "main_url" : "https://demo.bitmask.net/" + "main_url" : "https://demo.bitmask.net" } diff --git a/app/assets/urls/riseup.net.url b/app/assets/urls/riseup.net.url index 4548b433..42cdb979 100644 --- a/app/assets/urls/riseup.net.url +++ b/app/assets/urls/riseup.net.url @@ -1,3 +1,3 @@ { - "main_url" : "https://riseup.net/" + "main_url" : "https://riseup.net" } diff --git a/app/src/main/java/se/leap/bitmaskclient/Provider.java b/app/src/main/java/se/leap/bitmaskclient/Provider.java index 7104143c..f4e2c477 100644 --- a/app/src/main/java/se/leap/bitmaskclient/Provider.java +++ b/app/src/main/java/se/leap/bitmaskclient/Provider.java @@ -297,8 +297,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(); } @@ -446,5 +444,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..b625c336 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 { private File externalFilesDir; private Set defaultProviders; private Set customProviders; + private Set defaultProviderURLs; + private Set customProviderURLs; private static ProviderManager instance; @@ -52,11 +54,20 @@ public class ProviderManager implements AdapteeCollection { private void addDefaultProviders(AssetManager assets_manager) { try { defaultProviders = providersFromAssets(URLS, assets_manager.list(URLS)); + defaultProviderURLs = getProviderUrlSetFromProviderSet(defaultProviders); } catch (IOException e) { e.printStackTrace(); } } + private Set getProviderUrlSetFromProviderSet(Set providers) { + HashSet providerUrls = new HashSet<>(); + for (Provider provider : providers) { + providerUrls.add(provider.getMainUrl().getUrl()); + } + return providerUrls; + } + private Set providersFromAssets(String directory, String[] relativeFilePaths) { Set providers = new HashSet<>(); @@ -89,6 +100,7 @@ public class ProviderManager implements AdapteeCollection { customProviders = externalFilesDir != null && externalFilesDir.isDirectory() ? providersFromFiles(externalFilesDir.list()) : new HashSet(); + customProviderURLs = getProviderUrlSetFromProviderSet(customProviders); } private Set providersFromFiles(String[] files) { @@ -132,6 +144,8 @@ public class ProviderManager implements AdapteeCollection { 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,30 +167,56 @@ public class ProviderManager implements AdapteeCollection { @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 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(); } void saveCustomProvidersToFile() { diff --git a/app/src/production/java/se/leap/bitmaskclient/ProviderListActivity.java b/app/src/production/java/se/leap/bitmaskclient/ProviderListActivity.java index b6e67331..cf1d1aa6 100644 --- a/app/src/production/java/se/leap/bitmaskclient/ProviderListActivity.java +++ b/app/src/production/java/se/leap/bitmaskclient/ProviderListActivity.java @@ -39,11 +39,10 @@ public class ProviderListActivity extends ProviderListBaseActivity { setUpProvider(); } + @Override public void showAndSelectProvider(String provider_main_url) { try { provider = new Provider(new URL((provider_main_url))); - adapter.add(provider); - adapter.saveProviders(); autoSelectProvider(provider); } catch (MalformedURLException e) { e.printStackTrace(); diff --git a/app/src/test/java/se/leap/bitmaskclient/ProviderTest.java b/app/src/test/java/se/leap/bitmaskclient/ProviderTest.java index 794c3087..495d5b3f 100644 --- a/app/src/test/java/se/leap/bitmaskclient/ProviderTest.java +++ b/app/src/test/java/se/leap/bitmaskclient/ProviderTest.java @@ -1,9 +1,15 @@ package se.leap.bitmaskclient; +import org.json.JSONException; import org.junit.Test; +import java.io.IOException; +import java.util.HashSet; +import java.util.Set; + import se.leap.bitmaskclient.testutils.TestSetupHelper; +import static junit.framework.Assert.assertFalse; import static org.junit.Assert.assertTrue; /** @@ -18,4 +24,28 @@ public class ProviderTest { assertTrue("Providers should be same:", p1.equals(p2)); } + @Test + public void testEquals_sameFields_returnsFalse() throws Exception { + Provider p1 = TestSetupHelper.getConfiguredProvider(); + Provider p2 = TestSetupHelper.getConfiguredProvider(); + p2.setMainUrl("http://somethingsdiffer.org"); + assertFalse("Providers should be same:", p1.equals(p2)); + } + + // see ProviderManagerTest testing add(...) + @Test + public void testEqualsThroughSetContains_differentFields_returnsFalse() throws Exception { + Provider p1 = TestSetupHelper.getConfiguredProvider(); + Provider p2 = TestSetupHelper.getConfiguredProvider(); + p2.setMainUrl("http://somethingsdiffer.org"); + Provider p3 = new Provider("https://anotherprovider.net"); + + Set defaultProviders = new HashSet<>(); + defaultProviders.add(p1); + defaultProviders.add(p2); + + assertTrue(defaultProviders.contains(p1)); + assertTrue(defaultProviders.contains(p2)); + assertFalse(defaultProviders.contains(p3)); + } } -- cgit v1.2.3