Account REST API: adjust the deletable condition of external ids

Set the 'canDelete' field of external ids to be true if the external id
for the last login is null.

This is different from the old remote JSON service implementation, but it
seems reasonable because an external id should be deletable if the external
id is not a 'SCHEME_USERNAME' and it's not used to login.

And this change will make it much easier for us to test the get/delete
external ids REST API.

Change-Id: I0ab347bddf782b20a3be1826279df5a1ba88c5df

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java

@@ -38,7 +38,6 @@ import com.google.common.io.BaseEncoding;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.AccountCreator;
 import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.acceptance.UseSsh;
 import com.google.gerrit.common.data.Permission;
@@ -47,7 +46,6 @@ import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.StarsInput;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
-import com.google.gerrit.extensions.common.AccountExternalIdInfo;
 import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.GpgKeyInfo;
@@ -72,7 +70,6 @@ import com.google.gerrit.server.project.RefPattern;
 import com.google.gerrit.server.util.MagicBranch;
 import com.google.gerrit.testutil.ConfigSuite;
 import com.google.gerrit.testutil.FakeEmailSender.Message;
-import com.google.gson.reflect.TypeToken;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -101,7 +98,6 @@ import java.util.EnumSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 public class AccountIT extends AbstractDaemonTest {
   @ConfigSuite.Default
@@ -747,33 +743,6 @@ public class AccountIT extends AbstractDaemonTest {
     gApi.accounts().id(admin.username).index();
   }
 
-  @Test
-  public void getAccountExternalIds() throws Exception {
-    List<AccountExternalIdInfo> expectedIdInfoList = getExternalIds(user)
-        .stream().map(AccountIT::toInfo).sorted().collect(Collectors.toList());
-
-    RestResponse response = userRestSession.get("/accounts/self/external.ids");
-    response.assertOK();
-    List<AccountExternalIdInfo> externalIdInfoList =
-        newGson().fromJson(response.getReader(),
-            new TypeToken<List<AccountExternalIdInfo>>() {}.getType());
-
-    // 'canDelete' field will be all null (false). It will be better if we can
-    // find a way to test it. But it looks a little difficult.
-    externalIdInfoList.stream().sorted();
-    assertThat(expectedIdInfoList)
-        .containsExactlyElementsIn(expectedIdInfoList);
-  }
-
-  private static AccountExternalIdInfo toInfo(AccountExternalId id) {
-    AccountExternalIdInfo info = new AccountExternalIdInfo();
-    info.identity = id.getExternalId();
-    info.emailAddress = id.getEmailAddress();
-    info.trusted = id.isTrusted();
-    info.canDelete = id.canDelete();
-    return info;
-  }
-
   private void assertSequenceNumbers(List<SshKeyInfo> sshKeys) {
     int seq = 1;
     for (SshKeyInfo key : sshKeys) {

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java

@@ -0,0 +1,65 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.rest.account;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.extensions.common.AccountExternalIdInfo;
+import com.google.gerrit.reviewdb.client.AccountExternalId;
+import com.google.gson.reflect.TypeToken;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+public class ExternalIdIT extends AbstractDaemonTest {
+  @Test
+  public void getExternalIDs() throws Exception {
+    Collection<AccountExternalId> expectedIds =
+        accountCache.get(user.getId()).getExternalIds();
+
+    List<AccountExternalIdInfo> expectedIdInfos = new ArrayList<>();
+    for (AccountExternalId id : expectedIds) {
+      id.setCanDelete(!id.getExternalId().equals("username:" + user.username));
+      id.setTrusted(true);
+      expectedIdInfos.add(toInfo(id));
+    }
+
+    RestResponse response = userRestSession.get("/accounts/self/external.ids");
+    response.assertOK();
+
+    List<AccountExternalIdInfo> results =
+        newGson().fromJson(response.getReader(),
+            new TypeToken<List<AccountExternalIdInfo>>() {}.getType());
+
+    Collections.sort(expectedIdInfos);
+    Collections.sort(results);
+    assertThat(results).containsExactlyElementsIn(expectedIdInfos);
+  }
+
+  private static AccountExternalIdInfo toInfo(AccountExternalId id) {
+    AccountExternalIdInfo info = new AccountExternalIdInfo();
+    info.identity = id.getExternalId();
+    info.emailAddress = id.getEmailAddress();
+    info.trusted = id.isTrusted() ? true : null;
+    info.canDelete = id.canDelete() ? true : null;
+    return info;
+  }
+}

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountExternalIdInfo.java

@@ -16,6 +16,8 @@ package com.google.gerrit.extensions.common;
 
 import com.google.common.collect.ComparisonChain;
 
+import java.util.Objects;
+
 public class AccountExternalIdInfo
     implements Comparable<AccountExternalIdInfo> {
   public String identity;
@@ -30,4 +32,16 @@ public class AccountExternalIdInfo
         .compare(a.emailAddress, emailAddress)
         .result();
   }
+
+  @Override
+  public boolean equals(Object o) {
+    if (o instanceof  AccountExternalIdInfo) {
+      AccountExternalIdInfo a = (AccountExternalIdInfo) o;
+      return (Objects.equals(a.identity, identity))
+          && (Objects.equals(a.emailAddress, emailAddress))
+          && (Objects.equals(a.trusted, trusted))
+          && (Objects.equals(a.canDelete, canDelete));
+    }
+    return false;
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/account/GetExternalIds.java

@@ -75,7 +75,7 @@ public class GetExternalIds implements RestReadView<AccountResource> {
         AccountExternalId.Key last = resource.getUser()
             .getLastLoginExternalIdKey();
         info.canDelete =
-            toBoolean(last != null && !last.get().equals(info.identity));
+            toBoolean(last == null || !last.get().equals(info.identity));
       }
       result.add(info);
     }