Merge "Fix addition of groups as reviewer."
diff --git a/Documentation/licenses.txt b/Documentation/licenses.txt
index 2d5ab1d..275006f9 100644
--- a/Documentation/licenses.txt
+++ b/Documentation/licenses.txt
@@ -1082,6 +1082,7 @@
[[bouncycastle]]
bouncycastle
+* bouncycastle:bcpg
* bouncycastle:bcpg-neverlink
* bouncycastle:bcpkix-neverlink
* bouncycastle:bcprov-neverlink
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 7646777..64bdce0 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -186,6 +186,27 @@
}
----
+[[delete-account]]
+=== Delete Account
+--
+'DELETE /accounts/link:#account-id[\{account-id\}]'
+--
+
+Deletes the given account.
+
+Currently only supporting self deletion (regardless of the way
+link:#account-id[\{account-id\}] is provided).
+
+.Request
+----
+ DELETE /accounts/self HTTP/1.0
+----
+
+.Response
+----
+ HTTP/1.1 204 No Content
+----
+
[[get-detail]]
=== Get Account Details
--
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index e93c152..28ef0ed 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -293,6 +293,7 @@
protected TestAccount admin;
protected TestAccount user;
protected TestRepository<InMemoryRepository> testRepo;
+ protected String testMethodName;
protected String resourcePrefix;
protected Description description;
protected GerritServer.Description testMethodDescription;
@@ -479,9 +480,10 @@
initSsh();
+ testMethodName = description.getMethodName();
resourcePrefix =
UNSAFE_PROJECT_NAME
- .matcher(description.getClassName() + "_" + description.getMethodName() + "_")
+ .matcher(description.getClassName() + "_" + testMethodName + "_")
.replaceAll("");
Context ctx = newRequestContext(admin);
diff --git a/java/com/google/gerrit/acceptance/AccountCreator.java b/java/com/google/gerrit/acceptance/AccountCreator.java
index ff5bc00..310d141 100644
--- a/java/com/google/gerrit/acceptance/AccountCreator.java
+++ b/java/com/google/gerrit/acceptance/AccountCreator.java
@@ -166,6 +166,10 @@
accounts.get(username), () -> String.format("No TestAccount created for %s ", username));
}
+ public void evict(Account.Id id) {
+ evict(ImmutableSet.of(id));
+ }
+
public void evict(Collection<Account.Id> ids) {
accounts.values().removeIf(a -> ids.contains(a.id()));
}
diff --git a/java/com/google/gerrit/extensions/api/accounts/AccountApi.java b/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
index 9c9c282..0d019aa 100644
--- a/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
+++ b/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
@@ -124,6 +124,8 @@
*/
String setHttpPassword(String httpPassword) throws RestApiException;
+ void delete() throws RestApiException;
+
/**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
@@ -327,5 +329,10 @@
public String setHttpPassword(String httpPassword) throws RestApiException {
throw new NotImplementedException();
}
+
+ @Override
+ public void delete() throws RestApiException {
+ throw new NotImplementedException();
+ }
}
}
diff --git a/java/com/google/gerrit/git/BUILD b/java/com/google/gerrit/git/BUILD
index 0574716..98dacfa 100644
--- a/java/com/google/gerrit/git/BUILD
+++ b/java/com/google/gerrit/git/BUILD
@@ -10,5 +10,6 @@
"//lib:jgit",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
+ "//lib/errorprone:annotations",
],
)
diff --git a/java/com/google/gerrit/git/GitUpdateFailureException.java b/java/com/google/gerrit/git/GitUpdateFailureException.java
index 7fcb828..339339c 100644
--- a/java/com/google/gerrit/git/GitUpdateFailureException.java
+++ b/java/com/google/gerrit/git/GitUpdateFailureException.java
@@ -46,6 +46,11 @@
.collect(toImmutableList());
}
+ protected GitUpdateFailureException(String message, Throwable cause) {
+ super(message, cause);
+ this.failures = ImmutableList.of();
+ }
+
/** Returns the names of the refs for which the update failed. */
public ImmutableList<String> getFailedRefs() {
return failures.stream().map(GitUpdateFailure::ref).collect(toImmutableList());
diff --git a/java/com/google/gerrit/git/LockFailureException.java b/java/com/google/gerrit/git/LockFailureException.java
index 371488d..2908db2 100644
--- a/java/com/google/gerrit/git/LockFailureException.java
+++ b/java/com/google/gerrit/git/LockFailureException.java
@@ -14,6 +14,7 @@
package com.google.gerrit.git;
+import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.RefUpdate;
@@ -21,6 +22,9 @@
public class LockFailureException extends GitUpdateFailureException {
private static final long serialVersionUID = 1L;
+ private static final String REF_UPDATE_RETURN_CODE_WAS_LOCK_FAILURE =
+ "RefUpdate return code was: LOCK_FAILURE";
+
public LockFailureException(String message, RefUpdate refUpdate) {
super(message, refUpdate);
}
@@ -28,4 +32,15 @@
public LockFailureException(String message, BatchRefUpdate batchRefUpdate) {
super(message, batchRefUpdate);
}
+
+ protected LockFailureException(String message, Throwable cause) {
+ super(message, cause);
+ }
+
+ public static void throwIfLockFailure(ConcurrentRefUpdateException e)
+ throws LockFailureException {
+ if (e.getMessage().contains(REF_UPDATE_RETURN_CODE_WAS_LOCK_FAILURE)) {
+ throw new LockFailureException(e.getMessage(), e);
+ }
+ }
}
diff --git a/java/com/google/gerrit/git/RefUpdateUtil.java b/java/com/google/gerrit/git/RefUpdateUtil.java
index bd88962..c2db073 100644
--- a/java/com/google/gerrit/git/RefUpdateUtil.java
+++ b/java/com/google/gerrit/git/RefUpdateUtil.java
@@ -15,6 +15,7 @@
package com.google.gerrit.git;
import com.google.common.annotations.VisibleForTesting;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.io.IOException;
import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.lib.BatchRefUpdate;
@@ -147,18 +148,19 @@
* occurs.
* @throws IOException if an error occurred.
*/
- public static void deleteChecked(Repository repo, String refName) throws IOException {
+ @CanIgnoreReturnValue
+ public static RefUpdate deleteChecked(Repository repo, String refName) throws IOException {
RefUpdate ru = repo.updateRef(refName);
ru.setForceUpdate(true);
ru.setCheckConflicting(false);
switch (ru.delete()) {
case FORCED:
// Ref was deleted.
- return;
+ return ru;
case NEW:
// Ref didn't exist (yes, really).
- return;
+ return ru;
case LOCK_FAILURE:
throw new LockFailureException("Failed to delete " + refName + ": " + ru.getResult(), ru);
diff --git a/java/com/google/gerrit/gpg/BUILD b/java/com/google/gerrit/gpg/BUILD
index fcf4f0f..3958821 100644
--- a/java/com/google/gerrit/gpg/BUILD
+++ b/java/com/google/gerrit/gpg/BUILD
@@ -4,6 +4,9 @@
name = "gpg",
srcs = glob(["**/*.java"]),
visibility = ["//visibility:public"],
+ runtime_deps = [
+ "//lib/bouncycastle:bcpg",
+ ],
deps = [
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/entities",
diff --git a/java/com/google/gerrit/gpg/PublicKeyStoreUtil.java b/java/com/google/gerrit/gpg/PublicKeyStoreUtil.java
index 2c1cae6..1140bcd 100644
--- a/java/com/google/gerrit/gpg/PublicKeyStoreUtil.java
+++ b/java/com/google/gerrit/gpg/PublicKeyStoreUtil.java
@@ -107,4 +107,9 @@
}
return res;
}
+
+ public List<RefUpdate.Result> deleteAllPgpKeysForUser(
+ Account.Id id, PersonIdent committer, PersonIdent author) throws PGPException, IOException {
+ return deletePgpKeys(listGpgKeysForUser(id), committer, author);
+ }
}
diff --git a/java/com/google/gerrit/gpg/SignedPushModule.java b/java/com/google/gerrit/gpg/SignedPushModule.java
index f4fb9f9..98487ca 100644
--- a/java/com/google/gerrit/gpg/SignedPushModule.java
+++ b/java/com/google/gerrit/gpg/SignedPushModule.java
@@ -33,6 +33,7 @@
import com.google.inject.Provider;
import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
+import com.google.inject.multibindings.OptionalBinder;
import java.io.IOException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
@@ -54,7 +55,12 @@
if (!BouncyCastleUtil.havePGP()) {
throw new ProvisionException("Bouncy Castle PGP not installed");
}
- bind(PublicKeyStore.class).toProvider(StoreProvider.class);
+ // This binding is optional as some modules might bind
+ // {@code UnimplementedPublicKeyStoreProvider} as default binding.
+ OptionalBinder.newOptionalBinder(binder(), PublicKeyStore.class)
+ .setBinding()
+ .toProvider(StoreProvider.class);
+
DynamicSet.bind(binder(), ReceivePackInitializer.class).to(Initializer.class);
}
diff --git a/java/com/google/gerrit/gpg/UnimplementedPublicKeyStoreProvider.java b/java/com/google/gerrit/gpg/UnimplementedPublicKeyStoreProvider.java
new file mode 100644
index 0000000..12e8edb
--- /dev/null
+++ b/java/com/google/gerrit/gpg/UnimplementedPublicKeyStoreProvider.java
@@ -0,0 +1,27 @@
+// Copyright (C) 2023 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.gpg;
+
+import com.google.gerrit.extensions.restapi.NotImplementedException;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+@Singleton
+public class UnimplementedPublicKeyStoreProvider implements Provider<PublicKeyStore> {
+ @Override
+ public PublicKeyStore get() {
+ throw new NotImplementedException("UnimplementedPublicKeyStoreProvider was bound.");
+ }
+}
diff --git a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
index 36fa61b..402e48a 100644
--- a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
@@ -87,8 +87,7 @@
ImmutableSet.of(
ListChangesOption.LABELS,
ListChangesOption.DETAILED_ACCOUNTS,
- ListChangesOption.SUBMIT_REQUIREMENTS,
- ListChangesOption.STAR);
+ ListChangesOption.SUBMIT_REQUIREMENTS);
public static final ImmutableSet<ListChangesOption> CHANGE_DETAIL_OPTIONS =
ImmutableSet.of(
diff --git a/java/com/google/gerrit/server/StarredChangesUtil.java b/java/com/google/gerrit/server/StarredChangesUtil.java
index cf04029..8b41356 100644
--- a/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -294,6 +294,10 @@
}
public ImmutableSet<Change.Id> byAccountId(Account.Id accountId) {
+ return byAccountId(accountId, true);
+ }
+
+ public ImmutableSet<Change.Id> byAccountId(Account.Id accountId, boolean skipInvalidChanges) {
try (Repository repo = repoManager.openRepository(allUsers)) {
ImmutableSet.Builder<Change.Id> builder = ImmutableSet.builder();
for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_STARRED_CHANGES)) {
@@ -310,7 +314,7 @@
// Skip invalid change ids.
Change.Id changeId = Change.Id.fromAllUsersRef(ref.getName());
- if (changeId == null) {
+ if (skipInvalidChanges && changeId == null) {
continue;
}
builder.add(changeId);
diff --git a/java/com/google/gerrit/server/account/AccountDelta.java b/java/com/google/gerrit/server/account/AccountDelta.java
index 8f285b5..f074522 100644
--- a/java/com/google/gerrit/server/account/AccountDelta.java
+++ b/java/com/google/gerrit/server/account/AccountDelta.java
@@ -18,6 +18,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.NotifyConfig.NotifyType;
@@ -161,6 +162,15 @@
*/
public abstract Optional<EditPreferencesInfo> getEditPreferences();
+ /**
+ * Returns whether the delta for this account is deleting the account.
+ *
+ * <p>If set to true, deletion takes precedence on any other change in this delta.
+ *
+ * @return whether the account should be deleted.
+ */
+ public abstract Optional<Boolean> getShouldDeleteAccount();
+
public boolean hasExternalIdUpdates() {
return !this.getCreatedExternalIds().isEmpty()
|| !this.getDeletedExternalIds().isEmpty()
@@ -182,6 +192,7 @@
*
* @param fullName the new full name, if {@code null} or empty string the full name is unset
*/
+ @CanIgnoreReturnValue
public abstract Builder setFullName(@Nullable String fullName);
/**
@@ -190,6 +201,7 @@
* @param displayName the new display name, if {@code null} or empty string the display name is
* unset
*/
+ @CanIgnoreReturnValue
public abstract Builder setDisplayName(@Nullable String displayName);
/**
@@ -198,6 +210,7 @@
* @param preferredEmail the new preferred email, if {@code null} or empty string the preferred
* email is unset
*/
+ @CanIgnoreReturnValue
public abstract Builder setPreferredEmail(@Nullable String preferredEmail);
/**
@@ -206,6 +219,7 @@
* @param active {@code true} if the account should be set to active, {@code false} if the
* account should be set to inactive
*/
+ @CanIgnoreReturnValue
public abstract Builder setActive(boolean active);
/**
@@ -213,6 +227,7 @@
*
* @param status the new status, if {@code null} or empty string the status is unset
*/
+ @CanIgnoreReturnValue
public abstract Builder setStatus(@Nullable String status);
/**
@@ -234,6 +249,7 @@
* @param extId external ID that should be added
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder addExternalId(ExternalId extId) {
return addExternalIds(ImmutableSet.of(extId));
}
@@ -250,6 +266,7 @@
* @param extIds external IDs that should be added
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder addExternalIds(Collection<ExternalId> extIds) {
createdExternalIdsBuilder().addAll(extIds);
return this;
@@ -273,6 +290,7 @@
* @param extId external ID that should be updated
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder updateExternalId(ExternalId extId) {
return updateExternalIds(ImmutableSet.of(extId));
}
@@ -289,6 +307,7 @@
* @param extIds external IDs that should be updated
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder updateExternalIds(Collection<ExternalId> extIds) {
updatedExternalIdsBuilder().addAll(extIds);
return this;
@@ -312,6 +331,7 @@
* @param extId external ID that should be deleted
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder deleteExternalId(ExternalId extId) {
return deleteExternalIds(ImmutableSet.of(extId));
}
@@ -327,6 +347,7 @@
* @param extIds external IDs that should be deleted
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder deleteExternalIds(Collection<ExternalId> extIds) {
deletedExternalIdsBuilder().addAll(extIds);
return this;
@@ -339,6 +360,7 @@
* @param extIdToAdd external ID that should be added
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder replaceExternalId(ExternalId extIdToDelete, ExternalId extIdToAdd) {
return replaceExternalIds(ImmutableSet.of(extIdToDelete), ImmutableSet.of(extIdToAdd));
}
@@ -350,6 +372,7 @@
* @param extIdsToAdd external IDs that should be added
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder replaceExternalIds(
Collection<ExternalId> extIdsToDelete, Collection<ExternalId> extIdsToAdd) {
return deleteExternalIds(extIdsToDelete).addExternalIds(extIdsToAdd);
@@ -371,6 +394,7 @@
* @param notifyTypes the notify types that should be set for the project watch
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder updateProjectWatch(
ProjectWatchKey projectWatchKey, Set<NotifyType> notifyTypes) {
return updateProjectWatches(ImmutableMap.of(projectWatchKey, notifyTypes));
@@ -385,6 +409,7 @@
* @param projectWatches project watches that should be updated
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder updateProjectWatches(Map<ProjectWatchKey, Set<NotifyType>> projectWatches) {
updatedProjectWatchesBuilder().putAll(projectWatches);
return this;
@@ -405,6 +430,7 @@
* @param projectWatch project watch that should be deleted
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder deleteProjectWatch(ProjectWatchKey projectWatch) {
return deleteProjectWatches(ImmutableSet.of(projectWatch));
}
@@ -417,6 +443,7 @@
* @param projectWatches project watches that should be deleted
* @return the builder
*/
+ @CanIgnoreReturnValue
public Builder deleteProjectWatches(Collection<ProjectWatchKey> projectWatches) {
deletedProjectWatchesBuilder().addAll(projectWatches);
return this;
@@ -430,6 +457,7 @@
* @param generalPreferences the general preferences that should be set
* @return the builder
*/
+ @CanIgnoreReturnValue
public abstract Builder setGeneralPreferences(GeneralPreferencesInfo generalPreferences);
/**
@@ -440,6 +468,7 @@
* @param diffPreferences the diff preferences that should be set
* @return the builder
*/
+ @CanIgnoreReturnValue
public abstract Builder setDiffPreferences(DiffPreferencesInfo diffPreferences);
/**
@@ -450,8 +479,25 @@
* @param editPreferences the edit preferences that should be set
* @return the builder
*/
+ @CanIgnoreReturnValue
public abstract Builder setEditPreferences(EditPreferencesInfo editPreferences);
+ @CanIgnoreReturnValue
+ public abstract Builder setShouldDeleteAccount(boolean shouldDelete);
+
+ /**
+ * Builds an AccountDelta that deletes all data.
+ *
+ * @param extIdsToDelete external IDs that should be deleted
+ * @return the builder
+ */
+ @CanIgnoreReturnValue
+ public Builder deleteAccount(Collection<ExternalId> extIdsToDelete) {
+ deleteExternalIds(extIdsToDelete);
+ setShouldDeleteAccount(true);
+ return this;
+ }
+
/** Builds the instance. */
public abstract AccountDelta build();
@@ -602,6 +648,12 @@
delegate.setEditPreferences(editPreferences);
return this;
}
+
+ @Override
+ public Builder setShouldDeleteAccount(boolean shouldDelete) {
+ delegate.setShouldDeleteAccount(shouldDelete);
+ return this;
+ }
}
}
}
diff --git a/java/com/google/gerrit/server/account/AccountsUpdate.java b/java/com/google/gerrit/server/account/AccountsUpdate.java
index b706bca..7dd4828 100644
--- a/java/com/google/gerrit/server/account/AccountsUpdate.java
+++ b/java/com/google/gerrit/server/account/AccountsUpdate.java
@@ -25,6 +25,7 @@
import com.google.common.base.Strings;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
@@ -34,6 +35,7 @@
import com.google.gerrit.git.RefUpdateUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdNotes;
import com.google.gerrit.server.account.externalids.ExternalIdNotes.ExternalIdNotesLoader;
import com.google.gerrit.server.account.externalids.ExternalIds;
@@ -53,6 +55,7 @@
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
@@ -62,8 +65,10 @@
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.transport.ReceiveCommand;
/**
* Creates and updates accounts.
@@ -380,6 +385,22 @@
.get(0);
}
+ /**
+ * Deletes all the account state data.
+ *
+ * @param message commit message for the account update, must not be {@code null or empty}
+ * @param accountId ID of the account
+ * @throws IOException if updating the user branch fails due to an IO error
+ * @throws ConfigInvalidException if any of the account fields has an invalid value
+ */
+ public void delete(String message, Account.Id accountId)
+ throws IOException, ConfigInvalidException {
+ ImmutableSet<ExternalId> accountExternalIds = externalIds.byAccount(accountId);
+ Consumer<AccountDelta.Builder> delta =
+ deltaBuilder -> deltaBuilder.deleteAccount(accountExternalIds);
+ update(message, accountId, delta);
+ }
+
private ExecutableUpdate createExecutableUpdate(UpdateArguments updateArguments) {
return repo -> {
AccountConfig accountConfig = read(repo, updateArguments.accountId);
@@ -395,7 +416,7 @@
updateArguments.configureDeltaFromState.configure(accountState.get(), deltaBuilder);
AccountDelta delta = deltaBuilder.build();
- accountConfig.setAccountDelta(delta);
+
ExternalIdNotes.checkSameAccount(
Iterables.concat(
delta.getCreatedExternalIds(),
@@ -415,9 +436,13 @@
externalIdNotes.upsert(delta.getUpdatedExternalIds());
}
+ if (delta.getShouldDeleteAccount().orElse(false)) {
+ return new DeletedAccount(updateArguments.message, accountConfig.getRefName());
+ }
+
+ accountConfig.setAccountDelta(delta);
CachedPreferences cachedDefaultPreferences =
CachedPreferences.fromConfig(VersionedDefaultPreferences.get(repo, allUsersName));
-
return new UpdatedAccount(
updateArguments.message, accountConfig, cachedDefaultPreferences, false);
};
@@ -468,7 +493,8 @@
allUsersRepo,
updatedAccounts.stream().filter(Objects::nonNull).collect(toList()));
for (UpdatedAccount ua : updatedAccounts) {
- accountState.add(ua == null ? Optional.empty() : ua.getAccountState());
+ accountState.add(
+ ua == null || ua.deleted ? Optional.empty() : ua.getAccountState());
}
}
return null;
@@ -514,6 +540,7 @@
beforeCommit.run();
BatchRefUpdate batchRefUpdate = allUsersRepo.getRefDatabase().newBatchUpdate();
+ Set<Account.Id> accountsToSkipForReindex = new HashSet<>();
// External ids may be not updated if:
// * externalIdNotes is not loaded (there were no externalId updates in the delta)
// * new revCommit is identical to the previous externalId tip
@@ -531,7 +558,12 @@
externalIdsUpdated = !Objects.equals(revCommit.getId(), oldExternalIdsRevision);
}
for (UpdatedAccount updatedAccount : updatedAccounts) {
-
+ if (updatedAccount.deleted) {
+ RefUpdate ru = RefUpdateUtil.deleteChecked(allUsersRepo, updatedAccount.refName);
+ gitRefUpdated.fire(allUsersName, ru, ReceiveCommand.Type.DELETE, null);
+ accountsToSkipForReindex.add(Account.Id.fromRef(updatedAccount.refName));
+ continue;
+ }
// These updates are all for different refs (because batches never update the same account
// more than once), so there can be multiple commits in the same batch, all with the same base
// revision in their AccountConfig.
@@ -540,7 +572,7 @@
// when no account properties are set and hence no
// 'account.config' file will be created.
// 2) When updating "refs/meta/external-ids", so that refs/users/* meta ref is updated too.
- // This allows to schedule reindexing of account transactionally on refs/users/* meta
+ // This allows to schedule reindexing of account transactionally on refs/users/* meta
// updates.
boolean allowEmptyCommit = externalIdsUpdated || updatedAccount.created;
commitAccountConfig(
@@ -553,8 +585,8 @@
RefUpdateUtil.executeChecked(batchRefUpdate, allUsersRepo);
- Set<Account.Id> accountsToSkipForReindex = getUpdatedAccountIds(batchRefUpdate);
if (externalIdsUpdated) {
+ accountsToSkipForReindex.addAll(getUpdatedAccountIds(batchRefUpdate));
extIdNotesLoader.updateExternalIdCacheAndMaybeReindexAccounts(
externalIdNotes, accountsToSkipForReindex);
}
@@ -615,18 +647,38 @@
final String message;
final AccountConfig accountConfig;
final CachedPreferences defaultPreferences;
+ final String refName;
final boolean created;
+ final boolean deleted;
UpdatedAccount(
String message,
AccountConfig accountConfig,
CachedPreferences defaultPreferences,
boolean created) {
+ this(
+ message,
+ requireNonNull(accountConfig),
+ defaultPreferences,
+ accountConfig.getRefName(),
+ created,
+ false);
+ }
+
+ protected UpdatedAccount(
+ String message,
+ AccountConfig accountConfig,
+ CachedPreferences defaultPreferences,
+ String refName,
+ boolean created,
+ boolean deleted) {
checkState(!Strings.isNullOrEmpty(message), "message for account update must be set");
this.message = requireNonNull(message);
- this.accountConfig = requireNonNull(accountConfig);
+ this.accountConfig = accountConfig;
this.defaultPreferences = defaultPreferences;
+ this.refName = refName;
this.created = created;
+ this.deleted = deleted;
}
Optional<AccountState> getAccountState() throws IOException {
@@ -634,4 +686,10 @@
externalIds, accountConfig, externalIdNotes, defaultPreferences);
}
}
+
+ private class DeletedAccount extends UpdatedAccount {
+ DeletedAccount(String message, String refName) {
+ super(message, null, null, refName, false, true);
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalId.java b/java/com/google/gerrit/server/account/externalids/ExternalId.java
index 9196db8..ef1781e 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalId.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalId.java
@@ -27,7 +27,6 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.client.AuthType;
-import com.google.gerrit.git.ObjectIds;
import java.io.Serializable;
import java.util.Collection;
import java.util.Locale;
@@ -294,15 +293,6 @@
return key().isScheme(scheme);
}
- public byte[] toByteArray() {
- checkState(blobId() != null, "Missing blobId in external ID %s", key().get());
- byte[] b = new byte[2 * ObjectIds.STR_LEN + 1];
- key().sha1().copyTo(b, 0);
- b[ObjectIds.STR_LEN] = ':';
- blobId().copyTo(b, ObjectIds.STR_LEN + 1);
- return b;
- }
-
/**
* For checking if two external IDs are equals the blobId is excluded and external IDs that have
* different blob IDs but identical other fields are considered equal. This way an external ID
diff --git a/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java b/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
index 828f868..400521b 100644
--- a/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
+++ b/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
@@ -54,6 +54,7 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.restapi.account.AddSshKey;
import com.google.gerrit.server.restapi.account.CreateEmail;
+import com.google.gerrit.server.restapi.account.DeleteAccount;
import com.google.gerrit.server.restapi.account.DeleteActive;
import com.google.gerrit.server.restapi.account.DeleteDraftComments;
import com.google.gerrit.server.restapi.account.DeleteEmail;
@@ -135,6 +136,7 @@
private final EmailApiImpl.Factory emailApi;
private final PutName putName;
private final PutHttpPassword putHttpPassword;
+ private final DeleteAccount deleteAccount;
@Inject
AccountApiImpl(
@@ -176,6 +178,7 @@
EmailApiImpl.Factory emailApi,
PutName putName,
PutHttpPassword putPassword,
+ DeleteAccount deleteAccount,
@Assisted AccountResource account) {
this.account = account;
this.accountLoaderFactory = ailf;
@@ -216,6 +219,7 @@
this.emailApi = emailApi;
this.putName = putName;
this.putHttpPassword = putPassword;
+ this.deleteAccount = deleteAccount;
}
@Override
@@ -604,4 +608,13 @@
throw asRestApiException("Cannot generate HTTP password", e);
}
}
+
+ @Override
+ public void delete() throws RestApiException {
+ try {
+ deleteAccount.apply(account, null);
+ } catch (Exception e) {
+ throw asRestApiException("Cannot delete account " + account.getUser().getNameEmail(), e);
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/change/AccountPatchReviewStore.java b/java/com/google/gerrit/server/change/AccountPatchReviewStore.java
index 1b9008d..dc34095 100644
--- a/java/com/google/gerrit/server/change/AccountPatchReviewStore.java
+++ b/java/com/google/gerrit/server/change/AccountPatchReviewStore.java
@@ -19,6 +19,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.extensions.restapi.NotImplementedException;
import java.util.Collection;
import java.util.Optional;
@@ -91,6 +92,16 @@
void clearReviewed(Change.Id changeId);
/**
+ * Clears the reviewed flags for the given user in all the relevant changes/patch-set/files.
+ *
+ * @param accountId account ID of the user
+ */
+ default void clearReviewedBy(Account.Id accountId) {
+ throw new NotImplementedException(
+ "clearReviewedBy() is not implemented for this AccountPatchReviewStore.");
+ }
+
+ /**
* Find the latest patch set, that is smaller or equals to the given patch set, where at least,
* one file has been reviewed by the given user.
*
diff --git a/java/com/google/gerrit/server/edit/ChangeEdit.java b/java/com/google/gerrit/server/edit/ChangeEdit.java
index c652289..d7a3a11 100644
--- a/java/com/google/gerrit/server/edit/ChangeEdit.java
+++ b/java/com/google/gerrit/server/edit/ChangeEdit.java
@@ -18,6 +18,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
/**
@@ -53,6 +54,10 @@
return editCommit;
}
+ public ObjectId getEditCommitId() {
+ return editCommit.getId();
+ }
+
public PatchSet getBasePatchSet() {
return basePatchSet;
}
diff --git a/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/java/com/google/gerrit/server/edit/ChangeEditUtil.java
index 3594afb..c09dac3 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditUtil.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditUtil.java
@@ -250,7 +250,7 @@
try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
String refName = edit.getRefName();
RefUpdate ru = repo.updateRef(refName, true);
- ru.setExpectedOldObjectId(edit.getEditCommit());
+ ru.setExpectedOldObjectId(edit.getEditCommitId());
ru.setForceUpdate(true);
RefUpdate.Result result = ru.delete();
switch (result) {
diff --git a/java/com/google/gerrit/server/index/account/AccountField.java b/java/com/google/gerrit/server/index/account/AccountField.java
index e675003..f87fbf5 100644
--- a/java/com/google/gerrit/server/index/account/AccountField.java
+++ b/java/com/google/gerrit/server/index/account/AccountField.java
@@ -14,13 +14,16 @@
package com.google.gerrit.server.index.account;
+import static com.google.common.base.Preconditions.checkState;
import static java.util.stream.Collectors.toSet;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.index.IndexedField;
import com.google.gerrit.index.RefState;
import com.google.gerrit.index.SchemaUtil;
@@ -237,12 +240,22 @@
a ->
a.externalIds().stream()
.filter(e -> e.blobId() != null)
- .map(ExternalId::toByteArray)
+ .map(AccountField::serializeExternalId)
.collect(toSet()));
public static final IndexedField<AccountState, Iterable<byte[]>>.SearchSpec
EXTERNAL_ID_STATE_SPEC = EXTERNAL_ID_STATE_FIELD.storedOnly("external_id_state");
+ @VisibleForTesting
+ public static byte[] serializeExternalId(ExternalId extId) {
+ checkState(extId.blobId() != null, "Missing blobId in external ID %s", extId.key().get());
+ byte[] b = new byte[2 * ObjectIds.STR_LEN + 1];
+ extId.key().sha1().copyTo(b, 0);
+ b[ObjectIds.STR_LEN] = ':';
+ extId.blobId().copyTo(b, ObjectIds.STR_LEN + 1);
+ return b;
+ }
+
private static final Set<String> getNameParts(AccountState a, Iterable<String> emails) {
String fullName = a.account().fullName();
Set<String> parts = SchemaUtil.getNameParts(fullName, emails);
diff --git a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index 62c070c..76ee627 100644
--- a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -27,6 +27,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.common.UsedAt;
+import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
@@ -73,6 +74,10 @@
return ChangeStatusPredicate.forStatus(status);
}
+ private static Predicate<ChangeData> editBy(Account.Id accountId) {
+ return ChangePredicates.editBy(accountId);
+ }
+
private static Predicate<ChangeData> commit(String id) {
return ChangePredicates.commitPrefix(id);
}
@@ -210,6 +215,10 @@
return query(and(ChangePredicates.exactTopic(topic), open()));
}
+ public List<ChangeData> byOpenEditByUser(Account.Id accountId) {
+ return query(editBy(accountId));
+ }
+
public List<ChangeData> byCommit(ObjectId id) {
return byCommit(id.name());
}
diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD
index dd0ec78d..a153399 100644
--- a/java/com/google/gerrit/server/restapi/BUILD
+++ b/java/com/google/gerrit/server/restapi/BUILD
@@ -15,6 +15,7 @@
"//java/com/google/gerrit/exceptions",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/git",
+ "//java/com/google/gerrit/gpg",
"//java/com/google/gerrit/index",
"//java/com/google/gerrit/index:query_exception",
"//java/com/google/gerrit/index/project",
diff --git a/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java b/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java
index 2a8f55f..af93b8a 100644
--- a/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java
@@ -23,18 +23,24 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.RestApiModule;
+import com.google.gerrit.gpg.PublicKeyStore;
+import com.google.gerrit.gpg.UnimplementedPublicKeyStoreProvider;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.UserInitiated;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.externalids.ExternalIdNotes;
import com.google.inject.Provides;
+import com.google.inject.multibindings.OptionalBinder;
public class AccountRestApiModule extends RestApiModule {
@Override
protected void configure() {
bind(AccountsCollection.class);
bind(Capabilities.class);
+ OptionalBinder.newOptionalBinder(binder(), PublicKeyStore.class)
+ .setDefault()
+ .toProvider(UnimplementedPublicKeyStoreProvider.class);
DynamicMap.mapOf(binder(), ACCOUNT_KIND);
DynamicMap.mapOf(binder(), CAPABILITY_KIND);
@@ -45,6 +51,7 @@
create(ACCOUNT_KIND).to(CreateAccount.class);
put(ACCOUNT_KIND).to(PutAccount.class);
+ delete(ACCOUNT_KIND).to(DeleteAccount.class);
get(ACCOUNT_KIND).to(GetAccount.class);
get(ACCOUNT_KIND, "detail").to(GetDetail.class);
post(ACCOUNT_KIND, "index").to(Index.class);
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteAccount.java b/java/com/google/gerrit/server/restapi/account/DeleteAccount.java
new file mode 100644
index 0000000..f8faba3d
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/account/DeleteAccount.java
@@ -0,0 +1,194 @@
+// Copyright (C) 2023 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.server.restapi.account;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Table;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.common.Input;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.gpg.PublicKeyStoreUtil;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.StarredChangesUtil;
+import com.google.gerrit.server.UserInitiated;
+import com.google.gerrit.server.account.AccountException;
+import com.google.gerrit.server.account.AccountResource;
+import com.google.gerrit.server.account.AccountSshKey;
+import com.google.gerrit.server.account.AccountsUpdate;
+import com.google.gerrit.server.account.VersionedAuthorizedKeys;
+import com.google.gerrit.server.change.AccountPatchReviewStore;
+import com.google.gerrit.server.edit.ChangeEdit;
+import com.google.gerrit.server.edit.ChangeEditUtil;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.plugincontext.PluginItemContext;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.ssh.SshKeyCache;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.List;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+/**
+ * REST endpoint for deleting an account.
+ *
+ * <p>This REST endpoint handles {@code DELETE /accounts/<account-identifier>} requests. Currently,
+ * only self deletions are allowed.
+ */
+@Singleton
+public class DeleteAccount implements RestModifyView<AccountResource, Input> {
+ private final Provider<CurrentUser> self;
+ private final Provider<PersonIdent> serverIdent;
+ private final Provider<AccountsUpdate> accountsUpdateProvider;
+ private final VersionedAuthorizedKeys.Accessor authorizedKeys;
+ private final SshKeyCache sshKeyCache;
+ private final StarredChangesUtil starredChangesUtil;
+ private final DeleteDraftCommentsUtil deleteDraftCommentsUtil;
+ private final GitRepositoryManager gitManager;
+ private final Provider<InternalChangeQuery> queryProvider;
+ private final ChangeEditUtil changeEditUtil;
+ private final PluginItemContext<AccountPatchReviewStore> accountPatchReviewStore;
+ private final Provider<PublicKeyStoreUtil> publicKeyStoreUtilProvider;
+
+ @Inject
+ public DeleteAccount(
+ Provider<CurrentUser> self,
+ @GerritPersonIdent Provider<PersonIdent> serverIdent,
+ @UserInitiated Provider<AccountsUpdate> accountsUpdateProvider,
+ VersionedAuthorizedKeys.Accessor authorizedKeys,
+ SshKeyCache sshKeyCache,
+ StarredChangesUtil starredChangesUtil,
+ DeleteDraftCommentsUtil deleteDraftCommentsUtil,
+ GitRepositoryManager gitManager,
+ Provider<InternalChangeQuery> queryProvider,
+ ChangeEditUtil changeEditUtil,
+ PluginItemContext<AccountPatchReviewStore> accountPatchReviewStore,
+ Provider<PublicKeyStoreUtil> publicKeyStoreUtilProvider) {
+ this.self = self;
+ this.serverIdent = serverIdent;
+ this.accountsUpdateProvider = accountsUpdateProvider;
+ this.authorizedKeys = authorizedKeys;
+ this.sshKeyCache = sshKeyCache;
+ this.starredChangesUtil = starredChangesUtil;
+ this.deleteDraftCommentsUtil = deleteDraftCommentsUtil;
+ this.gitManager = gitManager;
+ this.queryProvider = queryProvider;
+ this.changeEditUtil = changeEditUtil;
+ this.accountPatchReviewStore = accountPatchReviewStore;
+ this.publicKeyStoreUtilProvider = publicKeyStoreUtilProvider;
+ }
+
+ @Override
+ @CanIgnoreReturnValue
+ public Response<?> apply(AccountResource rsrc, Input unusedInput)
+ throws AuthException, AccountException {
+ IdentifiedUser user = rsrc.getUser();
+ if (!self.get().hasSameAccountId(user)) {
+ throw new AuthException("Delete account is only permitted for self");
+ }
+
+ Account.Id userId = user.getAccountId();
+ try {
+ deletePgpKeys(user);
+ deleteSshKeys(user);
+ deleteStarredChanges(userId);
+ deleteChangeEdits(userId);
+ deleteDraftCommentsUtil.deleteDraftComments(user, null);
+ accountPatchReviewStore.run(a -> a.clearReviewedBy(userId));
+ accountsUpdateProvider
+ .get()
+ .delete("Deleting user through `DELETE /accounts/{ID}`", user.getAccountId());
+ } catch (Exception e) {
+ throw new AccountException("Could not delete account", e);
+ }
+ return Response.none();
+ }
+
+ private void deletePgpKeys(IdentifiedUser user) {
+ if (publicKeyStoreUtilProvider == null || publicKeyStoreUtilProvider.get() == null) {
+ return;
+ }
+ try {
+ PublicKeyStoreUtil storeUtil = publicKeyStoreUtilProvider.get();
+ List<RefUpdate.Result> deletedKeyResults =
+ storeUtil.deleteAllPgpKeysForUser(
+ user.getAccountId(), serverIdent.get(), serverIdent.get());
+ for (RefUpdate.Result saveResult : deletedKeyResults) {
+ if (saveResult != RefUpdate.Result.NO_CHANGE
+ && saveResult != RefUpdate.Result.FAST_FORWARD) {
+ throw new StorageException(String.format("Failed to delete PGP key: %s", saveResult));
+ }
+ }
+ } catch (Exception e) {
+ throw new StorageException("Failed to delete PGP keys.", e);
+ }
+ }
+
+ private void deleteSshKeys(IdentifiedUser user) throws ConfigInvalidException, IOException {
+ List<AccountSshKey> keys = authorizedKeys.getKeys(user.getAccountId());
+ for (AccountSshKey key : keys) {
+ authorizedKeys.deleteKey(user.getAccountId(), key.seq());
+ }
+ user.getUserName().ifPresent(sshKeyCache::evict);
+ }
+
+ private void deleteStarredChanges(Account.Id accountId)
+ throws StarredChangesUtil.IllegalLabelException {
+ ImmutableSet<Change.Id> staredChanges = starredChangesUtil.byAccountId(accountId, false);
+ for (Change.Id change : staredChanges) {
+ starredChangesUtil.star(
+ self.get().getAccountId(), change, StarredChangesUtil.Operation.REMOVE);
+ }
+ }
+
+ private void deleteChangeEdits(Account.Id accountId) throws IOException {
+ // Note: in case of a stale index, the results of this query might be incomplete.
+ List<ChangeData> changesWithEdits = queryProvider.get().byOpenEditByUser(accountId);
+
+ for (ChangeData cd : changesWithEdits) {
+ for (Table.Cell<Account.Id, PatchSet.Id, Ref> edit : cd.editRefs().cellSet()) {
+ if (!accountId.equals(edit.getRowKey())) {
+ continue;
+ }
+ try (Repository repo = gitManager.openRepository(cd.project());
+ RevWalk rw = new RevWalk(repo)) {
+ RevCommit commit = rw.parseCommit(edit.getValue().getObjectId());
+ changeEditUtil.delete(
+ new ChangeEdit(
+ cd.change(),
+ edit.getValue().getName(),
+ commit,
+ cd.patchSet(edit.getColumnKey())));
+ }
+ }
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteDraftCommentsUtil.java b/java/com/google/gerrit/server/restapi/account/DeleteDraftCommentsUtil.java
index 9e02592..875496e 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteDraftCommentsUtil.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteDraftCommentsUtil.java
@@ -19,6 +19,7 @@
import com.google.common.base.CharMatcher;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.HumanComment;
@@ -88,8 +89,9 @@
this.psUtil = psUtil;
}
+ @CanIgnoreReturnValue
public ImmutableList<DeletedDraftCommentInfo> deleteDraftComments(
- IdentifiedUser user, String query) throws RestApiException, UpdateException {
+ IdentifiedUser user, @Nullable String query) throws RestApiException, UpdateException {
CommentJson.HumanCommentFormatter humanCommentFormatter =
commentJsonProvider.get().newHumanCommentFormatter();
Account.Id accountId = user.getAccountId();
diff --git a/java/com/google/gerrit/server/restapi/project/CreateTag.java b/java/com/google/gerrit/server/restapi/project/CreateTag.java
index 34c3ff7..b12ceef 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateTag.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateTag.java
@@ -29,6 +29,7 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionCreateView;
+import com.google.gerrit.git.LockFailureException;
import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -48,6 +49,7 @@
import java.time.ZoneId;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.TagCommand;
+import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
@@ -141,18 +143,23 @@
.newCommitterIdent(TimeUtil.now(), ZoneId.systemDefault()));
}
- Ref result = tag.call();
- tagCache.updateFastForward(
- resource.getNameKey(), ref, ObjectId.zeroId(), result.getObjectId());
- referenceUpdated.fire(
- resource.getNameKey(),
- ref,
- ObjectId.zeroId(),
- result.getObjectId(),
- resource.getUser().asIdentifiedUser().state());
- try (RevWalk w = new RevWalk(repo)) {
- return Response.created(
- ListTags.createTagInfo(perm, result, w, resource.getProjectState(), links));
+ try {
+ Ref result = tag.call();
+ tagCache.updateFastForward(
+ resource.getNameKey(), ref, ObjectId.zeroId(), result.getObjectId());
+ referenceUpdated.fire(
+ resource.getNameKey(),
+ ref,
+ ObjectId.zeroId(),
+ result.getObjectId(),
+ resource.getUser().asIdentifiedUser().state());
+ try (RevWalk w = new RevWalk(repo)) {
+ return Response.created(
+ ListTags.createTagInfo(perm, result, w, resource.getProjectState(), links));
+ }
+ } catch (ConcurrentRefUpdateException e) {
+ LockFailureException.throwIfLockFailure(e);
+ throw e;
}
}
} catch (GitAPIException e) {
diff --git a/java/com/google/gerrit/server/schema/JdbcAccountPatchReviewStore.java b/java/com/google/gerrit/server/schema/JdbcAccountPatchReviewStore.java
index 189d448..35b9893 100644
--- a/java/com/google/gerrit/server/schema/JdbcAccountPatchReviewStore.java
+++ b/java/com/google/gerrit/server/schema/JdbcAccountPatchReviewStore.java
@@ -343,6 +343,22 @@
}
@Override
+ public void clearReviewedBy(Account.Id accountId) {
+ try (TraceTimer ignored =
+ TraceContext.newTimer(
+ "Clear all reviewed flags by user",
+ Metadata.builder().accountId(accountId.get()).build());
+ Connection con = ds.getConnection();
+ PreparedStatement stmt =
+ con.prepareStatement("DELETE FROM account_patch_reviews WHERE account_id = ?")) {
+ stmt.setInt(1, accountId.get());
+ stmt.executeUpdate();
+ } catch (SQLException e) {
+ throw convertError("delete", e);
+ }
+ }
+
+ @Override
public Optional<PatchSetWithReviewedFiles> findReviewed(PatchSet.Id psId, Account.Id accountId) {
try (TraceTimer ignored =
TraceContext.newTimer(
diff --git a/java/com/google/gerrit/testing/FakeAccountPatchReviewStore.java b/java/com/google/gerrit/testing/FakeAccountPatchReviewStore.java
index 1533aeb..0c612f0 100644
--- a/java/com/google/gerrit/testing/FakeAccountPatchReviewStore.java
+++ b/java/com/google/gerrit/testing/FakeAccountPatchReviewStore.java
@@ -117,6 +117,19 @@
}
@Override
+ public void clearReviewedBy(Account.Id accountId) {
+ synchronized (store) {
+ List<Entity> toRemove = new ArrayList<>();
+ for (Entity entity : store) {
+ if (entity.accountId().equals(accountId)) {
+ toRemove.add(entity);
+ }
+ }
+ store.removeAll(toRemove);
+ }
+ }
+
+ @Override
public Optional<PatchSetWithReviewedFiles> findReviewed(PatchSet.Id psId, Account.Id accountId) {
synchronized (store) {
int matchedPsNumber = -1;
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index dd04200..2c65019 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -83,6 +83,7 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.acceptance.testsuite.request.SshSessionFactory;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.entities.AccessSection;
import com.google.gerrit.entities.Account;
@@ -145,6 +146,7 @@
import com.google.gerrit.server.account.externalids.ExternalIdKeyFactory;
import com.google.gerrit.server.account.externalids.ExternalIdNotes;
import com.google.gerrit.server.account.externalids.ExternalIds;
+import com.google.gerrit.server.change.AccountPatchReviewStore;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
@@ -175,6 +177,8 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -255,6 +259,8 @@
@Inject protected GroupOperations groupOperations;
+ @Inject private AccountPatchReviewStore accountPatchReviewStore;
+
private BasicCookieStore httpCookieStore;
private CloseableHttpClient httpclient;
@@ -3224,6 +3230,214 @@
}
}
+ @Test
+ public void deleteAccount_deletesAccountIdentifiers() throws Exception {
+ TestAccount deleted = accountCreator.createValid(testMethodName);
+ String secondaryEmail = "secondary@email.com";
+ gApi.accounts().id(deleted.id().get()).addEmail(newEmailInput(secondaryEmail));
+
+ requestScopeOperations.setApiUser(deleted.id());
+ gApi.accounts().self().delete();
+
+ requestScopeOperations.setApiUser(admin.id());
+ assertThrows(
+ ResourceNotFoundException.class, () -> gApi.accounts().id(deleted.id().get()).get());
+ assertThrows(NoSuchElementException.class, () -> accountCache.get(deleted.id()).get());
+
+ // Verifies the account is not queryable
+ assertThat(gApi.accounts().query(deleted.id().toString()).get()).isEmpty();
+ assertThat(gApi.accounts().query(deleted.username()).get()).isEmpty();
+ assertThat(gApi.accounts().query(deleted.fullName()).get()).isEmpty();
+ assertThat(gApi.accounts().query(deleted.displayName()).get()).isEmpty();
+ assertThat(gApi.accounts().query(deleted.email()).get()).isEmpty();
+ assertThat(gApi.accounts().query(secondaryEmail).get()).isEmpty();
+
+ // Clean up the test framework
+ accountCreator.evict(deleted.id());
+ }
+
+ @Test
+ public void deleteAccount_deletesAccountExternalIds() throws Exception {
+ TestAccount deleted =
+ accountCreator.create("deleted", "deleted@internal.com", "Full Name", "Display");
+ requestScopeOperations.setApiUser(deleted.id());
+ addExternalIdEmail(deleted, "deleted@external.com");
+ assertExternalEmails(
+ deleted.id(), ImmutableSet.of("deleted@internal.com", "deleted@external.com"));
+
+ gApi.accounts().self().delete();
+
+ requestScopeOperations.setApiUser(admin.id());
+ assertThat(externalIds.allByEmail().keySet())
+ .containsNoneOf("deleted@internal.com", "deleted@external.com");
+
+ // Clean up the test framework
+ accountCreator.evict(deleted.id());
+ }
+
+ @Test
+ @UseSsh
+ public void deleteAccount_deletesSshKeys() throws Exception {
+ TestAccount deleted = accountCreator.createValid(testMethodName);
+ requestScopeOperations.setApiUser(deleted.id());
+ String newKey = TestSshKeys.publicKey(SshSessionFactory.genSshKey(), deleted.email());
+ gApi.accounts().self().addSshKey(newKey);
+ assertThat(gApi.accounts().self().listSshKeys()).hasSize(1);
+ assertThat(authorizedKeys.getKeys(deleted.id())).hasSize(1);
+
+ gApi.accounts().self().delete();
+
+ requestScopeOperations.setApiUser(admin.id());
+ assertThat(authorizedKeys.getKeys(deleted.id())).isEmpty();
+
+ // Clean up the test framework
+ accountCreator.evict(deleted.id());
+ }
+
+ @Test
+ public void deleteAccount_deletesGpgKeys() throws Exception {
+ TestAccount deleted = accountCreator.createValid(testMethodName);
+
+ requestScopeOperations.setApiUser(deleted.id());
+ addExternalIdEmail(
+ deleted,
+ PushCertificateIdent.parse(validKeyWithoutExpiration().getFirstUserId()).getEmailAddress());
+ TestKey key = validKeyWithoutExpiration();
+ addGpgKey(deleted, key.getPublicKeyArmored());
+ assertKeys(key);
+ assertIteratorSize(1, getOnlyKeyFromStore(key).getUserIDs());
+
+ gApi.accounts().self().delete();
+
+ requestScopeOperations.setApiUser(admin.id());
+ try (PublicKeyStore store = publicKeyStoreProvider.get()) {
+ Iterable<PGPPublicKeyRing> keys = store.get(key.getKeyId());
+ assertThat(keys).isEmpty();
+ }
+
+ // Clean up the test framework
+ accountCreator.evict(deleted.id());
+ }
+
+ @Test
+ public void deleteAccount_deletesStarredChanges() throws Exception {
+ TestAccount deleted = accountCreator.createValid(testMethodName);
+ PushOneCommit.Result r = createChange();
+ String triplet = project.get() + "~master~" + r.getChangeId();
+
+ requestScopeOperations.setApiUser(deleted.id());
+
+ gApi.accounts().self().starChange(triplet);
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ assertThat(
+ repo.getRefDatabase()
+ .getRefsByPrefix(RefNames.refsStarredChangesPrefix(r.getChange().getId())))
+ .hasSize(1);
+
+ gApi.accounts().self().delete();
+
+ assertThat(
+ repo.getRefDatabase()
+ .getRefsByPrefix(RefNames.refsStarredChangesPrefix(r.getChange().getId())))
+ .isEmpty();
+ }
+
+ // Clean up the test framework
+ accountCreator.evict(deleted.id());
+ }
+
+ @Test
+ public void deleteAccount_deletesChangeEdits() throws Exception {
+ TestAccount deleted = accountCreator.createValid(testMethodName);
+ PushOneCommit.Result r = createChange();
+
+ requestScopeOperations.setApiUser(deleted.id());
+
+ gApi.changes().id(r.getChangeId()).edit().create();
+ gApi.changes()
+ .id(r.getChangeId())
+ .edit()
+ .modifyFile(PushOneCommit.FILE_NAME, RawInputUtil.create("foo".getBytes(UTF_8)));
+ try (Repository repo = repoManager.openRepository(r.getChange().change().getProject())) {
+ assertThat(repo.getRefDatabase().getRefsByPrefix(RefNames.refsEditPrefix(deleted.id())))
+ .hasSize(1);
+
+ gApi.accounts().self().delete();
+
+ assertThat(repo.getRefDatabase().getRefsByPrefix(RefNames.refsEditPrefix(deleted.id())))
+ .isEmpty();
+ }
+
+ // Clean up the test framework
+ accountCreator.evict(deleted.id());
+ }
+
+ @Test
+ public void deleteAccount_deletesDraftComments() throws Exception {
+ TestAccount deleted = accountCreator.createValid(testMethodName);
+ PushOneCommit.Result r = createChange();
+
+ requestScopeOperations.setApiUser(deleted.id());
+
+ createDraft(r, PushOneCommit.FILE_NAME, "draft");
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ assertThat(
+ repo.getRefDatabase()
+ .getRefsByPrefix(RefNames.refsDraftCommentsPrefix(r.getChange().getId())))
+ .hasSize(1);
+
+ gApi.accounts().self().delete();
+
+ assertThat(
+ repo.getRefDatabase()
+ .getRefsByPrefix(RefNames.refsDraftCommentsPrefix(r.getChange().getId())))
+ .isEmpty();
+ }
+
+ // Clean up the test framework
+ accountCreator.evict(deleted.id());
+ }
+
+ @Test
+ public void deleteAccount_deletesReviewedFlags() throws Exception {
+ PushOneCommit.Result r = createChange();
+ TestAccount deleted = accountCreator.createValid(testMethodName);
+ ReviewerInput in = new ReviewerInput();
+ in.reviewer = deleted.email();
+ gApi.changes().id(r.getChangeId()).addReviewer(in);
+
+ requestScopeOperations.setApiUser(deleted.id());
+
+ accountPatchReviewStore.markReviewed(r.getPatchSetId(), deleted.id(), PushOneCommit.FILE_NAME);
+ assertThat(accountPatchReviewStore.findReviewed(r.getPatchSetId(), deleted.id())).isPresent();
+
+ gApi.accounts().self().delete();
+
+ assertThat(accountPatchReviewStore.findReviewed(r.getPatchSetId(), deleted.id())).isEmpty();
+
+ // Clean up the test framework
+ accountCreator.evict(deleted.id());
+ }
+
+ @Test
+ public void deleteAccount_appliesForSelfById() throws Exception {
+ TestAccount deleted = accountCreator.createValid(testMethodName);
+ requestScopeOperations.setApiUser(deleted.id());
+ gApi.accounts().id(deleted.id().get()).delete();
+
+ // Clean up the test framework
+ accountCreator.evict(deleted.id());
+ }
+
+ @Test
+ public void deleteAccount_throwsForOtherUsers() throws Exception {
+ TestAccount deleted = accountCreator.createValid(testMethodName);
+ requestScopeOperations.setApiUser(user.id());
+ AuthException thrown =
+ assertThrows(AuthException.class, () -> gApi.accounts().id(deleted.id().get()).delete());
+ assertThat(thrown).hasMessageThat().isEqualTo("Delete account is only permitted for self");
+ }
+
private TestGroupBackend createTestGroupBackendWithAllUsersGroup(String nameOfAllUsersGroup)
throws IOException {
TestGroupBackend testGroupBackend = new TestGroupBackend();
@@ -3281,6 +3495,16 @@
.isEqualTo(extIds);
}
+ private void assertExternalEmails(Account.Id accountId, ImmutableSet<String> extIds)
+ throws Exception {
+ assertThat(
+ gApi.accounts().id(accountId.get()).getExternalIds().stream()
+ .map(e -> e.emailAddress)
+ .filter(Objects::nonNull)
+ .collect(toImmutableSet()))
+ .isEqualTo(extIds);
+ }
+
private static Correspondence<GroupInfo, String> getGroupToNameCorrespondence() {
return NullAwareCorrespondence.transforming(groupInfo -> groupInfo.name, "has name");
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/AccountsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/AccountsRestApiBindingsIT.java
index 13353bd..eed4d63 100644
--- a/javatests/com/google/gerrit/acceptance/rest/binding/AccountsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/AccountsRestApiBindingsIT.java
@@ -97,7 +97,9 @@
RestCall.get("/accounts/%s/capabilities"),
RestCall.get("/accounts/%s/capabilities/viewPlugins"),
RestCall.get("/accounts/%s/gpgkeys"),
- RestCall.post("/accounts/%s/gpgkeys"));
+ RestCall.post("/accounts/%s/gpgkeys"),
+ // Account deletion must be the last tested endpoint
+ RestCall.delete("/accounts/%s"));
/**
* Email REST endpoints to be tested, each URL contains a placeholders for the account and email
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index aea4b95..c530c79 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -705,7 +705,7 @@
for (AccountExternalIdInfo info : externalIdInfos) {
Optional<ExternalId> extId = externalIds.get(externalIdKeyFactory.parse(info.identity));
assertThat(extId).isPresent();
- blobs.add(new ByteArrayWrapper(extId.get().toByteArray()));
+ blobs.add(new ByteArrayWrapper(AccountField.serializeExternalId(extId.get())));
}
// Some installations do not store EXTERNAL_ID_STATE_SPEC
diff --git a/javatests/com/google/gerrit/server/query/account/BUILD b/javatests/com/google/gerrit/server/query/account/BUILD
index c781d8b..2c31aef 100644
--- a/javatests/com/google/gerrit/server/query/account/BUILD
+++ b/javatests/com/google/gerrit/server/query/account/BUILD
@@ -16,6 +16,7 @@
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/git",
"//java/com/google/gerrit/index",
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/server",
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index 158799d..1d2b39e 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -60,7 +60,6 @@
import {Subject} from 'rxjs';
import {debounceTime} from 'rxjs/operators';
import {changeModelToken} from '../../../models/change/change-model';
-import {KnownExperimentId} from '../../../services/flags/flags';
import {isBase64FileContent} from '../../../api/rest-api';
import {createDiffUrl} from '../../../models/views/change';
import {userModelToken} from '../../../models/user/user-model';
@@ -207,8 +206,6 @@
private readonly reporting = getAppContext().reportingService;
- private readonly flagsService = getAppContext().flagsService;
-
private readonly getChangeModel = resolve(this, changeModelToken);
private readonly getCommentsModel = resolve(this, commentsModelToken);
@@ -777,9 +774,6 @@
}
private renderSuggestEditButton() {
- if (!this.flagsService.isEnabled(KnownExperimentId.SUGGEST_EDIT)) {
- return nothing;
- }
if (
!this.editing ||
this.permanentEditingMode ||
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 59485e2..607d0f2 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -378,6 +378,17 @@
</gr-tooltip-content>
</div>
<div class="headerMiddle"></div>
+ <gr-button
+ aria-disabled="false"
+ class="action suggestEdit"
+ link=""
+ role="button"
+ tabindex="0"
+ title="This button copies the text to make a suggestion"
+ >
+ <gr-icon filled="" icon="edit" id="icon"> </gr-icon>
+ Suggest edit
+ </gr-button>
<span class="patchset-text">Patchset 1</span>
<span class="separator"></span>
<span class="date" tabindex="0">
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index 405409d..9f3d79b 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -19,8 +19,6 @@
import {linkifyUrlsAndApplyRewrite} from '../../../utils/link-util';
import '../gr-account-chip/gr-account-chip';
import '../gr-user-suggestion-fix/gr-user-suggestion-fix';
-import {KnownExperimentId} from '../../../services/flags/flags';
-import {getAppContext} from '../../../services/app-context';
import {
getUserSuggestionFromString,
USER_SUGGESTION_INFO_STRING,
@@ -42,8 +40,6 @@
@state()
private repoCommentLinks: CommentLinks = {};
- private readonly flagsService = getAppContext().flagsService;
-
private readonly getConfigModel = resolve(this, configModelToken);
// Private const but used in tests.
@@ -158,10 +154,6 @@
}
private renderAsMarkdown() {
- // Need to find out here, since customRender is not arrow function
- const suggestEditsEnable = this.flagsService.isEnabled(
- KnownExperimentId.SUGGEST_EDIT
- );
// Bind `this` via closure.
const boundRewriteText = (text: string) => {
const nonAsteriskRewrites = Object.fromEntries(
@@ -216,7 +208,7 @@
renderer['codespan'] = (text: string) =>
`<code>${unescapeHTML(text)}</code>`;
renderer['code'] = (text: string, infostring: string) => {
- if (suggestEditsEnable && infostring === USER_SUGGESTION_INFO_STRING) {
+ if (infostring === USER_SUGGESTION_INFO_STRING) {
// default santizer in markedjs is very restrictive, we need to use
// existing html element to mark element. We cannot use css class for
// it. Therefore we pick mark - as not frequently used html element to
@@ -275,9 +267,7 @@
override updated() {
// Look for @mentions and replace them with an account-label chip.
this.convertEmailsToAccountChips();
- if (this.flagsService.isEnabled(KnownExperimentId.SUGGEST_EDIT)) {
- this.convertCodeToSuggestions();
- }
+ this.convertCodeToSuggestions();
}
private convertEmailsToAccountChips() {
diff --git a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
index ea67661..95c525a 100644
--- a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
+++ b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
@@ -5,8 +5,6 @@
*/
import {css, html, LitElement, nothing} from 'lit';
import {customElement} from 'lit/decorators.js';
-import {getAppContext} from '../../../services/app-context';
-import {KnownExperimentId} from '../../../services/flags/flags';
import {fire} from '../../../utils/event-util';
declare global {
@@ -23,8 +21,6 @@
@customElement('gr-user-suggestion-fix')
export class GrUserSuggetionFix extends LitElement {
- private readonly flagsService = getAppContext().flagsService;
-
static override get styles() {
return [
css`
@@ -65,9 +61,6 @@
}
override render() {
- if (!this.flagsService.isEnabled(KnownExperimentId.SUGGEST_EDIT)) {
- return nothing;
- }
if (!this.textContent) return nothing;
const code = this.textContent;
return html`<div class="header">
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 7488e79..e55c6d3 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -18,5 +18,4 @@
CHECKS_DEVELOPER = 'UiFeature__checks_developer',
PUSH_NOTIFICATIONS_DEVELOPER = 'UiFeature__push_notifications_developer',
PUSH_NOTIFICATIONS = 'UiFeature__push_notifications',
- SUGGEST_EDIT = 'UiFeature__suggest_edit',
}
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index ceb5e9d..ba6e384 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -1016,12 +1016,6 @@
});
}
- /**
- * Construct the uri to get list of changes.
- *
- * If options is undefined then default options (see _getChangesOptionsHex) is
- * used.
- */
getRequestForGetChanges(
changesPerPage?: number,
query?: string[] | string,
@@ -1050,12 +1044,6 @@
return request;
}
- /**
- * For every query fetches the matching changes.
- *
- * If options is undefined then default options (see _getChangesOptionsHex) is
- * used.
- */
getChangesForMultipleQueries(
changesPerPage?: number,
query?: string[],
@@ -1097,12 +1085,6 @@
});
}
- /**
- * Fetches changes that match the query.
- *
- * If options is undefined then default options (see _getChangesOptionsHex) is
- * used.
- */
getChanges(
changesPerPage?: number,
query?: string,
@@ -1208,7 +1190,6 @@
ListChangesOption.LABELS,
ListChangesOption.DETAILED_ACCOUNTS,
ListChangesOption.SUBMIT_REQUIREMENTS,
- ListChangesOption.STAR,
];
return listChangesOptionsToHex(...options);
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index c51931d..0853941 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -297,3 +297,24 @@
assertNever(suggestion, 'Received an incorrect suggestion');
}
+
+export function getSuggestedReviewerID(suggestion: Suggestion) {
+ if (isAccountSuggestion(suggestion)) {
+ // Reviewer is an account suggestion from getSuggestedAccounts.
+ return suggestion.email;
+ }
+
+ if (isReviewerAccountSuggestion(suggestion)) {
+ // Reviewer is an account suggestion from getChangeSuggestedReviewers.
+ return suggestion.account.email;
+ }
+
+ if (isReviewerGroupSuggestion(suggestion)) {
+ // Reviewer is a group suggestion from getChangeSuggestedReviewers.
+ // Groups are special users in Gerrit that do not have an email associated
+ // with them but instead have a groupID.
+ // Adding a group adds all members of that group as reviewer.
+ return suggestion.group.id;
+ }
+ return '';
+}
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index a62fcf3..7de5e7e 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -90,19 +90,14 @@
// Skip mergeability data.
SKIP_MERGEABLE: 22,
- // Skip diffstat computation that compute the insertions field (number of lines inserted) and
- // deletions field (number of lines deleted)
+ /**
+ * Skip diffstat computation that compute the insertions field (number of lines inserted) and
+ * deletions field (number of lines deleted)
+ */
SKIP_DIFFSTAT: 23,
- // Include the evaluated submit requirements for the caller.
+ /** Include the evaluated submit requirements for the caller. */
SUBMIT_REQUIREMENTS: 24,
-
- // Include custom keyed values.
- CUSTOM_KEYED_VALUES: 25,
-
- // Include the 'starred' field, that is if the change is starred by the
- // current user.
- STAR: 26,
};
export function listChangesOptionsToHex(...args: number[]) {
