From a2ffc209a44456445d2427531a0a4e5f452c27d3 Mon Sep 17 00:00:00 2001 From: Changcheng Xiao Date: Mon, 16 Jan 2017 16:09:23 +0100 Subject: [PATCH] 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 --- .../acceptance/api/accounts/AccountIT.java | 31 --------- .../acceptance/rest/account/ExternalIdIT.java | 65 +++++++++++++++++++ .../common/AccountExternalIdInfo.java | 14 ++++ .../gerrit/server/account/GetExternalIds.java | 2 +- 4 files changed, 80 insertions(+), 32 deletions(-) create mode 100644 gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java index ea074ad9ae..cb828026c6 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/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 expectedIdInfoList = getExternalIds(user) - .stream().map(AccountIT::toInfo).sorted().collect(Collectors.toList()); - - RestResponse response = userRestSession.get("/accounts/self/external.ids"); - response.assertOK(); - List externalIdInfoList = - newGson().fromJson(response.getReader(), - new TypeToken>() {}.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 sshKeys) { int seq = 1; for (SshKeyInfo key : sshKeys) { diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java new file mode 100644 index 0000000000..f7f6409b58 --- /dev/null +++ b/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 expectedIds = + accountCache.get(user.getId()).getExternalIds(); + + List 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 results = + newGson().fromJson(response.getReader(), + new TypeToken>() {}.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; + } +} diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountExternalIdInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountExternalIdInfo.java index 38ea3b8312..39f4847896 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountExternalIdInfo.java +++ b/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 { 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; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetExternalIds.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetExternalIds.java index 2aecf6c4c5..5120c67579 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetExternalIds.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetExternalIds.java @@ -75,7 +75,7 @@ public class GetExternalIds implements RestReadView { 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); }