Bug fixes and perf improvements for setting providers in AuthUI

auth/src/main/java/com/firebase/ui/auth/AuthUI.java

@@ -55,7 +55,6 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.IdentityHashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -315,9 +314,9 @@ public static class IdpConfig implements Parcelable {
         private final String mProviderId;
         private final List<String> mScopes;
 
-        private IdpConfig(@NonNull String providerId, List<String> scopes) {
-            mScopes = scopes;
+        private IdpConfig(String providerId, List<String> scopes) {
             mProviderId = providerId;
+            mScopes = scopes;
         }
 
         private IdpConfig(Parcel in) {
@@ -356,11 +355,33 @@ public void writeToParcel(Parcel parcel, int i) {
             parcel.writeStringList(mScopes);
         }
 
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            IdpConfig config = (IdpConfig) o;
+
+            return mProviderId.equals(config.mProviderId);
+        }
+
+        @Override
+        public int hashCode() {
+            return mProviderId.hashCode();
+        }
+
+        @Override
+        public String toString() {
+            return "IdpConfig{" +
+                    "mProviderId='" + mProviderId + '\'' +
+                    ", mScopes=" + mScopes +
+                    '}';
+        }
+
         public static class Builder {
             private String mProviderId;
             private List<String> mScopes = new ArrayList<>();
 
-
             /**
              * Builds the configuration parameters for an identity provider.
              *
@@ -406,7 +427,7 @@ public IdpConfig build() {
     public final class SignInIntentBuilder {
         private int mLogo = NO_LOGO;
         private int mTheme = getDefaultTheme();
-        private LinkedHashSet<IdpConfig> mProviders = new LinkedHashSet<>();
+        private List<IdpConfig> mProviders = new ArrayList<>();
         private String mTosUrl;
         private boolean mIsSmartLockEnabled = true;
         private boolean mAllowNewEmailAccounts = true;
@@ -458,19 +479,30 @@ public SignInIntentBuilder setTosUrl(@Nullable String tosUrl) {
          */
         public SignInIntentBuilder setProviders(@NonNull List<IdpConfig> idpConfigs) {
             mProviders.clear();
-            Set<String> configuredProviders = new HashSet<>();
-            for (IdpConfig idpConfig : idpConfigs) {
-                if (configuredProviders.contains(idpConfig.getProviderId())) {
-                    throw new IllegalArgumentException("Each provider can only be set once. "
-                                                               + idpConfig.getProviderId()
-                                                               + " was set twice.");
+            for (IdpConfig config : idpConfigs) {
+                if (mProviders.contains(config)) {
+                    int i = mProviders.indexOf(config);
+                    IdpConfig newConfig =
+                            new IdpConfig.Builder(config.getProviderId())
+                                    .setPermissions(getUniqueScopes(config.getScopes(),
+                                                                    mProviders.get(i).getScopes()))
+                                    .build();
+                    mProviders.set(i, newConfig);
+                } else {
+                    mProviders.add(config);
                 }
-                configuredProviders.add(idpConfig.getProviderId());
-                mProviders.add(idpConfig);
             }
             return this;
         }
 
+        private List<String> getUniqueScopes(List<String> scopes1, List<String> scopes2) {
+            List<String> mergedScopes = new ArrayList<>(scopes2);
+            for (String scope : scopes1) {
+                if (!mergedScopes.contains(scope)) mergedScopes.add(scope);
+            }
+            return mergedScopes;
+        }
+
         /**
          * Specifies the set of supported authentication providers. At least one provider
          * must be specified, and the set of providers must be a subset of
@@ -495,6 +527,15 @@ public SignInIntentBuilder setProviders(@NonNull String... providers) {
             return this;
         }
 
+        private boolean isIdpAlreadyConfigured(@NonNull String providerId) {
+            for (IdpConfig config : mProviders) {
+                if (config.getProviderId().equals(providerId)) {
+                    return true;
+                }
+            }
+            return false;
+        }
+
         /**
          * Enables or disables the use of Smart Lock for Passwords in the sign in flow.
          * <p>
@@ -515,23 +556,14 @@ public SignInIntentBuilder setAllowNewEmailAccounts(boolean enabled) {
             return this;
         }
 
-        private boolean isIdpAlreadyConfigured(@NonNull String providerId) {
-            for (IdpConfig config : mProviders) {
-                if (config.getProviderId().equals(providerId)) {
-                    return true;
-                }
-            }
-            return false;
-        }
-
         public Intent build() {
             return KickoffActivity.createIntent(mApp.getApplicationContext(), getFlowParams());
         }
 
         @VisibleForTesting()
         public FlowParameters getFlowParams() {
             return new FlowParameters(mApp.getName(),
-                                      new ArrayList<>(mProviders),
+                                      mProviders,
                                       mTheme,
                                       mLogo,
                                       mTosUrl,

auth/src/test/java/com/firebase/ui/auth/AuthUITest.java

@@ -29,8 +29,11 @@
 import org.robolectric.annotation.Config;
 
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 
 import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertTrue;
 
 @RunWith(CustomRobolectricGradleTestRunner.class)
 @Config(constants = BuildConfig.class, sdk = 25)
@@ -63,13 +66,30 @@ public void testCreateStartIntent_shouldHaveEmailAsDefaultProvider() {
         assertEquals(AuthUI.EMAIL_PROVIDER, flowParameters.providerInfo.get(0).getProviderId());
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void testCreateStartIntent_shouldOnlyAllowOneInstanceOfAnIdp() {
-        SignInIntentBuilder startIntent =
-                AuthUI.getInstance(mFirebaseApp).createSignInIntentBuilder();
-        startIntent.setProviders(
-                Arrays.asList(new IdpConfig.Builder(AuthUI.EMAIL_PROVIDER).build(),
-                              new IdpConfig.Builder(AuthUI.EMAIL_PROVIDER).build()));
+        SignInIntentBuilder builder = AuthUI.getInstance(mFirebaseApp).createSignInIntentBuilder();
+        builder.setProviders(Arrays.asList(
+                new IdpConfig.Builder(AuthUI.EMAIL_PROVIDER)
+                        .setPermissions(Collections.singletonList("A"))
+                        .build(),
+                new IdpConfig.Builder(AuthUI.EMAIL_PROVIDER)
+                        .setPermissions(Arrays.asList("A", "B"))
+                        .build(),
+                new IdpConfig.Builder(AuthUI.GOOGLE_PROVIDER)
+                        .build()));
+
+
+        List<IdpConfig> providers = builder.getFlowParams().providerInfo;
+        assertTrue(providers.size() == 2);
+
+        IdpConfig email = providers.get(0);
+        List<String> emailScopes = email.getScopes();
+        assertTrue(email.getProviderId().equals(AuthUI.EMAIL_PROVIDER));
+        assertTrue(emailScopes.size() == 2);
+        assertTrue(emailScopes.contains("A") && emailScopes.contains("B"));
+
+        assertTrue(providers.get(1).getProviderId().equals(AuthUI.GOOGLE_PROVIDER));
     }
 
     @Test