Merge "Load default site theme synchronously"
diff --git a/Documentation/config-accounts.txt b/Documentation/config-accounts.txt
index 97f698e..6378632 100644
--- a/Documentation/config-accounts.txt
+++ b/Documentation/config-accounts.txt
@@ -202,9 +202,8 @@
If the value for a preference is the same as the default value for this
preference, it can be omitted in the `preference.config` file.
-Defaults for general and diff preferences that apply for all accounts
-can be configured in the `refs/users/default` branch in the `All-Users`
-repository.
+Defaults for preferences that apply for all accounts can be configured
+in the `refs/users/default` branch in the `All-Users` repository.
[[project-watches]]
=== Project Watches
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 0c30a4b..7b51567 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -306,6 +306,7 @@
* `SKIP_MERGEABLE`: skip the `mergeable` field in
link:#change-info[ChangeInfo]. For fast moving projects, this field must
be recomputed often, which is slow for projects with big trees.
+--
[[submittable]]
--
diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt
index 8aa1f42..cba91bd 100644
--- a/Documentation/rest-api-config.txt
+++ b/Documentation/rest-api-config.txt
@@ -1223,6 +1223,103 @@
}
----
+[[get-edit-preferences]]
+=== Get Default Edit Preferences
+
+--
+'GET /config/server/preferences.edit'
+--
+
+Returns the default edit preferences for the server.
+
+.Request
+----
+ GET /a/config/server/preferences.edit HTTP/1.0
+----
+
+As response a link:rest-api-accounts.html#edit-preferences-info[
+EditPreferencesInfo] is returned.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "tab_size": 8,
+ "line_length": 100,
+ "indent_unit": 2,
+ "cursor_blink_rate": 0,
+ "show_tabs": true,
+ "syntax_highlighting": true,
+ "match_brackets": true,
+ "auto_close_brackets": true,
+ "theme": "DEFAULT",
+ "key_map_type": "DEFAULT"
+ }
+----
+
+[[set-edit-preferences]]
+=== Set Default Edit Preferences
+
+--
+'PUT /config/server/preferences.edit'
+--
+
+Sets the default edit preferences for the server.
+
+The new edit preferences must be provided in the request body as a
+link:rest-api-accounts.html#edit-preferences-input[
+EditPreferencesInput] entity.
+
+To be allowed to set default edit preferences, a user must be a member
+of a group that is granted the
+link:access-control.html#capability_administrateServer[
+Administrate Server] capability.
+
+.Request
+----
+ PUT /a/config/server/preferences.edit HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "tab_size": 8,
+ "line_length": 80,
+ "indent_unit": 2,
+ "cursor_blink_rate": 0,
+ "show_tabs": true,
+ "syntax_highlighting": true,
+ "match_brackets": true,
+ "auto_close_brackets": true,
+ "theme": "DEFAULT",
+ "key_map_type": "DEFAULT"
+ }
+----
+
+As response a link:rest-api-accounts.html#edit-preferences-info[
+EditPreferencesInfo] is returned.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "tab_size": 8,
+ "line_length": 80,
+ "indent_unit": 2,
+ "cursor_blink_rate": 0,
+ "show_tabs": true,
+ "syntax_highlighting": true,
+ "match_brackets": true,
+ "auto_close_brackets": true,
+ "theme": "DEFAULT",
+ "key_map_type": "DEFAULT"
+ }
+----
+
[[ids]]
== IDs
diff --git a/java/com/google/gerrit/extensions/api/config/Server.java b/java/com/google/gerrit/extensions/api/config/Server.java
index ba81698..5ec63af 100644
--- a/java/com/google/gerrit/extensions/api/config/Server.java
+++ b/java/com/google/gerrit/extensions/api/config/Server.java
@@ -15,6 +15,7 @@
package com.google.gerrit.extensions.api.config;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.common.ServerInfo;
import com.google.gerrit.extensions.restapi.NotImplementedException;
@@ -34,6 +35,10 @@
DiffPreferencesInfo setDefaultDiffPreferences(DiffPreferencesInfo in) throws RestApiException;
+ EditPreferencesInfo getDefaultEditPreferences() throws RestApiException;
+
+ EditPreferencesInfo setDefaultEditPreferences(EditPreferencesInfo in) throws RestApiException;
+
ConsistencyCheckInfo checkConsistency(ConsistencyCheckInput in) throws RestApiException;
/**
@@ -74,6 +79,17 @@
}
@Override
+ public EditPreferencesInfo getDefaultEditPreferences() throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
+ public EditPreferencesInfo setDefaultEditPreferences(EditPreferencesInfo in)
+ throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public ConsistencyCheckInfo checkConsistency(ConsistencyCheckInput in) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java b/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
index 853d173..b417f6a 100644
--- a/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
+++ b/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
@@ -103,10 +103,10 @@
res = byUserName(req.getParameter("user_name"));
} else if (req.getParameter("preferred_email") != null) {
- res = byPreferredEmail(req.getParameter("preferred_email"));
+ res = byPreferredEmail(req.getParameter("preferred_email")).orElse(null);
} else if (req.getParameter("account_id") != null) {
- res = byAccountId(req.getParameter("account_id"));
+ res = byAccountId(req.getParameter("account_id")).orElse(null);
} else {
byte[] raw;
@@ -183,11 +183,8 @@
return HtmlDomUtil.toUTF8(doc);
}
- private AuthResult auth(AccountState account) {
- if (account != null) {
- return new AuthResult(account.getAccount().getId(), null, false);
- }
- return null;
+ private Optional<AuthResult> auth(Optional<AccountState> account) {
+ return account.map(a -> new AuthResult(a.getAccount().getId(), null, false));
}
private AuthResult auth(Account.Id account) {
@@ -216,29 +213,29 @@
}
}
- private AuthResult byPreferredEmail(String email) {
+ private Optional<AuthResult> byPreferredEmail(String email) {
try (ReviewDb db = schema.open()) {
Optional<AccountState> match =
queryProvider.get().byPreferredEmail(email).stream().findFirst();
- return match.isPresent() ? auth(match.get()) : null;
+ return auth(match);
} catch (OrmException e) {
getServletContext().log("cannot query database", e);
- return null;
+ return Optional.empty();
}
}
- private AuthResult byAccountId(String idStr) {
+ private Optional<AuthResult> byAccountId(String idStr) {
final Account.Id id;
try {
id = Account.Id.parse(idStr);
} catch (NumberFormatException nfe) {
- return null;
+ return Optional.empty();
}
try {
return auth(accounts.get(id));
} catch (IOException | ConfigInvalidException e) {
getServletContext().log("cannot query database", e);
- return null;
+ return Optional.empty();
}
}
diff --git a/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java b/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
index 497354d..43dddf9 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
@@ -24,7 +24,6 @@
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -176,7 +175,7 @@
ChangeResource rsrc;
try {
rsrc = changes.parse(changeId);
- } catch (ResourceNotFoundException e) {
+ } catch (RestApiException e) {
throw new IOException(e);
}
addProjectOwnersAsReviewers(rsrc);
diff --git a/java/com/google/gerrit/pgm/init/InitAdminUser.java b/java/com/google/gerrit/pgm/init/InitAdminUser.java
index e9f5cd5..1c27f5e 100644
--- a/java/com/google/gerrit/pgm/init/InitAdminUser.java
+++ b/java/com/google/gerrit/pgm/init/InitAdminUser.java
@@ -21,7 +21,6 @@
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.client.AuthType;
-import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.pgm.init.api.AllUsersNameOnInitProvider;
import com.google.gerrit.pgm.init.api.ConsoleUI;
import com.google.gerrit.pgm.init.api.InitFlags;
@@ -45,7 +44,6 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.List;
import java.util.Optional;
import org.apache.commons.validator.routines.EmailValidator;
@@ -151,13 +149,7 @@
authorizedKeys.save("Add SSH key for initial admin user\n");
}
- AccountState as =
- new AccountState(
- new AllUsersName(allUsers.get()),
- a,
- extIds,
- new HashMap<>(),
- GeneralPreferencesInfo.defaults());
+ AccountState as = AccountState.forAccount(new AllUsersName(allUsers.get()), a, extIds);
for (AccountIndex accountIndex : accountIndexCollection.getWriteIndexes()) {
accountIndex.replace(as);
}
diff --git a/java/com/google/gerrit/pgm/init/api/Section.java b/java/com/google/gerrit/pgm/init/api/Section.java
index c1c8745..009e989 100644
--- a/java/com/google/gerrit/pgm/init/api/Section.java
+++ b/java/com/google/gerrit/pgm/init/api/Section.java
@@ -23,6 +23,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.EnumSet;
+import java.util.Objects;
import java.util.Set;
/** Helper to edit a section of the configuration files. */
@@ -100,7 +101,7 @@
if (nullIfDefault && nv.equals(dv)) {
nv = null;
}
- if (!eq(ov, nv)) {
+ if (!Objects.equals(ov, nv)) {
set(name, nv);
}
return nv;
@@ -148,7 +149,7 @@
public String select(final String title, String name, String dv, Set<String> allowedValues) {
final String ov = get(name);
String nv = ui.readString(ov != null ? ov : dv, allowedValues, "%s", title);
- if (!eq(ov, nv)) {
+ if (!Objects.equals(ov, nv)) {
set(name, nv);
}
return nv;
@@ -177,7 +178,7 @@
}
final String nv = ui.password("%s's password", user);
- if (!eq(ov, nv)) {
+ if (!Objects.equals(ov, nv)) {
setSecure(password, nv);
}
return nv;
@@ -195,7 +196,7 @@
}
final String nv = ui.password("%s", prompt);
- if (!eq(ov, nv)) {
+ if (!Objects.equals(ov, nv)) {
setSecure(passwordKey, nv);
}
return nv;
@@ -216,11 +217,4 @@
String getName() {
return section;
}
-
- private static boolean eq(String a, String b) {
- if (a == null && b == null) {
- return true;
- }
- return a != null && a.equals(b);
- }
}
diff --git a/java/com/google/gerrit/server/account/AccountCacheImpl.java b/java/com/google/gerrit/server/account/AccountCacheImpl.java
index e910ffb..57e14d6 100644
--- a/java/com/google/gerrit/server/account/AccountCacheImpl.java
+++ b/java/com/google/gerrit/server/account/AccountCacheImpl.java
@@ -20,7 +20,6 @@
import com.google.common.cache.LoadingCache;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIds;
@@ -34,8 +33,6 @@
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import java.io.IOException;
-import java.util.Collections;
-import java.util.HashMap;
import java.util.Optional;
import java.util.concurrent.ExecutionException;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -131,12 +128,7 @@
private AccountState missing(Account.Id accountId) {
Account account = new Account(accountId, TimeUtil.nowTs());
account.setActive(false);
- return new AccountState(
- allUsersName,
- account,
- Collections.emptySet(),
- new HashMap<>(),
- GeneralPreferencesInfo.defaults());
+ return AccountState.forAccount(allUsersName, account);
}
static class ByIdLoader extends CacheLoader<Account.Id, Optional<AccountState>> {
@@ -149,7 +141,7 @@
@Override
public Optional<AccountState> load(Account.Id who) throws Exception {
- return Optional.ofNullable(accounts.get(who));
+ return accounts.get(who);
}
}
}
diff --git a/java/com/google/gerrit/server/account/AccountConfig.java b/java/com/google/gerrit/server/account/AccountConfig.java
index b20aa0b..d4379a2 100644
--- a/java/com/google/gerrit/server/account/AccountConfig.java
+++ b/java/com/google/gerrit/server/account/AccountConfig.java
@@ -19,7 +19,11 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -33,6 +37,7 @@
import java.io.IOException;
import java.sql.Timestamp;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -157,7 +162,7 @@
*
* @return the project watches of the loaded account
*/
- public Map<ProjectWatchKey, Set<NotifyType>> getProjectWatches() {
+ public ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>> getProjectWatches() {
checkLoaded();
return watchConfig.getProjectWatches();
}
@@ -173,6 +178,26 @@
}
/**
+ * Get the diff preferences of the loaded account.
+ *
+ * @return the diff preferences of the loaded account
+ */
+ public DiffPreferencesInfo getDiffPreferences() {
+ checkLoaded();
+ return prefConfig.getDiffPreferences();
+ }
+
+ /**
+ * Get the edit preferences of the loaded account.
+ *
+ * @return the edit preferences of the loaded account
+ */
+ public EditPreferencesInfo getEditPreferences() {
+ checkLoaded();
+ return prefConfig.getEditPreferences();
+ }
+
+ /**
* Sets the account. This means the loaded account will be overwritten with the given account.
*
* <p>Changing the registration date of an account is not supported.
@@ -237,10 +262,6 @@
Config accountConfig = readConfig(ACCOUNT_CONFIG);
loadedAccount = Optional.of(parse(accountConfig, revision.name()));
- Ref externalIdsRef = repo.exactRef(RefNames.REFS_EXTERNAL_IDS);
- externalIdsRev =
- externalIdsRef != null ? Optional.of(externalIdsRef.getObjectId()) : Optional.empty();
-
watchConfig = new WatchConfig(accountId, readConfig(WatchConfig.WATCH_CONFIG), this);
prefConfig =
@@ -256,7 +277,16 @@
}
} else {
loadedAccount = Optional.empty();
+
+ watchConfig = new WatchConfig(accountId, new Config(), this);
+
+ prefConfig =
+ new PreferencesConfig(
+ accountId, new Config(), PreferencesConfig.readDefaultConfig(repo), this);
}
+
+ Ref externalIdsRef = repo.exactRef(RefNames.REFS_EXTERNAL_IDS);
+ externalIdsRev = Optional.ofNullable(externalIdsRef).map(Ref::getObjectId);
}
private Account parse(Config cfg, String metaId) {
@@ -302,7 +332,7 @@
Config accountConfig = saveAccount();
saveProjectWatches();
- saveGeneralPreferences();
+ savePreferences();
// metaId is set in the commit(MetaDataUpdate) method after the commit is created
loadedAccount = Optional.of(parse(accountConfig, null));
@@ -334,7 +364,8 @@
if (accountUpdate.isPresent()
&& (!accountUpdate.get().getDeletedProjectWatches().isEmpty()
|| !accountUpdate.get().getUpdatedProjectWatches().isEmpty())) {
- Map<ProjectWatchKey, Set<NotifyType>> projectWatches = watchConfig.getProjectWatches();
+ Map<ProjectWatchKey, Set<NotifyType>> projectWatches =
+ new HashMap<>(watchConfig.getProjectWatches());
accountUpdate.get().getDeletedProjectWatches().forEach(pw -> projectWatches.remove(pw));
accountUpdate
.get()
@@ -344,12 +375,20 @@
}
}
- private void saveGeneralPreferences() throws IOException, ConfigInvalidException {
- if (accountUpdate.isPresent() && accountUpdate.get().getGeneralPreferences().isPresent()) {
- saveConfig(
- PreferencesConfig.PREFERENCES_CONFIG,
- prefConfig.saveGeneralPreferences(accountUpdate.get().getGeneralPreferences().get()));
+ private void savePreferences() throws IOException, ConfigInvalidException {
+ if (!accountUpdate.isPresent()
+ || (!accountUpdate.get().getGeneralPreferences().isPresent()
+ && !accountUpdate.get().getDiffPreferences().isPresent()
+ && !accountUpdate.get().getEditPreferences().isPresent())) {
+ return;
}
+
+ saveConfig(
+ PreferencesConfig.PREFERENCES_CONFIG,
+ prefConfig.saveGeneralPreferences(
+ accountUpdate.get().getGeneralPreferences(),
+ accountUpdate.get().getDiffPreferences(),
+ accountUpdate.get().getEditPreferences()));
}
/**
diff --git a/java/com/google/gerrit/server/account/AccountManager.java b/java/com/google/gerrit/server/account/AccountManager.java
index aac515d..8913ac0 100644
--- a/java/com/google/gerrit/server/account/AccountManager.java
+++ b/java/com/google/gerrit/server/account/AccountManager.java
@@ -49,6 +49,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
@@ -225,36 +226,30 @@
if (!realm.allowsEdit(AccountFieldName.FULL_NAME)
&& !Strings.isNullOrEmpty(who.getDisplayName())
- && !eq(user.getAccount().getFullName(), who.getDisplayName())) {
+ && !Objects.equals(user.getAccount().getFullName(), who.getDisplayName())) {
accountUpdates.add(u -> u.setFullName(who.getDisplayName()));
}
if (!realm.allowsEdit(AccountFieldName.USER_NAME)
&& who.getUserName() != null
- && !eq(user.getUserName(), who.getUserName())) {
+ && !Objects.equals(user.getUserName(), who.getUserName())) {
log.warn(
String.format(
"Not changing already set username %s to %s", user.getUserName(), who.getUserName()));
}
if (!accountUpdates.isEmpty()) {
- Account account =
- accountsUpdateFactory
- .create()
- .update(
- "Update Account on Login",
- user.getAccountId(),
- AccountUpdater.joinConsumers(accountUpdates));
- if (account == null) {
- throw new OrmException("Account " + user.getAccountId() + " has been deleted");
- }
+ accountsUpdateFactory
+ .create()
+ .update(
+ "Update Account on Login",
+ user.getAccountId(),
+ AccountUpdater.joinConsumers(accountUpdates))
+ .orElseThrow(
+ () -> new OrmException("Account " + user.getAccountId() + " has been deleted"));
}
}
- private static boolean eq(String a, String b) {
- return (a == null && b == null) || (a != null && a.equals(b));
- }
-
private AuthResult create(ReviewDb db, AuthRequest who)
throws OrmException, AccountException, IOException, ConfigInvalidException {
Account.Id newId = new Account.Id(sequences.nextAccountId());
@@ -266,9 +261,9 @@
boolean isFirstAccount = awaitsFirstAccountCheck.getAndSet(false) && !accounts.hasAnyAccount();
- Account account;
+ AccountState accountState;
try {
- account =
+ accountState =
accountsUpdateFactory
.create()
.insert(
@@ -318,7 +313,7 @@
addGroupMember(db, adminGroupUuid, user);
}
- realm.onCreateAccount(who, account);
+ realm.onCreateAccount(who, accountState.getAccount());
return new AuthResult(newId, extId.key(), true);
}
@@ -378,7 +373,7 @@
(a, u) -> {
u.addExternalId(
ExternalId.createWithEmail(who.getExternalIdKey(), to, who.getEmailAddress()));
- if (who.getEmailAddress() != null && a.getPreferredEmail() == null) {
+ if (who.getEmailAddress() != null && a.getAccount().getPreferredEmail() == null) {
u.setPreferredEmail(who.getEmailAddress());
}
});
@@ -401,23 +396,26 @@
*/
public AuthResult updateLink(Account.Id to, AuthRequest who)
throws OrmException, AccountException, IOException, ConfigInvalidException {
- Collection<ExternalId> filteredExtIdsByScheme =
- externalIds.byAccount(to, who.getExternalIdKey().scheme());
+ accountsUpdateFactory
+ .create()
+ .update(
+ "Delete External IDs on Update Link",
+ to,
+ (a, u) -> {
+ Collection<ExternalId> filteredExtIdsByScheme =
+ a.getExternalIds(who.getExternalIdKey().scheme());
+ if (filteredExtIdsByScheme.isEmpty()) {
+ return;
+ }
- if (!filteredExtIdsByScheme.isEmpty()
- && (filteredExtIdsByScheme.size() > 1
- || !filteredExtIdsByScheme
- .stream()
- .filter(e -> e.key().equals(who.getExternalIdKey()))
- .findAny()
- .isPresent())) {
- accountsUpdateFactory
- .create()
- .update(
- "Delete External IDs on Update Link",
- to,
- u -> u.deleteExternalIds(filteredExtIdsByScheme));
- }
+ if (filteredExtIdsByScheme.size() > 1
+ || !filteredExtIdsByScheme
+ .stream()
+ .anyMatch(e -> e.key().equals(who.getExternalIdKey()))) {
+ u.deleteExternalIds(filteredExtIdsByScheme);
+ }
+ });
+
return link(to, who);
}
@@ -468,8 +466,10 @@
from,
(a, u) -> {
u.deleteExternalIds(extIds);
- if (a.getPreferredEmail() != null
- && extIds.stream().anyMatch(e -> a.getPreferredEmail().equals(e.email()))) {
+ if (a.getAccount().getPreferredEmail() != null
+ && extIds
+ .stream()
+ .anyMatch(e -> a.getAccount().getPreferredEmail().equals(e.email()))) {
u.setPreferredEmail(null);
}
});
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index a1cca06..ae2bee6 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -14,8 +14,10 @@
package com.google.gerrit.server.account;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.stream.Collectors.toSet;
+import com.google.common.collect.Streams;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.OrmException;
@@ -95,18 +97,12 @@
Matcher m = Pattern.compile("^.* \\(([1-9][0-9]*)\\)$").matcher(nameOrEmail);
if (m.matches()) {
Account.Id id = Account.Id.parse(m.group(1));
- if (accounts.get(id) != null) {
- return Collections.singleton(id);
- }
- return Collections.emptySet();
+ return Streams.stream(accounts.get(id)).map(a -> id).collect(toImmutableSet());
}
if (nameOrEmail.matches("^[1-9][0-9]*$")) {
Account.Id id = Account.Id.parse(nameOrEmail);
- if (accounts.get(id) != null) {
- return Collections.singleton(id);
- }
- return Collections.emptySet();
+ return Streams.stream(accounts.get(id)).map(a -> id).collect(toImmutableSet());
}
if (nameOrEmail.matches(Account.USER_NAME_PATTERN)) {
diff --git a/java/com/google/gerrit/server/account/AccountState.java b/java/com/google/gerrit/server/account/AccountState.java
index 6601df7..8bd12c0 100644
--- a/java/com/google/gerrit/server/account/AccountState.java
+++ b/java/com/google/gerrit/server/account/AccountState.java
@@ -14,14 +14,19 @@
package com.google.gerrit.server.account;
-import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAILTO;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
import com.google.common.base.Function;
import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser.PropertyKey;
@@ -29,18 +34,24 @@
import com.google.gerrit.server.account.WatchConfig.NotifyType;
import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdNotes;
+import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.config.AllUsersName;
+import java.io.IOException;
import java.util.Collection;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
+import java.util.Optional;
+import java.util.function.Supplier;
import org.apache.commons.codec.DecoderException;
+import org.eclipse.jgit.lib.ObjectId;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* Superset of all information related to an Account. This includes external IDs, project watches,
* and properties from the account config file. AccountState maps one-to-one to Account.
+ *
+ * <p>Most callers should not construct AccountStates directly but rather lookup accounts via the
+ * account cache (see {@link AccountCache#get(Account.Id)}).
*/
public class AccountState {
private static final Logger logger = LoggerFactory.getLogger(AccountState.class);
@@ -48,25 +59,126 @@
public static final Function<AccountState, Account.Id> ACCOUNT_ID_FUNCTION =
a -> a.getAccount().getId();
+ /**
+ * Creates an AccountState from the given account config.
+ *
+ * @param allUsersName the name of the All-Users repository
+ * @param externalIds class to access external IDs
+ * @param accountConfig the account config, must already be loaded
+ * @return the account state, {@link Optional#empty()} if the account doesn't exist
+ * @throws IOException if accessing the external IDs fails
+ */
+ public static Optional<AccountState> fromAccountConfig(
+ AllUsersName allUsersName, ExternalIds externalIds, AccountConfig accountConfig)
+ throws IOException {
+ return fromAccountConfig(allUsersName, externalIds, accountConfig, null);
+ }
+
+ /**
+ * Creates an AccountState from the given account config.
+ *
+ * <p>If external ID notes are provided the revision of the external IDs branch from which the
+ * external IDs for the account should be loaded is taken from the external ID notes. If external
+ * ID notes are not given the revision of the external IDs branch is taken from the account
+ * config. Updating external IDs is done via {@link ExternalIdNotes} and if external IDs were
+ * updated the revision of the external IDs branch in account config is outdated. Hence after
+ * updating external IDs the external ID notes must be provided.
+ *
+ * @param allUsersName the name of the All-Users repository
+ * @param externalIds class to access external IDs
+ * @param accountConfig the account config, must already be loaded
+ * @param extIdNotes external ID notes, must already be loaded, may be {@code null}
+ * @return the account state, {@link Optional#empty()} if the account doesn't exist
+ * @throws IOException if accessing the external IDs fails
+ */
+ public static Optional<AccountState> fromAccountConfig(
+ AllUsersName allUsersName,
+ ExternalIds externalIds,
+ AccountConfig accountConfig,
+ @Nullable ExternalIdNotes extIdNotes)
+ throws IOException {
+ if (!accountConfig.getLoadedAccount().isPresent()) {
+ return Optional.empty();
+ }
+ Account account = accountConfig.getLoadedAccount().get();
+
+ Optional<ObjectId> extIdsRev =
+ extIdNotes != null
+ ? Optional.ofNullable(extIdNotes.getRevision())
+ : accountConfig.getExternalIdsRev();
+ ImmutableSet<ExternalId> extIds =
+ extIdsRev.isPresent()
+ ? externalIds.byAccount(account.getId(), extIdsRev.get())
+ : ImmutableSet.of();
+
+ return Optional.of(
+ new AccountState(
+ allUsersName,
+ account,
+ extIds,
+ Suppliers.memoize(() -> accountConfig.getProjectWatches()),
+ Suppliers.memoize(() -> accountConfig.getGeneralPreferences()),
+ Suppliers.memoize(() -> accountConfig.getDiffPreferences()),
+ Suppliers.memoize(() -> accountConfig.getEditPreferences())));
+ }
+
+ /**
+ * Creates an AccountState for a given account with no external IDs, no project watches and
+ * default preferences.
+ *
+ * @param allUsersName the name of the All-Users repository
+ * @param account the account
+ * @return the account state
+ */
+ public static AccountState forAccount(AllUsersName allUsersName, Account account) {
+ return forAccount(allUsersName, account, ImmutableSet.of());
+ }
+
+ /**
+ * Creates an AccountState for a given account with no project watches and default preferences.
+ *
+ * @param allUsersName the name of the All-Users repository
+ * @param account the account
+ * @param extIds the external IDs
+ * @return the account state
+ */
+ public static AccountState forAccount(
+ AllUsersName allUsersName, Account account, Collection<ExternalId> extIds) {
+ return new AccountState(
+ allUsersName,
+ account,
+ ImmutableSet.copyOf(extIds),
+ Suppliers.ofInstance(ImmutableMap.of()),
+ Suppliers.ofInstance(GeneralPreferencesInfo.defaults()),
+ Suppliers.ofInstance(DiffPreferencesInfo.defaults()),
+ Suppliers.ofInstance(EditPreferencesInfo.defaults()));
+ }
+
private final AllUsersName allUsersName;
private final Account account;
- private final Collection<ExternalId> externalIds;
- private final Map<ProjectWatchKey, Set<NotifyType>> projectWatches;
- private final GeneralPreferencesInfo generalPreferences;
+ private final ImmutableSet<ExternalId> externalIds;
+ private final Supplier<ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>>> projectWatches;
+ private final Supplier<GeneralPreferencesInfo> generalPreferences;
+ private final Supplier<DiffPreferencesInfo> diffPreferences;
+ private final Supplier<EditPreferencesInfo> editPreferences;
private Cache<IdentifiedUser.PropertyKey<Object>, Object> properties;
- public AccountState(
+ private AccountState(
AllUsersName allUsersName,
Account account,
- Collection<ExternalId> externalIds,
- Map<ProjectWatchKey, Set<NotifyType>> projectWatches,
- GeneralPreferencesInfo generalPreferences) {
+ ImmutableSet<ExternalId> externalIds,
+ Supplier<ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>>> projectWatches,
+ Supplier<GeneralPreferencesInfo> generalPreferences,
+ Supplier<DiffPreferencesInfo> diffPreferences,
+ Supplier<EditPreferencesInfo> editPreferences) {
this.allUsersName = allUsersName;
this.account = account;
this.externalIds = externalIds;
this.projectWatches = projectWatches;
this.generalPreferences = generalPreferences;
- this.account.setUserName(getUserName(externalIds));
+ this.diffPreferences = diffPreferences;
+ this.editPreferences = editPreferences;
+ this.account.setUserName(ExternalId.getUserName(externalIds).orElse(null));
}
public AllUsersName getAllUsersNameForIndexing() {
@@ -112,37 +224,33 @@
}
/** The external identities that identify the account holder. */
- public Collection<ExternalId> getExternalIds() {
+ public ImmutableSet<ExternalId> getExternalIds() {
return externalIds;
}
+ /** The external identities that identify the account holder that match the given scheme. */
+ public ImmutableSet<ExternalId> getExternalIds(String scheme) {
+ return externalIds.stream().filter(e -> e.key().isScheme(scheme)).collect(toImmutableSet());
+ }
+
/** The project watches of the account. */
- public Map<ProjectWatchKey, Set<NotifyType>> getProjectWatches() {
- return projectWatches;
+ public ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>> getProjectWatches() {
+ return projectWatches.get();
}
/** The general preferences of the account. */
public GeneralPreferencesInfo getGeneralPreferences() {
- return generalPreferences;
+ return generalPreferences.get();
}
- public static String getUserName(Collection<ExternalId> ids) {
- for (ExternalId extId : ids) {
- if (extId.isScheme(SCHEME_USERNAME)) {
- return extId.key().id();
- }
- }
- return null;
+ /** The diff preferences of the account. */
+ public DiffPreferencesInfo getDiffPreferences() {
+ return diffPreferences.get();
}
- public static Set<String> getEmails(Collection<ExternalId> ids) {
- Set<String> emails = new HashSet<>();
- for (ExternalId extId : ids) {
- if (extId.isScheme(SCHEME_MAILTO)) {
- emails.add(extId.key().id());
- }
- }
- return emails;
+ /** The edit preferences of the account. */
+ public EditPreferencesInfo getEditPreferences() {
+ return editPreferences.get();
}
/**
diff --git a/java/com/google/gerrit/server/account/Accounts.java b/java/com/google/gerrit/server/account/Accounts.java
index 45831ae..f72c9c3 100644
--- a/java/com/google/gerrit/server/account/Accounts.java
+++ b/java/com/google/gerrit/server/account/Accounts.java
@@ -18,8 +18,6 @@
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
-import com.google.common.collect.ImmutableSet;
-import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.externalids.ExternalIds;
@@ -56,10 +54,10 @@
this.externalIds = externalIds;
}
- @Nullable
- public AccountState get(Account.Id accountId) throws IOException, ConfigInvalidException {
+ public Optional<AccountState> get(Account.Id accountId)
+ throws IOException, ConfigInvalidException {
try (Repository repo = repoManager.openRepository(allUsersName)) {
- return read(repo, accountId).orElse(null);
+ return read(repo, accountId);
}
}
@@ -136,20 +134,8 @@
private Optional<AccountState> read(Repository allUsersRepository, Account.Id accountId)
throws IOException, ConfigInvalidException {
- AccountConfig accountConfig = new AccountConfig(accountId, allUsersRepository).load();
- if (!accountConfig.getLoadedAccount().isPresent()) {
- return Optional.empty();
- }
- Account account = accountConfig.getLoadedAccount().get();
- return Optional.of(
- new AccountState(
- allUsersName,
- account,
- accountConfig.getExternalIdsRev().isPresent()
- ? externalIds.byAccount(accountId, accountConfig.getExternalIdsRev().get())
- : ImmutableSet.of(),
- accountConfig.getProjectWatches(),
- accountConfig.getGeneralPreferences()));
+ return AccountState.fromAccountConfig(
+ allUsersName, externalIds, new AccountConfig(accountId, allUsersRepository).load());
}
public static Stream<Account.Id> readUserRefs(Repository repo) throws IOException {
diff --git a/java/com/google/gerrit/server/account/AccountsUpdate.java b/java/com/google/gerrit/server/account/AccountsUpdate.java
index fb8067c..5221a44 100644
--- a/java/com/google/gerrit/server/account/AccountsUpdate.java
+++ b/java/com/google/gerrit/server/account/AccountsUpdate.java
@@ -28,6 +28,7 @@
import com.google.gerrit.server.IdentifiedUser;
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;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -48,6 +49,7 @@
import java.util.function.Consumer;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
@@ -82,10 +84,10 @@
* <p>Use the provided account only to read the current state of the account. Don't do updates
* to the account. For updates use the provided account update builder.
*
- * @param account the account that is being updated
+ * @param accountState the account that is being updated
* @param update account update builder
*/
- void update(Account account, InternalAccountUpdate.Builder update);
+ void update(AccountState accountState, InternalAccountUpdate.Builder update);
static AccountUpdater join(List<AccountUpdater> updaters) {
return (a, u) -> updaters.stream().forEach(updater -> updater.update(a, u));
@@ -111,6 +113,7 @@
private final GitRepositoryManager repoManager;
private final GitReferenceUpdated gitRefUpdated;
private final AllUsersName allUsersName;
+ private final ExternalIds externalIds;
private final Provider<PersonIdent> serverIdentProvider;
private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
private final RetryHelper retryHelper;
@@ -121,6 +124,7 @@
GitRepositoryManager repoManager,
GitReferenceUpdated gitRefUpdated,
AllUsersName allUsersName,
+ ExternalIds externalIds,
@GerritPersonIdent Provider<PersonIdent> serverIdentProvider,
Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
RetryHelper retryHelper,
@@ -128,6 +132,7 @@
this.repoManager = repoManager;
this.gitRefUpdated = gitRefUpdated;
this.allUsersName = allUsersName;
+ this.externalIds = externalIds;
this.serverIdentProvider = serverIdentProvider;
this.metaDataUpdateInternalFactory = metaDataUpdateInternalFactory;
this.retryHelper = retryHelper;
@@ -141,6 +146,7 @@
gitRefUpdated,
null,
allUsersName,
+ externalIds,
metaDataUpdateInternalFactory,
retryHelper,
extIdNotesFactory,
@@ -163,6 +169,7 @@
private final GitRepositoryManager repoManager;
private final GitReferenceUpdated gitRefUpdated;
private final AllUsersName allUsersName;
+ private final ExternalIds externalIds;
private final Provider<PersonIdent> serverIdentProvider;
private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
private final RetryHelper retryHelper;
@@ -173,6 +180,7 @@
GitRepositoryManager repoManager,
GitReferenceUpdated gitRefUpdated,
AllUsersName allUsersName,
+ ExternalIds externalIds,
@GerritPersonIdent Provider<PersonIdent> serverIdentProvider,
Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
RetryHelper retryHelper,
@@ -180,6 +188,7 @@
this.repoManager = repoManager;
this.gitRefUpdated = gitRefUpdated;
this.allUsersName = allUsersName;
+ this.externalIds = externalIds;
this.serverIdentProvider = serverIdentProvider;
this.metaDataUpdateInternalFactory = metaDataUpdateInternalFactory;
this.retryHelper = retryHelper;
@@ -193,6 +202,7 @@
gitRefUpdated,
null,
allUsersName,
+ externalIds,
metaDataUpdateInternalFactory,
retryHelper,
extIdNotesFactory,
@@ -212,6 +222,7 @@
private final GitRepositoryManager repoManager;
private final GitReferenceUpdated gitRefUpdated;
private final AllUsersName allUsersName;
+ private final ExternalIds externalIds;
private final Provider<PersonIdent> serverIdentProvider;
private final Provider<IdentifiedUser> identifiedUser;
private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
@@ -223,6 +234,7 @@
GitRepositoryManager repoManager,
GitReferenceUpdated gitRefUpdated,
AllUsersName allUsersName,
+ ExternalIds externalIds,
@GerritPersonIdent Provider<PersonIdent> serverIdentProvider,
Provider<IdentifiedUser> identifiedUser,
Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
@@ -231,6 +243,7 @@
this.repoManager = repoManager;
this.gitRefUpdated = gitRefUpdated;
this.allUsersName = allUsersName;
+ this.externalIds = externalIds;
this.serverIdentProvider = serverIdentProvider;
this.identifiedUser = identifiedUser;
this.metaDataUpdateInternalFactory = metaDataUpdateInternalFactory;
@@ -247,6 +260,7 @@
gitRefUpdated,
user,
allUsersName,
+ externalIds,
metaDataUpdateInternalFactory,
retryHelper,
extIdNotesFactory,
@@ -263,18 +277,25 @@
private final GitReferenceUpdated gitRefUpdated;
@Nullable private final IdentifiedUser currentUser;
private final AllUsersName allUsersName;
+ private final ExternalIds externalIds;
private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
private final RetryHelper retryHelper;
private final ExternalIdNotesLoader extIdNotesLoader;
private final PersonIdent committerIdent;
private final PersonIdent authorIdent;
+
+ // Invoked after reading the account config.
private final Runnable afterReadRevision;
+ // Invoked after updating the account but before committing the changes.
+ private final Runnable beforeCommit;
+
private AccountsUpdate(
GitRepositoryManager repoManager,
GitReferenceUpdated gitRefUpdated,
@Nullable IdentifiedUser currentUser,
AllUsersName allUsersName,
+ ExternalIds externalIds,
Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
RetryHelper retryHelper,
ExternalIdNotesLoader extIdNotesLoader,
@@ -285,11 +306,13 @@
gitRefUpdated,
currentUser,
allUsersName,
+ externalIds,
metaDataUpdateInternalFactory,
retryHelper,
extIdNotesLoader,
committerIdent,
authorIdent,
+ Runnables.doNothing(),
Runnables.doNothing());
}
@@ -299,23 +322,27 @@
GitReferenceUpdated gitRefUpdated,
@Nullable IdentifiedUser currentUser,
AllUsersName allUsersName,
+ ExternalIds externalIds,
Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
RetryHelper retryHelper,
ExternalIdNotesLoader extIdNotesLoader,
PersonIdent committerIdent,
PersonIdent authorIdent,
- Runnable afterReadRevision) {
+ Runnable afterReadRevision,
+ Runnable beforeCommit) {
this.repoManager = checkNotNull(repoManager, "repoManager");
this.gitRefUpdated = checkNotNull(gitRefUpdated, "gitRefUpdated");
this.currentUser = currentUser;
this.allUsersName = checkNotNull(allUsersName, "allUsersName");
+ this.externalIds = checkNotNull(externalIds, "externalIds");
this.metaDataUpdateInternalFactory =
checkNotNull(metaDataUpdateInternalFactory, "metaDataUpdateInternalFactory");
this.retryHelper = checkNotNull(retryHelper, "retryHelper");
this.extIdNotesLoader = checkNotNull(extIdNotesLoader, "extIdNotesLoader");
this.committerIdent = checkNotNull(committerIdent, "committerIdent");
this.authorIdent = checkNotNull(authorIdent, "authorIdent");
- this.afterReadRevision = afterReadRevision;
+ this.afterReadRevision = checkNotNull(afterReadRevision, "afterReadRevision");
+ this.beforeCommit = checkNotNull(beforeCommit, "beforeCommit");
}
/**
@@ -330,7 +357,7 @@
* @throws OrmException if creating the user branch fails
* @throws ConfigInvalidException if any of the account fields has an invalid value
*/
- public Account insert(
+ public AccountState insert(
String message, Account.Id accountId, Consumer<InternalAccountUpdate.Builder> init)
throws OrmException, IOException, ConfigInvalidException {
return insert(message, accountId, AccountUpdater.fromConsumer(init));
@@ -348,23 +375,27 @@
* @throws OrmException if creating the user branch fails
* @throws ConfigInvalidException if any of the account fields has an invalid value
*/
- public Account insert(String message, Account.Id accountId, AccountUpdater updater)
+ public AccountState insert(String message, Account.Id accountId, AccountUpdater updater)
throws OrmException, IOException, ConfigInvalidException {
return updateAccount(
- r -> {
- AccountConfig accountConfig = read(r, accountId);
- Account account =
- accountConfig.getNewAccount(new Timestamp(committerIdent.getWhen().getTime()));
- InternalAccountUpdate.Builder updateBuilder = InternalAccountUpdate.builder();
- updater.update(account, updateBuilder);
+ r -> {
+ AccountConfig accountConfig = read(r, accountId);
+ Account account =
+ accountConfig.getNewAccount(new Timestamp(committerIdent.getWhen().getTime()));
+ AccountState accountState = AccountState.forAccount(allUsersName, account);
+ InternalAccountUpdate.Builder updateBuilder = InternalAccountUpdate.builder();
+ updater.update(accountState, updateBuilder);
- InternalAccountUpdate update = updateBuilder.build();
- accountConfig.setAccountUpdate(update);
- ExternalIdNotes extIdNotes = createExternalIdNotes(r, accountId, update);
- UpdatedAccount updatedAccounts = new UpdatedAccount(message, accountConfig, extIdNotes);
- updatedAccounts.setCreated(true);
- return updatedAccounts;
- });
+ InternalAccountUpdate update = updateBuilder.build();
+ accountConfig.setAccountUpdate(update);
+ ExternalIdNotes extIdNotes =
+ createExternalIdNotes(r, accountConfig.getExternalIdsRev(), accountId, update);
+ UpdatedAccount updatedAccounts =
+ new UpdatedAccount(allUsersName, externalIds, message, accountConfig, extIdNotes);
+ updatedAccounts.setCreated(true);
+ return updatedAccounts;
+ })
+ .get();
}
/**
@@ -375,12 +406,12 @@
* @param message commit message for the account update, must not be {@code null or empty}
* @param accountId ID of the account
* @param update consumer to update the account, only invoked if the account exists
- * @return the updated account, {@code null} if the account doesn't exist
+ * @return the updated account, {@link Optional#empty()} if the account doesn't exist
* @throws IOException if updating the user branch fails due to an IO error
* @throws OrmException if updating the user branch fails
* @throws ConfigInvalidException if any of the account fields has an invalid value
*/
- public Account update(
+ public Optional<AccountState> update(
String message, Account.Id accountId, Consumer<InternalAccountUpdate.Builder> update)
throws OrmException, IOException, ConfigInvalidException {
return update(message, accountId, AccountUpdater.fromConsumer(update));
@@ -394,18 +425,18 @@
* @param message commit message for the account update, must not be {@code null or empty}
* @param accountId ID of the account
* @param updater updater to update the account, only invoked if the account exists
- * @return the updated account, {@code null} if the account doesn't exist
+ * @return the updated account, {@link Optional#empty} if the account doesn't exist
* @throws IOException if updating the user branch fails due to an IO error
* @throws OrmException if updating the user branch fails
* @throws ConfigInvalidException if any of the account fields has an invalid value
*/
- @Nullable
- public Account update(String message, Account.Id accountId, AccountUpdater updater)
+ public Optional<AccountState> update(String message, Account.Id accountId, AccountUpdater updater)
throws OrmException, IOException, ConfigInvalidException {
return updateAccount(
r -> {
AccountConfig accountConfig = read(r, accountId);
- Optional<Account> account = accountConfig.getLoadedAccount();
+ Optional<AccountState> account =
+ AccountState.fromAccountConfig(allUsersName, externalIds, accountConfig);
if (!account.isPresent()) {
return null;
}
@@ -415,8 +446,10 @@
InternalAccountUpdate update = updateBuilder.build();
accountConfig.setAccountUpdate(update);
- ExternalIdNotes extIdNotes = createExternalIdNotes(r, accountId, update);
- UpdatedAccount updatedAccounts = new UpdatedAccount(message, accountConfig, extIdNotes);
+ ExternalIdNotes extIdNotes =
+ createExternalIdNotes(r, accountConfig.getExternalIdsRev(), accountId, update);
+ UpdatedAccount updatedAccounts =
+ new UpdatedAccount(allUsersName, externalIds, message, accountConfig, extIdNotes);
return updatedAccounts;
});
}
@@ -428,7 +461,7 @@
return accountConfig;
}
- private Account updateAccount(AccountUpdate accountUpdate)
+ private Optional<AccountState> updateAccount(AccountUpdate accountUpdate)
throws IOException, ConfigInvalidException, OrmException {
return retryHelper.execute(
ActionType.ACCOUNT_UPDATE,
@@ -436,17 +469,20 @@
try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
UpdatedAccount updatedAccount = accountUpdate.update(allUsersRepo);
if (updatedAccount == null) {
- return null;
+ return Optional.empty();
}
commit(allUsersRepo, updatedAccount);
- return updatedAccount.getAccount();
+ return Optional.of(updatedAccount.getAccount());
}
});
}
private ExternalIdNotes createExternalIdNotes(
- Repository allUsersRepo, Account.Id accountId, InternalAccountUpdate update)
+ Repository allUsersRepo,
+ Optional<ObjectId> rev,
+ Account.Id accountId,
+ InternalAccountUpdate update)
throws IOException, ConfigInvalidException, OrmDuplicateKeyException {
ExternalIdNotes.checkSameAccount(
Iterables.concat(
@@ -455,14 +491,17 @@
update.getDeletedExternalIds()),
accountId);
- ExternalIdNotes extIdNotes = extIdNotesLoader.load(allUsersRepo);
+ ExternalIdNotes extIdNotes = extIdNotesLoader.load(allUsersRepo, rev.orElse(ObjectId.zeroId()));
extIdNotes.replace(update.getDeletedExternalIds(), update.getCreatedExternalIds());
extIdNotes.upsert(update.getUpdatedExternalIds());
return extIdNotes;
}
private void commit(Repository allUsersRepo, UpdatedAccount updatedAccount) throws IOException {
+ beforeCommit.run();
+
BatchRefUpdate batchRefUpdate = allUsersRepo.getRefDatabase().newBatchUpdate();
+
if (updatedAccount.isCreated()) {
commitNewAccountConfig(
updatedAccount.getMessage(),
@@ -482,6 +521,7 @@
allUsersRepo,
batchRefUpdate,
updatedAccount.getExternalIdNotes());
+
RefUpdateUtil.executeChecked(batchRefUpdate, allUsersRepo);
updatedAccount.getExternalIdNotes().updateCaches();
gitRefUpdated.fire(
@@ -554,6 +594,8 @@
}
private static class UpdatedAccount {
+ private final AllUsersName allUsersName;
+ private final ExternalIds externalIds;
private final String message;
private final AccountConfig accountConfig;
private final ExternalIdNotes extIdNotes;
@@ -561,8 +603,14 @@
private boolean created;
private UpdatedAccount(
- String message, AccountConfig accountConfig, ExternalIdNotes extIdNotes) {
+ AllUsersName allUsersName,
+ ExternalIds externalIds,
+ String message,
+ AccountConfig accountConfig,
+ ExternalIdNotes extIdNotes) {
checkState(!Strings.isNullOrEmpty(message), "message for account update must be set");
+ this.allUsersName = checkNotNull(allUsersName);
+ this.externalIds = checkNotNull(externalIds);
this.message = checkNotNull(message);
this.accountConfig = checkNotNull(accountConfig);
this.extIdNotes = checkNotNull(extIdNotes);
@@ -576,8 +624,9 @@
return accountConfig;
}
- public Account getAccount() {
- return accountConfig.getLoadedAccount().get();
+ public AccountState getAccount() throws IOException {
+ return AccountState.fromAccountConfig(allUsersName, externalIds, accountConfig, extIdNotes)
+ .get();
}
public ExternalIdNotes getExternalIdNotes() {
diff --git a/java/com/google/gerrit/server/account/InternalAccountDirectory.java b/java/com/google/gerrit/server/account/InternalAccountDirectory.java
index e7ff314..4aea478 100644
--- a/java/com/google/gerrit/server/account/InternalAccountDirectory.java
+++ b/java/com/google/gerrit/server/account/InternalAccountDirectory.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.account;
+import static java.util.stream.Collectors.toList;
+
import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.common.AccountInfo;
@@ -95,7 +97,7 @@
info.secondaryEmails = externalIds != null ? getSecondaryEmails(account, externalIds) : null;
}
if (options.contains(FillOptions.USERNAME)) {
- info.username = externalIds != null ? AccountState.getUserName(externalIds) : null;
+ info.username = externalIds != null ? ExternalId.getUserName(externalIds).orElse(null) : null;
}
if (options.contains(FillOptions.STATUS)) {
@@ -125,12 +127,10 @@
}
public List<String> getSecondaryEmails(Account account, Collection<ExternalId> externalIds) {
- List<String> emails = new ArrayList<>(AccountState.getEmails(externalIds));
- if (account.getPreferredEmail() != null) {
- emails.remove(account.getPreferredEmail());
- }
- Collections.sort(emails);
- return emails;
+ return ExternalId.getEmails(externalIds)
+ .filter(e -> !e.equals(account.getPreferredEmail()))
+ .sorted()
+ .collect(toList());
}
private static void addAvatar(
diff --git a/java/com/google/gerrit/server/account/InternalAccountUpdate.java b/java/com/google/gerrit/server/account/InternalAccountUpdate.java
index 05c431e..c7fa309 100644
--- a/java/com/google/gerrit/server/account/InternalAccountUpdate.java
+++ b/java/com/google/gerrit/server/account/InternalAccountUpdate.java
@@ -18,6 +18,8 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.WatchConfig.NotifyType;
@@ -123,6 +125,26 @@
public abstract Optional<GeneralPreferencesInfo> getGeneralPreferences();
/**
+ * Returns the new value for the diff preferences.
+ *
+ * <p>Only preferences that are non-null in the returned DiffPreferencesInfo should be updated.
+ *
+ * @return the new value for the diff preferences, {@code Optional#empty()} if the diff
+ * preferences are not being updated, the wrapped value is never {@code null}
+ */
+ public abstract Optional<DiffPreferencesInfo> getDiffPreferences();
+
+ /**
+ * Returns the new value for the edit preferences.
+ *
+ * <p>Only preferences that are non-null in the returned DiffPreferencesInfo should be updated.
+ *
+ * @return the new value for the edit preferences, {@code Optional#empty()} if the edit
+ * preferences are not being updated, the wrapped value is never {@code null}
+ */
+ public abstract Optional<EditPreferencesInfo> getEditPreferences();
+
+ /**
* Class to build an account update.
*
* <p>Account data is only updated if the corresponding setter is invoked. If a setter is not
@@ -385,6 +407,26 @@
public abstract Builder setGeneralPreferences(GeneralPreferencesInfo generalPreferences);
/**
+ * Sets the diff preferences for the account.
+ *
+ * <p>Updates any preference that is non-null in the provided DiffPreferencesInfo.
+ *
+ * @param diffPreferences the diff preferences that should be set
+ * @return the builder
+ */
+ public abstract Builder setDiffPreferences(DiffPreferencesInfo diffPreferences);
+
+ /**
+ * Sets the edit preferences for the account.
+ *
+ * <p>Updates any preference that is non-null in the provided EditPreferencesInfo.
+ *
+ * @param editPreferences the edit preferences that should be set
+ * @return the builder
+ */
+ public abstract Builder setEditPreferences(EditPreferencesInfo editPreferences);
+
+ /**
* Builds the account update.
*
* @return the account update
@@ -520,6 +562,18 @@
delegate.setGeneralPreferences(generalPreferences);
return this;
}
+
+ @Override
+ public Builder setDiffPreferences(DiffPreferencesInfo diffPreferences) {
+ delegate.setDiffPreferences(diffPreferences);
+ return this;
+ }
+
+ @Override
+ public Builder setEditPreferences(EditPreferencesInfo editPreferences) {
+ delegate.setEditPreferences(editPreferences);
+ return this;
+ }
}
}
}
diff --git a/java/com/google/gerrit/server/account/PreferencesConfig.java b/java/com/google/gerrit/server/account/PreferencesConfig.java
index 32df659..c471139 100644
--- a/java/com/google/gerrit/server/account/PreferencesConfig.java
+++ b/java/com/google/gerrit/server/account/PreferencesConfig.java
@@ -31,6 +31,8 @@
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.client.MenuItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -47,6 +49,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
@@ -65,6 +68,8 @@
private final ValidationError.Sink validationErrorSink;
private GeneralPreferencesInfo generalPreferences;
+ private DiffPreferencesInfo diffPreferences;
+ private EditPreferencesInfo editPreferences;
public PreferencesConfig(
Account.Id accountId,
@@ -84,29 +89,85 @@
return generalPreferences;
}
- public void parse() {
- generalPreferences = parse(null);
+ public DiffPreferencesInfo getDiffPreferences() {
+ if (diffPreferences == null) {
+ parse();
+ }
+ return diffPreferences;
}
- public Config saveGeneralPreferences(GeneralPreferencesInfo input) throws ConfigInvalidException {
- // merge configs
- input = parse(input);
+ public EditPreferencesInfo getEditPreferences() {
+ if (editPreferences == null) {
+ parse();
+ }
+ return editPreferences;
+ }
- storeSection(
- cfg, UserConfigSections.GENERAL, null, input, parseDefaultPreferences(defaultCfg, null));
- setChangeTable(cfg, input.changeTable);
- setMy(cfg, input.my);
- setUrlAliases(cfg, input.urlAliases);
+ public void parse() {
+ generalPreferences = parseGeneralPreferences(null);
+ diffPreferences = parseDiffPreferences(null);
+ editPreferences = parseEditPreferences(null);
+ }
- // evict the cached general preferences
- this.generalPreferences = null;
+ public Config saveGeneralPreferences(
+ Optional<GeneralPreferencesInfo> generalPreferencesInput,
+ Optional<DiffPreferencesInfo> diffPreferencesInput,
+ Optional<EditPreferencesInfo> editPreferencesInput)
+ throws ConfigInvalidException {
+ if (generalPreferencesInput.isPresent()) {
+ GeneralPreferencesInfo mergedGeneralPreferencesInput =
+ parseGeneralPreferences(generalPreferencesInput.get());
+
+ storeSection(
+ cfg,
+ UserConfigSections.GENERAL,
+ null,
+ mergedGeneralPreferencesInput,
+ parseDefaultGeneralPreferences(defaultCfg, null));
+ setChangeTable(cfg, mergedGeneralPreferencesInput.changeTable);
+ setMy(cfg, mergedGeneralPreferencesInput.my);
+ setUrlAliases(cfg, mergedGeneralPreferencesInput.urlAliases);
+
+ // evict the cached general preferences
+ this.generalPreferences = null;
+ }
+
+ if (diffPreferencesInput.isPresent()) {
+ DiffPreferencesInfo mergedDiffPreferencesInput =
+ parseDiffPreferences(diffPreferencesInput.get());
+
+ storeSection(
+ cfg,
+ UserConfigSections.DIFF,
+ null,
+ mergedDiffPreferencesInput,
+ parseDefaultDiffPreferences(defaultCfg, null));
+
+ // evict the cached diff preferences
+ this.diffPreferences = null;
+ }
+
+ if (editPreferencesInput.isPresent()) {
+ EditPreferencesInfo mergedEditPreferencesInput =
+ parseEditPreferences(editPreferencesInput.get());
+
+ storeSection(
+ cfg,
+ UserConfigSections.EDIT,
+ null,
+ mergedEditPreferencesInput,
+ parseDefaultEditPreferences(defaultCfg, null));
+
+ // evict the cached edit preferences
+ this.editPreferences = null;
+ }
return cfg;
}
- private GeneralPreferencesInfo parse(@Nullable GeneralPreferencesInfo input) {
+ private GeneralPreferencesInfo parseGeneralPreferences(@Nullable GeneralPreferencesInfo input) {
try {
- return parse(cfg, defaultCfg, input);
+ return parseGeneralPreferences(cfg, defaultCfg, input);
} catch (ConfigInvalidException e) {
validationErrorSink.error(
new ValidationError(
@@ -118,7 +179,33 @@
}
}
- private static GeneralPreferencesInfo parse(
+ private DiffPreferencesInfo parseDiffPreferences(@Nullable DiffPreferencesInfo input) {
+ try {
+ return parseDiffPreferences(cfg, defaultCfg, input);
+ } catch (ConfigInvalidException e) {
+ validationErrorSink.error(
+ new ValidationError(
+ PREFERENCES_CONFIG,
+ String.format(
+ "Invalid diff preferences for account %d: %s", accountId.get(), e.getMessage())));
+ return new DiffPreferencesInfo();
+ }
+ }
+
+ private EditPreferencesInfo parseEditPreferences(@Nullable EditPreferencesInfo input) {
+ try {
+ return parseEditPreferences(cfg, defaultCfg, input);
+ } catch (ConfigInvalidException e) {
+ validationErrorSink.error(
+ new ValidationError(
+ PREFERENCES_CONFIG,
+ String.format(
+ "Invalid edit preferences for account %d: %s", accountId.get(), e.getMessage())));
+ return new EditPreferencesInfo();
+ }
+ }
+
+ private static GeneralPreferencesInfo parseGeneralPreferences(
Config cfg, @Nullable Config defaultCfg, @Nullable GeneralPreferencesInfo input)
throws ConfigInvalidException {
GeneralPreferencesInfo r =
@@ -128,7 +215,7 @@
null,
new GeneralPreferencesInfo(),
defaultCfg != null
- ? parseDefaultPreferences(defaultCfg, input)
+ ? parseDefaultGeneralPreferences(defaultCfg, input)
: GeneralPreferencesInfo.defaults(),
input);
if (input != null) {
@@ -143,7 +230,35 @@
return r;
}
- private static GeneralPreferencesInfo parseDefaultPreferences(
+ private static DiffPreferencesInfo parseDiffPreferences(
+ Config cfg, @Nullable Config defaultCfg, @Nullable DiffPreferencesInfo input)
+ throws ConfigInvalidException {
+ return loadSection(
+ cfg,
+ UserConfigSections.DIFF,
+ null,
+ new DiffPreferencesInfo(),
+ defaultCfg != null
+ ? parseDefaultDiffPreferences(defaultCfg, input)
+ : DiffPreferencesInfo.defaults(),
+ input);
+ }
+
+ private static EditPreferencesInfo parseEditPreferences(
+ Config cfg, @Nullable Config defaultCfg, @Nullable EditPreferencesInfo input)
+ throws ConfigInvalidException {
+ return loadSection(
+ cfg,
+ UserConfigSections.EDIT,
+ null,
+ new EditPreferencesInfo(),
+ defaultCfg != null
+ ? parseDefaultEditPreferences(defaultCfg, input)
+ : EditPreferencesInfo.defaults(),
+ input);
+ }
+
+ private static GeneralPreferencesInfo parseDefaultGeneralPreferences(
Config defaultCfg, GeneralPreferencesInfo input) throws ConfigInvalidException {
GeneralPreferencesInfo allUserPrefs = new GeneralPreferencesInfo();
loadSection(
@@ -153,10 +268,37 @@
allUserPrefs,
GeneralPreferencesInfo.defaults(),
input);
- return updateDefaults(allUserPrefs);
+ return updateGeneralPreferencesDefaults(allUserPrefs);
}
- private static GeneralPreferencesInfo updateDefaults(GeneralPreferencesInfo input) {
+ private static DiffPreferencesInfo parseDefaultDiffPreferences(
+ Config defaultCfg, DiffPreferencesInfo input) throws ConfigInvalidException {
+ DiffPreferencesInfo allUserPrefs = new DiffPreferencesInfo();
+ loadSection(
+ defaultCfg,
+ UserConfigSections.DIFF,
+ null,
+ allUserPrefs,
+ DiffPreferencesInfo.defaults(),
+ input);
+ return updateDiffPreferencesDefaults(allUserPrefs);
+ }
+
+ private static EditPreferencesInfo parseDefaultEditPreferences(
+ Config defaultCfg, EditPreferencesInfo input) throws ConfigInvalidException {
+ EditPreferencesInfo allUserPrefs = new EditPreferencesInfo();
+ loadSection(
+ defaultCfg,
+ UserConfigSections.EDIT,
+ null,
+ allUserPrefs,
+ EditPreferencesInfo.defaults(),
+ input);
+ return updateEditPreferencesDefaults(allUserPrefs);
+ }
+
+ private static GeneralPreferencesInfo updateGeneralPreferencesDefaults(
+ GeneralPreferencesInfo input) {
GeneralPreferencesInfo result = GeneralPreferencesInfo.defaults();
try {
for (Field field : input.getClass().getDeclaredFields()) {
@@ -175,6 +317,44 @@
return result;
}
+ private static DiffPreferencesInfo updateDiffPreferencesDefaults(DiffPreferencesInfo input) {
+ DiffPreferencesInfo result = DiffPreferencesInfo.defaults();
+ try {
+ for (Field field : input.getClass().getDeclaredFields()) {
+ if (skipField(field)) {
+ continue;
+ }
+ Object newVal = field.get(input);
+ if (newVal != null) {
+ field.set(result, newVal);
+ }
+ }
+ } catch (IllegalAccessException e) {
+ log.error("Failed to apply default diff preferences", e);
+ return DiffPreferencesInfo.defaults();
+ }
+ return result;
+ }
+
+ private static EditPreferencesInfo updateEditPreferencesDefaults(EditPreferencesInfo input) {
+ EditPreferencesInfo result = EditPreferencesInfo.defaults();
+ try {
+ for (Field field : input.getClass().getDeclaredFields()) {
+ if (skipField(field)) {
+ continue;
+ }
+ Object newVal = field.get(input);
+ if (newVal != null) {
+ field.set(result, newVal);
+ }
+ }
+ } catch (IllegalAccessException e) {
+ log.error("Failed to apply default edit preferences", e);
+ return EditPreferencesInfo.defaults();
+ }
+ return result;
+ }
+
private static List<String> parseChangeTableColumns(Config cfg, @Nullable Config defaultCfg) {
List<String> changeTable = changeTable(cfg);
if (changeTable == null && defaultCfg != null) {
@@ -207,9 +387,19 @@
return urlAliases;
}
- public static GeneralPreferencesInfo readDefaultPreferences(Repository allUsersRepo)
+ public static GeneralPreferencesInfo readDefaultGeneralPreferences(Repository allUsersRepo)
throws IOException, ConfigInvalidException {
- return parse(readDefaultConfig(allUsersRepo), null, null);
+ return parseGeneralPreferences(readDefaultConfig(allUsersRepo), null, null);
+ }
+
+ public static DiffPreferencesInfo readDefaultDiffPreferences(Repository allUsersRepo)
+ throws IOException, ConfigInvalidException {
+ return parseDiffPreferences(readDefaultConfig(allUsersRepo), null, null);
+ }
+
+ public static EditPreferencesInfo readDefaultEditPreferences(Repository allUsersRepo)
+ throws IOException, ConfigInvalidException {
+ return parseEditPreferences(readDefaultConfig(allUsersRepo), null, null);
}
static Config readDefaultConfig(Repository allUsersRepo)
@@ -219,7 +409,7 @@
return defaultPrefs.getConfig();
}
- public static GeneralPreferencesInfo updateDefaultPreferences(
+ public static GeneralPreferencesInfo updateDefaultGeneralPreferences(
MetaDataUpdate md, GeneralPreferencesInfo input) throws IOException, ConfigInvalidException {
VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences();
defaultPrefs.load(md);
@@ -234,7 +424,37 @@
setUrlAliases(defaultPrefs.getConfig(), input.urlAliases);
defaultPrefs.commit(md);
- return parse(defaultPrefs.getConfig(), null, null);
+ return parseGeneralPreferences(defaultPrefs.getConfig(), null, null);
+ }
+
+ public static DiffPreferencesInfo updateDefaultDiffPreferences(
+ MetaDataUpdate md, DiffPreferencesInfo input) throws IOException, ConfigInvalidException {
+ VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences();
+ defaultPrefs.load(md);
+ storeSection(
+ defaultPrefs.getConfig(),
+ UserConfigSections.DIFF,
+ null,
+ input,
+ DiffPreferencesInfo.defaults());
+ defaultPrefs.commit(md);
+
+ return parseDiffPreferences(defaultPrefs.getConfig(), null, null);
+ }
+
+ public static EditPreferencesInfo updateDefaultEditPreferences(
+ MetaDataUpdate md, EditPreferencesInfo input) throws IOException, ConfigInvalidException {
+ VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences();
+ defaultPrefs.load(md);
+ storeSection(
+ defaultPrefs.getConfig(),
+ UserConfigSections.EDIT,
+ null,
+ input,
+ EditPreferencesInfo.defaults());
+ defaultPrefs.commit(md);
+
+ return parseEditPreferences(defaultPrefs.getConfig(), null, null);
}
private static List<String> changeTable(Config cfg) {
diff --git a/java/com/google/gerrit/server/account/SetInactiveFlag.java b/java/com/google/gerrit/server/account/SetInactiveFlag.java
index f8cd650..6e4ec63 100644
--- a/java/com/google/gerrit/server/account/SetInactiveFlag.java
+++ b/java/com/google/gerrit/server/account/SetInactiveFlag.java
@@ -39,22 +39,19 @@
public Response<?> deactivate(Account.Id accountId)
throws RestApiException, IOException, ConfigInvalidException, OrmException {
AtomicBoolean alreadyInactive = new AtomicBoolean(false);
- Account account =
- accountsUpdate
- .create()
- .update(
- "Deactivate Account via API",
- accountId,
- (a, u) -> {
- if (!a.isActive()) {
- alreadyInactive.set(true);
- } else {
- u.setActive(false);
- }
- });
- if (account == null) {
- throw new ResourceNotFoundException("account not found");
- }
+ accountsUpdate
+ .create()
+ .update(
+ "Deactivate Account via API",
+ accountId,
+ (a, u) -> {
+ if (!a.getAccount().isActive()) {
+ alreadyInactive.set(true);
+ } else {
+ u.setActive(false);
+ }
+ })
+ .orElseThrow(() -> new ResourceNotFoundException("account not found"));
if (alreadyInactive.get()) {
throw new ResourceConflictException("account not active");
}
@@ -64,22 +61,19 @@
public Response<String> activate(Account.Id accountId)
throws ResourceNotFoundException, IOException, ConfigInvalidException, OrmException {
AtomicBoolean alreadyActive = new AtomicBoolean(false);
- Account account =
- accountsUpdate
- .create()
- .update(
- "Activate Account via API",
- accountId,
- (a, u) -> {
- if (a.isActive()) {
- alreadyActive.set(true);
- } else {
- u.setActive(true);
- }
- });
- if (account == null) {
- throw new ResourceNotFoundException("account not found");
- }
+ accountsUpdate
+ .create()
+ .update(
+ "Activate Account via API",
+ accountId,
+ (a, u) -> {
+ if (a.getAccount().isActive()) {
+ alreadyActive.set(true);
+ } else {
+ u.setActive(true);
+ }
+ })
+ .orElseThrow(() -> new ResourceNotFoundException("account not found"));
return alreadyActive.get() ? Response.ok("") : Response.created("");
}
}
diff --git a/java/com/google/gerrit/server/account/WatchConfig.java b/java/com/google/gerrit/server/account/WatchConfig.java
index 7adadf7..a51a1f0 100644
--- a/java/com/google/gerrit/server/account/WatchConfig.java
+++ b/java/com/google/gerrit/server/account/WatchConfig.java
@@ -23,8 +23,10 @@
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Multimap;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
@@ -104,7 +106,7 @@
private final Config cfg;
private final ValidationError.Sink validationErrorSink;
- private Map<ProjectWatchKey, Set<NotifyType>> projectWatches;
+ private ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>> projectWatches;
public WatchConfig(Account.Id accountId, Config cfg, ValidationError.Sink validationErrorSink) {
this.accountId = checkNotNull(accountId, "accountId");
@@ -112,7 +114,7 @@
this.validationErrorSink = checkNotNull(validationErrorSink, "validationErrorSink");
}
- public Map<ProjectWatchKey, Set<NotifyType>> getProjectWatches() {
+ public ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>> getProjectWatches() {
if (projectWatches == null) {
parse();
}
@@ -123,8 +125,30 @@
projectWatches = parse(accountId, cfg, validationErrorSink);
}
+ /**
+ * Parses project watches from the given config file and returns them as a map.
+ *
+ * <p>A project watch is defined on a project and has a filter to match changes for which the
+ * project watch should be applied. The project and the filter form the map key. The map value is
+ * a set of notify types that decide for which events email notifications should be sent.
+ *
+ * <p>A project watch on the {@code All-Projects} project applies for all projects unless the
+ * project has a matching project watch.
+ *
+ * <p>A project watch can have an empty set of notify types. An empty set of notify types means
+ * that no notification for matching changes should be set. This is different from no project
+ * watch as it overwrites matching project watches from the {@code All-Projects} project.
+ *
+ * <p>Since we must be able to differentiate a project watch with an empty set of notify types
+ * from no project watch we can't use a {@link Multimap} as return type.
+ *
+ * @param accountId the ID of the account for which the project watches should be parsed
+ * @param cfg the config file from which the project watches should be parsed
+ * @param validationErrorSink validation error sink
+ * @return the parsed project watches
+ */
@VisibleForTesting
- public static Map<ProjectWatchKey, Set<NotifyType>> parse(
+ public static ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>> parse(
Account.Id accountId, Config cfg, ValidationError.Sink validationErrorSink) {
Map<ProjectWatchKey, Set<NotifyType>> projectWatches = new HashMap<>();
for (String projectName : cfg.getSubsections(PROJECT)) {
@@ -148,11 +172,11 @@
projectWatches.get(key).addAll(notifyValue.notifyTypes());
}
}
- return projectWatches;
+ return immutableCopyOf(projectWatches);
}
public Config save(Map<ProjectWatchKey, Set<NotifyType>> projectWatches) {
- this.projectWatches = projectWatches;
+ this.projectWatches = immutableCopyOf(projectWatches);
for (String projectName : cfg.getSubsections(PROJECT)) {
cfg.unsetSection(PROJECT, projectName);
@@ -172,6 +196,16 @@
return cfg;
}
+ private static ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>> immutableCopyOf(
+ Map<ProjectWatchKey, Set<NotifyType>> projectWatches) {
+ ImmutableMap.Builder<ProjectWatchKey, ImmutableSet<NotifyType>> b = ImmutableMap.builder();
+ projectWatches
+ .entrySet()
+ .stream()
+ .forEach(e -> b.put(e.getKey(), ImmutableSet.copyOf(e.getValue())));
+ return b.build();
+ }
+
@AutoValue
public abstract static class NotifyValue {
public static NotifyValue parse(
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalId.java b/java/com/google/gerrit/server/account/externalids/ExternalId.java
index 8be7092..9f5e671 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalId.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalId.java
@@ -32,8 +32,10 @@
import java.io.Serializable;
import java.util.Collection;
import java.util.Objects;
+import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
+import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -47,6 +49,34 @@
return USER_NAME_PATTERN.matcher(username).matches();
}
+ /**
+ * Returns the ID of the first external ID from the provided external IDs that has the {@link
+ * ExternalId#SCHEME_USERNAME} scheme.
+ *
+ * @param extIds external IDs
+ * @return the ID of the first external ID from the provided external IDs that has the {@link
+ * ExternalId#SCHEME_USERNAME} scheme
+ */
+ public static Optional<String> getUserName(Collection<ExternalId> extIds) {
+ return extIds
+ .stream()
+ .filter(e -> e.isScheme(SCHEME_USERNAME))
+ .map(e -> e.key().id())
+ .findFirst();
+ }
+
+ /**
+ * Returns all IDs of the provided external IDs that have the {@link ExternalId#SCHEME_MAILTO}
+ * scheme as a distinct stream.
+ *
+ * @param extIds external IDs
+ * @return distinct stream of all IDs of the provided external IDs that have the {@link
+ * ExternalId#SCHEME_MAILTO} scheme
+ */
+ public static Stream<String> getEmails(Collection<ExternalId> extIds) {
+ return extIds.stream().filter(e -> e.isScheme(SCHEME_MAILTO)).map(e -> e.key().id()).distinct();
+ }
+
private static final long serialVersionUID = 1L;
private static final String EXTERNAL_ID_SECTION = "externalId";
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java b/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
index 8e69f2e..7acc0ab 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
@@ -79,7 +79,26 @@
private static final int MAX_NOTE_SZ = 1 << 19;
public interface ExternalIdNotesLoader {
+ /**
+ * Loads the external ID notes from the current tip of the {@code refs/meta/external-ids}
+ * branch.
+ *
+ * @param allUsersRepo the All-Users repository
+ */
ExternalIdNotes load(Repository allUsersRepo) throws IOException, ConfigInvalidException;
+
+ /**
+ * Loads the external ID notes from the specified revision of the {@code refs/meta/external-ids}
+ * branch.
+ *
+ * @param allUsersRepo the All-Users repository
+ * @param rev the revision from which the external ID notes should be loaded, if {@code null}
+ * the external ID notes are loaded from the current tip, if {@link ObjectId#zeroId()} it's
+ * assumed that the {@code refs/meta/external-ids} branch doesn't exist and the loaded
+ * external IDs will be empty
+ */
+ ExternalIdNotes load(Repository allUsersRepo, @Nullable ObjectId rev)
+ throws IOException, ConfigInvalidException;
}
@Singleton
@@ -98,6 +117,12 @@
throws IOException, ConfigInvalidException {
return new ExternalIdNotes(externalIdCache, accountCache, allUsersRepo).load();
}
+
+ @Override
+ public ExternalIdNotes load(Repository allUsersRepo, @Nullable ObjectId rev)
+ throws IOException, ConfigInvalidException {
+ return new ExternalIdNotes(externalIdCache, accountCache, allUsersRepo).load(rev);
+ }
}
@Singleton
@@ -114,11 +139,17 @@
throws IOException, ConfigInvalidException {
return new ExternalIdNotes(externalIdCache, null, allUsersRepo).load();
}
+
+ @Override
+ public ExternalIdNotes load(Repository allUsersRepo, @Nullable ObjectId rev)
+ throws IOException, ConfigInvalidException {
+ return new ExternalIdNotes(externalIdCache, null, allUsersRepo).load(rev);
+ }
}
/**
* Loads the external ID notes for reading only. The external ID notes are loaded from the current
- * HEAD revision of the {@code refs/meta/external-ids} branch.
+ * tip of the {@code refs/meta/external-ids} branch.
*
* @return read-only {@link ExternalIdNotes} instance
*/
@@ -134,7 +165,9 @@
* specified revision of the {@code refs/meta/external-ids} branch.
*
* @param rev the revision from which the external ID notes should be loaded, if {@code null} the
- * external ID notes are loaded from the current HEAD revision
+ * external ID notes are loaded from the current tip, if {@link ObjectId#zeroId()} it's
+ * assumed that the {@code refs/meta/external-ids} branch doesn't exist and the loaded
+ * external IDs will be empty
* @return read-only {@link ExternalIdNotes} instance
*/
public static ExternalIdNotes loadReadOnly(Repository allUsersRepo, @Nullable ObjectId rev)
@@ -146,7 +179,7 @@
/**
* Loads the external ID notes for updates without cache evictions. The external ID notes are
- * loaded from the current HEAD revision of the {@code refs/meta/external-ids} branch.
+ * loaded from the current tip of the {@code refs/meta/external-ids} branch.
*
* @return {@link ExternalIdNotes} instance that doesn't updates caches on save
*/
@@ -200,8 +233,7 @@
}
/**
- * Loads the external ID notes from the current HEAD revision of the {@code
- * refs/meta/external-ids} branch.
+ * Loads the external ID notes from the current tip of the {@code refs/meta/external-ids} branch.
*
* @return {@link ExternalIdNotes} instance for chaining
*/
@@ -215,13 +247,19 @@
* branch.
*
* @param rev the revision from which the external ID notes should be loaded, if {@code null} the
- * external ID notes are loaded from the current HEAD revision
+ * external ID notes are loaded from the current tip, if {@link ObjectId#zeroId()} it's
+ * assumed that the {@code refs/meta/external-ids} branch doesn't exist and the loaded
+ * external IDs will be empty
* @return {@link ExternalIdNotes} instance for chaining
*/
ExternalIdNotes load(@Nullable ObjectId rev) throws IOException, ConfigInvalidException {
if (rev == null) {
return load();
}
+ if (ObjectId.zeroId().equals(rev)) {
+ load(repo, null);
+ return this;
+ }
load(repo, rev);
return this;
}
@@ -269,19 +307,19 @@
*
* @return all external IDs
*/
- public Set<ExternalId> all() throws IOException {
+ public ImmutableSet<ExternalId> all() throws IOException {
checkLoaded();
try (RevWalk rw = new RevWalk(repo)) {
- Set<ExternalId> extIds = new HashSet<>();
+ ImmutableSet.Builder<ExternalId> b = ImmutableSet.builder();
for (Note note : noteMap) {
byte[] raw = readNoteData(rw, note.getData());
try {
- extIds.add(ExternalId.parse(note.getName(), raw, note.getData()));
+ b.add(ExternalId.parse(note.getName(), raw, note.getData()));
} catch (ConfigInvalidException | RuntimeException e) {
log.error(String.format("Ignoring invalid external ID note %s", note.getName()), e);
}
}
- return extIds;
+ return b.build();
}
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdReader.java b/java/com/google/gerrit/server/account/externalids/ExternalIdReader.java
index b4ff539..8d040d6 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdReader.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdReader.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.account.externalids;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Description.Units;
@@ -26,7 +27,6 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -93,7 +93,7 @@
}
/** Reads and returns all external IDs. */
- Set<ExternalId> all() throws IOException, ConfigInvalidException {
+ ImmutableSet<ExternalId> all() throws IOException, ConfigInvalidException {
checkReadEnabled();
try (Timer0.Context ctx = readAllLatency.start();
@@ -107,10 +107,12 @@
* refs/meta/external-ids} branch.
*
* @param rev the revision from which the external IDs should be read, if {@code null} the
- * external IDs are read from the current HEAD revision
+ * external IDs are read from the current tip, if {@link ObjectId#zeroId()} it's assumed that
+ * the {@code refs/meta/external-ids} branch doesn't exist and the loaded external IDs will be
+ * empty
* @return all external IDs that were read from the specified revision
*/
- Set<ExternalId> all(@Nullable ObjectId rev) throws IOException, ConfigInvalidException {
+ ImmutableSet<ExternalId> all(@Nullable ObjectId rev) throws IOException, ConfigInvalidException {
checkReadEnabled();
try (Timer0.Context ctx = readAllLatency.start();
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIds.java b/java/com/google/gerrit/server/account/externalids/ExternalIds.java
index a8ecc40..11f5855c 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIds.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIds.java
@@ -14,15 +14,15 @@
package com.google.gerrit.server.account.externalids;
-import static java.util.stream.Collectors.toSet;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
@@ -43,12 +43,12 @@
}
/** Returns all external IDs. */
- public Set<ExternalId> all() throws IOException, ConfigInvalidException {
+ public ImmutableSet<ExternalId> all() throws IOException, ConfigInvalidException {
return externalIdReader.all();
}
/** Returns all external IDs from the specified revision of the refs/meta/external-ids branch. */
- public Set<ExternalId> all(ObjectId rev) throws IOException, ConfigInvalidException {
+ public ImmutableSet<ExternalId> all(ObjectId rev) throws IOException, ConfigInvalidException {
return externalIdReader.all(rev);
}
@@ -66,27 +66,31 @@
}
/** Returns the external IDs of the specified account. */
- public Set<ExternalId> byAccount(Account.Id accountId) throws IOException {
+ public ImmutableSet<ExternalId> byAccount(Account.Id accountId) throws IOException {
return externalIdCache.byAccount(accountId);
}
/** Returns the external IDs of the specified account that have the given scheme. */
- public Set<ExternalId> byAccount(Account.Id accountId, String scheme) throws IOException {
- return byAccount(accountId).stream().filter(e -> e.key().isScheme(scheme)).collect(toSet());
+ public ImmutableSet<ExternalId> byAccount(Account.Id accountId, String scheme)
+ throws IOException {
+ return byAccount(accountId)
+ .stream()
+ .filter(e -> e.key().isScheme(scheme))
+ .collect(toImmutableSet());
}
/** Returns the external IDs of the specified account. */
- public Set<ExternalId> byAccount(Account.Id accountId, ObjectId rev) throws IOException {
+ public ImmutableSet<ExternalId> byAccount(Account.Id accountId, ObjectId rev) throws IOException {
return externalIdCache.byAccount(accountId, rev);
}
/** Returns the external IDs of the specified account that have the given scheme. */
- public Set<ExternalId> byAccount(Account.Id accountId, String scheme, ObjectId rev)
+ public ImmutableSet<ExternalId> byAccount(Account.Id accountId, String scheme, ObjectId rev)
throws IOException {
return byAccount(accountId, rev)
.stream()
.filter(e -> e.key().isScheme(scheme))
- .collect(toSet());
+ .collect(toImmutableSet());
}
/** Returns all external IDs by account. */
@@ -107,7 +111,7 @@
*
* @see #byEmails(String...)
*/
- public Set<ExternalId> byEmail(String email) throws IOException {
+ public ImmutableSet<ExternalId> byEmail(String email) throws IOException {
return externalIdCache.byEmail(email);
}
diff --git a/java/com/google/gerrit/server/api/config/ServerImpl.java b/java/com/google/gerrit/server/api/config/ServerImpl.java
index ce87d1c..ec08507 100644
--- a/java/com/google/gerrit/server/api/config/ServerImpl.java
+++ b/java/com/google/gerrit/server/api/config/ServerImpl.java
@@ -21,15 +21,18 @@
import com.google.gerrit.extensions.api.config.ConsistencyCheckInput;
import com.google.gerrit.extensions.api.config.Server;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.common.ServerInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.restapi.config.CheckConsistency;
import com.google.gerrit.server.restapi.config.GetDiffPreferences;
+import com.google.gerrit.server.restapi.config.GetEditPreferences;
import com.google.gerrit.server.restapi.config.GetPreferences;
import com.google.gerrit.server.restapi.config.GetServerInfo;
import com.google.gerrit.server.restapi.config.SetDiffPreferences;
+import com.google.gerrit.server.restapi.config.SetEditPreferences;
import com.google.gerrit.server.restapi.config.SetPreferences;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -41,6 +44,8 @@
private final SetPreferences setPreferences;
private final GetDiffPreferences getDiffPreferences;
private final SetDiffPreferences setDiffPreferences;
+ private final GetEditPreferences getEditPreferences;
+ private final SetEditPreferences setEditPreferences;
private final GetServerInfo getServerInfo;
private final Provider<CheckConsistency> checkConsistency;
@@ -50,12 +55,16 @@
SetPreferences setPreferences,
GetDiffPreferences getDiffPreferences,
SetDiffPreferences setDiffPreferences,
+ GetEditPreferences getEditPreferences,
+ SetEditPreferences setEditPreferences,
GetServerInfo getServerInfo,
Provider<CheckConsistency> checkConsistency) {
this.getPreferences = getPreferences;
this.setPreferences = setPreferences;
this.getDiffPreferences = getDiffPreferences;
this.setDiffPreferences = setDiffPreferences;
+ this.getEditPreferences = getEditPreferences;
+ this.setEditPreferences = setEditPreferences;
this.getServerInfo = getServerInfo;
this.checkConsistency = checkConsistency;
}
@@ -113,6 +122,25 @@
}
@Override
+ public EditPreferencesInfo getDefaultEditPreferences() throws RestApiException {
+ try {
+ return getEditPreferences.apply(new ConfigResource());
+ } catch (Exception e) {
+ throw asRestApiException("Cannot get default edit preferences", e);
+ }
+ }
+
+ @Override
+ public EditPreferencesInfo setDefaultEditPreferences(EditPreferencesInfo in)
+ throws RestApiException {
+ try {
+ return setEditPreferences.apply(new ConfigResource(), in);
+ } catch (Exception e) {
+ throw asRestApiException("Cannot set default edit preferences", e);
+ }
+ }
+
+ @Override
public ConsistencyCheckInfo checkConsistency(ConsistencyCheckInput in) throws RestApiException {
try {
return checkConsistency.get().apply(new ConfigResource(), in);
diff --git a/java/com/google/gerrit/server/api/projects/DashboardApiImpl.java b/java/com/google/gerrit/server/api/projects/DashboardApiImpl.java
index 016a593..07924fa 100644
--- a/java/com/google/gerrit/server/api/projects/DashboardApiImpl.java
+++ b/java/com/google/gerrit/server/api/projects/DashboardApiImpl.java
@@ -21,7 +21,6 @@
import com.google.gerrit.extensions.api.projects.DashboardInfo;
import com.google.gerrit.extensions.common.SetDashboardInput;
import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.DashboardResource;
@@ -88,8 +87,7 @@
}
private DashboardResource resource()
- throws ResourceNotFoundException, IOException, ConfigInvalidException,
- PermissionBackendException {
+ throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
return dashboards.parse(project, IdString.fromDecoded(id));
}
}
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 8ba755f..fe00904 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -123,7 +123,6 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RemoveReviewerControl;
import com.google.gerrit.server.project.SubmitRuleOptions;
@@ -344,7 +343,6 @@
| OrmException
| IOException
| PermissionBackendException
- | NoSuchProjectException
| RuntimeException e) {
if (!has(CHECK)) {
Throwables.throwIfInstanceOf(e, OrmException.class);
@@ -423,7 +421,6 @@
| OrmException
| IOException
| PermissionBackendException
- | NoSuchProjectException
| RuntimeException e) {
if (has(CHECK)) {
i = checkOnly(cd);
@@ -489,8 +486,8 @@
}
private ChangeInfo toChangeInfo(ChangeData cd, Optional<PatchSet.Id> limitToPsId)
- throws PatchListNotAvailableException, GpgException, OrmException, IOException,
- PermissionBackendException, NoSuchProjectException {
+ throws PatchListNotAvailableException, GpgException, OrmException, PermissionBackendException,
+ IOException {
ChangeInfo out = new ChangeInfo();
CurrentUser user = userProvider.get();
@@ -1122,7 +1119,7 @@
}
private Collection<AccountInfo> removableReviewers(ChangeData cd, ChangeInfo out)
- throws PermissionBackendException, NoSuchProjectException, OrmException, IOException {
+ throws PermissionBackendException, OrmException {
// Although this is called removableReviewers, this method also determines
// which CCs are removable.
//
diff --git a/java/com/google/gerrit/server/change/ConsistencyChecker.java b/java/com/google/gerrit/server/change/ConsistencyChecker.java
index e10197f..c58cde7 100644
--- a/java/com/google/gerrit/server/change/ConsistencyChecker.java
+++ b/java/com/google/gerrit/server/change/ConsistencyChecker.java
@@ -222,7 +222,7 @@
private void checkOwner() {
try {
- if (accounts.get(change().getOwner()) == null) {
+ if (!accounts.get(change().getOwner()).isPresent()) {
problem("Missing change owner: " + change().getOwner());
}
} catch (IOException | ConfigInvalidException e) {
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 292fa7a..a9e3ed5 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -92,6 +92,7 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.SetHashtagsOp;
@@ -174,6 +175,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutionException;
@@ -2444,7 +2446,8 @@
// or no parents were updated (rebase), else warn that only part
// of the commit was modified.
if (newCommit.getTree().equals(priorCommit.getTree())) {
- boolean messageEq = eq(newCommit.getFullMessage(), priorCommit.getFullMessage());
+ boolean messageEq =
+ Objects.equals(newCommit.getFullMessage(), priorCommit.getFullMessage());
boolean parentsEq = parentsEqual(newCommit, priorCommit);
boolean authorEq = authorEqual(newCommit, priorCommit);
ObjectReader reader = rp.getRevWalk().getObjectReader();
@@ -2722,18 +2725,8 @@
return false;
}
- return eq(aAuthor.getName(), bAuthor.getName())
- && eq(aAuthor.getEmailAddress(), bAuthor.getEmailAddress());
- }
-
- static boolean eq(String a, String b) {
- if (a == null && b == null) {
- return true;
- } else if (a == null || b == null) {
- return false;
- } else {
- return a.equals(b);
- }
+ return Objects.equals(aAuthor.getName(), bAuthor.getName())
+ && Objects.equals(aAuthor.getEmailAddress(), bAuthor.getEmailAddress());
}
private boolean validRefOperation(ReceiveCommand cmd) {
@@ -2980,20 +2973,20 @@
}
logDebug("Updating full name of caller");
try {
- Account account =
+ Optional<AccountState> accountState =
accountsUpdate
.create()
.update(
"Set Full Name on Receive Commits",
user.getAccountId(),
(a, u) -> {
- if (Strings.isNullOrEmpty(a.getFullName())) {
+ if (Strings.isNullOrEmpty(a.getAccount().getFullName())) {
u.setFullName(setFullNameTo);
}
});
- if (account != null) {
- user.getAccount().setFullName(account.getFullName());
- }
+ accountState
+ .map(AccountState::getAccount)
+ .ifPresent(a -> user.getAccount().setFullName(a.getFullName()));
} catch (OrmException | IOException | ConfigInvalidException e) {
logWarn("Failed to update full name of caller", e);
}
diff --git a/java/com/google/gerrit/server/group/db/GroupsConsistencyChecker.java b/java/com/google/gerrit/server/group/db/GroupsConsistencyChecker.java
index a0eae4a..2335d32 100644
--- a/java/com/google/gerrit/server/group/db/GroupsConsistencyChecker.java
+++ b/java/com/google/gerrit/server/group/db/GroupsConsistencyChecker.java
@@ -34,6 +34,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
import java.util.Set;
import javax.inject.Inject;
import javax.inject.Singleton;
@@ -114,7 +115,7 @@
}
for (Account.Id id : g.getMembers().asList()) {
- AccountState account;
+ Optional<AccountState> account;
try {
account = accounts.get(id);
} catch (ConfigInvalidException e) {
@@ -124,7 +125,7 @@
g.getName(), g.getGroupUUID(), id, e.getMessage()));
continue;
}
- if (account == null) {
+ if (!account.isPresent()) {
problems.add(
error("group %s (%s) has nonexistent member %s", g.getName(), g.getGroupUUID(), id));
}
diff --git a/java/com/google/gerrit/server/mail/send/ProjectWatch.java b/java/com/google/gerrit/server/mail/send/ProjectWatch.java
index 8b0cc7f..93ed4f13 100644
--- a/java/com/google/gerrit/server/mail/send/ProjectWatch.java
+++ b/java/com/google/gerrit/server/mail/send/ProjectWatch.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.mail.send;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.index.query.Predicate;
@@ -69,7 +70,8 @@
for (AccountState a : args.accountQueryProvider.get().byWatchedProject(project)) {
Account.Id accountId = a.getAccount().getId();
- for (Map.Entry<ProjectWatchKey, Set<NotifyType>> e : a.getProjectWatches().entrySet()) {
+ for (Map.Entry<ProjectWatchKey, ImmutableSet<NotifyType>> e :
+ a.getProjectWatches().entrySet()) {
if (project.equals(e.getKey().project())
&& add(matching, accountId, e.getKey(), e.getValue(), type)) {
// We only want to prevent matching All-Projects if this filter hits
@@ -79,7 +81,8 @@
}
for (AccountState a : args.accountQueryProvider.get().byWatchedProject(args.allProjectsName)) {
- for (Map.Entry<ProjectWatchKey, Set<NotifyType>> e : a.getProjectWatches().entrySet()) {
+ for (Map.Entry<ProjectWatchKey, ImmutableSet<NotifyType>> e :
+ a.getProjectWatches().entrySet()) {
if (args.allProjectsName.equals(e.getKey().project())) {
Account.Id accountId = a.getAccount().getId();
if (!projectWatchers.contains(accountId)) {
diff --git a/java/com/google/gerrit/server/project/DefaultPermissionBackend.java b/java/com/google/gerrit/server/project/DefaultPermissionBackend.java
index b679fca..c33c01f 100644
--- a/java/com/google/gerrit/server/project/DefaultPermissionBackend.java
+++ b/java/com/google/gerrit/server/project/DefaultPermissionBackend.java
@@ -45,10 +45,13 @@
private static final CurrentUser.PropertyKey<Boolean> IS_ADMIN = CurrentUser.PropertyKey.create();
private final ProjectCache projectCache;
+ private final ProjectControl.Factory projectControlFactory;
@Inject
- DefaultPermissionBackend(ProjectCache projectCache) {
+ DefaultPermissionBackend(
+ ProjectCache projectCache, ProjectControl.Factory projectControlFactory) {
this.projectCache = projectCache;
+ this.projectControlFactory = projectControlFactory;
}
private CapabilityCollection capabilities() {
@@ -73,7 +76,7 @@
try {
ProjectState state = projectCache.checkedGet(project);
if (state != null) {
- return state.controlFor(user).asForProject().database(db);
+ return projectControlFactory.create(user, state).asForProject().database(db);
}
return FailedPermissionBackend.project("not found", new NoSuchProjectException(project));
} catch (IOException e) {
diff --git a/java/com/google/gerrit/server/project/ProjectControl.java b/java/com/google/gerrit/server/project/ProjectControl.java
index 68dbf86..4d6cdfa 100644
--- a/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/java/com/google/gerrit/server/project/ProjectControl.java
@@ -79,7 +79,7 @@
private final Set<AccountGroup.UUID> uploadGroups;
private final Set<AccountGroup.UUID> receiveGroups;
- private final PermissionBackend.WithUser perm;
+ private final PermissionBackend permissionBackend;
private final CurrentUser user;
private final ProjectState state;
private final ChangeControl.Factory changeControlFactory;
@@ -102,13 +102,21 @@
this.uploadGroups = uploadGroups;
this.receiveGroups = receiveGroups;
this.permissionFilter = permissionFilter;
- this.perm = permissionBackend.user(who);
+ this.permissionBackend = permissionBackend;
user = who;
state = ps;
}
ProjectControl forUser(CurrentUser who) {
- ProjectControl r = state.controlFor(who);
+ ProjectControl r =
+ new ProjectControl(
+ uploadGroups,
+ receiveGroups,
+ permissionFilter,
+ changeControlFactory,
+ permissionBackend,
+ who,
+ state);
// Not per-user, and reusing saves lookup time.
r.allSections = allSections;
return r;
@@ -173,7 +181,7 @@
boolean isAdmin() {
try {
- perm.check(GlobalPermission.ADMINISTRATE_SERVER);
+ permissionBackend.user(user).check(GlobalPermission.ADMINISTRATE_SERVER);
return true;
} catch (AuthException | PermissionBackendException e) {
return false;
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index a8dc993..b45f119 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -87,7 +87,6 @@
private final SitePaths sitePaths;
private final AllProjectsName allProjectsName;
private final ProjectCache projectCache;
- private final ProjectControl.Factory projectControlFactory;
private final PrologEnvironment.Factory envFactory;
private final GitRepositoryManager gitMgr;
private final RulesCache rulesCache;
@@ -121,7 +120,6 @@
final ProjectCache projectCache,
final AllProjectsName allProjectsName,
final AllUsersName allUsersName,
- final ProjectControl.Factory projectControlFactory,
final PrologEnvironment.Factory envFactory,
final GitRepositoryManager gitMgr,
final RulesCache rulesCache,
@@ -133,7 +131,6 @@
this.isAllProjects = config.getProject().getNameKey().equals(allProjectsName);
this.isAllUsers = config.getProject().getNameKey().equals(allUsersName);
this.allProjectsName = allProjectsName;
- this.projectControlFactory = projectControlFactory;
this.envFactory = envFactory;
this.gitMgr = gitMgr;
this.rulesCache = rulesCache;
@@ -355,10 +352,6 @@
return result;
}
- public ProjectControl controlFor(CurrentUser user) {
- return projectControlFactory.create(user, this);
- }
-
/**
* @return an iterable that walks through this project and then the parents of this project.
* Starts from this project and progresses up the hierarchy to All-Projects.
diff --git a/java/com/google/gerrit/server/project/RemoveReviewerControl.java b/java/com/google/gerrit/server/project/RemoveReviewerControl.java
index e91d36e..a0091a3 100644
--- a/java/com/google/gerrit/server/project/RemoveReviewerControl.java
+++ b/java/com/google/gerrit/server/project/RemoveReviewerControl.java
@@ -22,39 +22,36 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
-import java.io.IOException;
@Singleton
public class RemoveReviewerControl {
private final PermissionBackend permissionBackend;
private final Provider<ReviewDb> dbProvider;
- private final ProjectCache projectCache;
@Inject
- RemoveReviewerControl(
- PermissionBackend permissionBackend,
- Provider<ReviewDb> dbProvider,
- ProjectCache projectCache) {
+ RemoveReviewerControl(PermissionBackend permissionBackend, Provider<ReviewDb> dbProvider) {
this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider;
- this.projectCache = projectCache;
}
/**
* Checks if removing the given reviewer and patch set approval is OK.
*
* @throws AuthException if this user is not allowed to remove this approval.
+ * @throws PermissionBackendException on failure of permission checks.
*/
public void checkRemoveReviewer(
ChangeNotes notes, CurrentUser currentUser, PatchSetApproval approval)
- throws PermissionBackendException, AuthException, NoSuchProjectException, IOException {
+ throws PermissionBackendException, AuthException {
checkRemoveReviewer(notes, currentUser, approval.getAccountId(), approval.getValue());
}
@@ -63,17 +60,19 @@
* reviewer might have given is OK.
*
* @throws AuthException if this user is not allowed to remove this approval.
+ * @throws PermissionBackendException on failure of permission checks.
*/
public void checkRemoveReviewer(ChangeNotes notes, CurrentUser currentUser, Account.Id reviewer)
- throws PermissionBackendException, AuthException, NoSuchProjectException, IOException {
+ throws PermissionBackendException, AuthException {
checkRemoveReviewer(notes, currentUser, reviewer, 0);
}
/** @return true if the user is allowed to remove this reviewer. */
public boolean testRemoveReviewer(
ChangeData cd, CurrentUser currentUser, Account.Id reviewer, int value)
- throws PermissionBackendException, NoSuchProjectException, OrmException, IOException {
- if (canRemoveReviewerWithoutPermissionCheck(cd.change(), currentUser, reviewer, value)) {
+ throws PermissionBackendException, OrmException {
+ if (canRemoveReviewerWithoutPermissionCheck(
+ permissionBackend, cd.change(), currentUser, reviewer, value)) {
return true;
}
return permissionBackend
@@ -85,8 +84,9 @@
private void checkRemoveReviewer(
ChangeNotes notes, CurrentUser currentUser, Account.Id reviewer, int val)
- throws PermissionBackendException, AuthException, NoSuchProjectException, IOException {
- if (canRemoveReviewerWithoutPermissionCheck(notes.getChange(), currentUser, reviewer, val)) {
+ throws PermissionBackendException, AuthException {
+ if (canRemoveReviewerWithoutPermissionCheck(
+ permissionBackend, notes.getChange(), currentUser, reviewer, val)) {
return;
}
@@ -97,9 +97,13 @@
.check(ChangePermission.REMOVE_REVIEWER);
}
- private boolean canRemoveReviewerWithoutPermissionCheck(
- Change change, CurrentUser currentUser, Account.Id reviewer, int value)
- throws NoSuchProjectException, IOException {
+ private static boolean canRemoveReviewerWithoutPermissionCheck(
+ PermissionBackend permissionBackend,
+ Change change,
+ CurrentUser currentUser,
+ Account.Id reviewer,
+ int value)
+ throws PermissionBackendException {
if (!change.getStatus().isOpen()) {
return false;
}
@@ -115,17 +119,32 @@
// Users with the remove reviewer permission, the branch owner, project
// owner and site admin can remove anyone
- // TODO(hiesel): Remove all Control usage
- ProjectState projectState = projectCache.checkedGet(change.getProject());
- if (projectState == null) {
- throw new NoSuchProjectException(change.getProject());
- }
- ProjectControl ctl = projectState.controlFor(currentUser);
- if (ctl.controlForRef(change.getDest()).isOwner() // branch owner
- || ctl.isOwner() // project owner
- || ctl.isAdmin()) { // project admin
+ PermissionBackend.WithUser withUser = permissionBackend.user(currentUser);
+ PermissionBackend.ForProject forProject = withUser.project(change.getProject());
+ if (check(forProject.ref(change.getDest().get()), RefPermission.WRITE_CONFIG)
+ || check(withUser, GlobalPermission.ADMINISTRATE_SERVER)) {
return true;
}
return false;
}
+
+ private static boolean check(PermissionBackend.ForRef forRef, RefPermission perm)
+ throws PermissionBackendException {
+ try {
+ forRef.check(perm);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
+ }
+
+ private static boolean check(PermissionBackend.WithUser withUser, GlobalPermission perm)
+ throws PermissionBackendException {
+ try {
+ withUser.check(perm);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/account/GetDiffPreferences.java b/java/com/google/gerrit/server/restapi/account/GetDiffPreferences.java
index c173079..3281f6e 100644
--- a/java/com/google/gerrit/server/restapi/account/GetDiffPreferences.java
+++ b/java/com/google/gerrit/server/restapi/account/GetDiffPreferences.java
@@ -14,19 +14,13 @@
package com.google.gerrit.server.restapi.account;
-import static com.google.gerrit.server.config.ConfigUtil.loadSection;
-import static com.google.gerrit.server.config.ConfigUtil.skipField;
-
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResource;
-import com.google.gerrit.server.account.VersionedAccountPreferences;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.UserConfigSections;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -34,32 +28,20 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.lang.reflect.Field;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Repository;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
public class GetDiffPreferences implements RestReadView<AccountResource> {
- private static final Logger log = LoggerFactory.getLogger(GetDiffPreferences.class);
-
private final Provider<CurrentUser> self;
- private final Provider<AllUsersName> allUsersName;
private final PermissionBackend permissionBackend;
- private final GitRepositoryManager gitMgr;
+ private final AccountCache accountCache;
@Inject
GetDiffPreferences(
- Provider<CurrentUser> self,
- Provider<AllUsersName> allUsersName,
- PermissionBackend permissionBackend,
- GitRepositoryManager gitMgr) {
+ Provider<CurrentUser> self, PermissionBackend permissionBackend, AccountCache accountCache) {
this.self = self;
- this.allUsersName = allUsersName;
this.permissionBackend = permissionBackend;
- this.gitMgr = gitMgr;
+ this.accountCache = accountCache;
}
@Override
@@ -70,53 +52,6 @@
}
Account.Id id = rsrc.getUser().getAccountId();
- return readFromGit(id, gitMgr, allUsersName.get(), null);
- }
-
- static DiffPreferencesInfo readFromGit(
- Account.Id id, GitRepositoryManager gitMgr, AllUsersName allUsersName, DiffPreferencesInfo in)
- throws IOException, ConfigInvalidException, RepositoryNotFoundException {
- try (Repository git = gitMgr.openRepository(allUsersName)) {
- VersionedAccountPreferences p = VersionedAccountPreferences.forUser(id);
- p.load(git);
- DiffPreferencesInfo prefs = new DiffPreferencesInfo();
- loadSection(
- p.getConfig(), UserConfigSections.DIFF, null, prefs, readDefaultsFromGit(git, in), in);
- return prefs;
- }
- }
-
- static DiffPreferencesInfo readDefaultsFromGit(Repository git, DiffPreferencesInfo in)
- throws ConfigInvalidException, IOException {
- VersionedAccountPreferences dp = VersionedAccountPreferences.forDefault();
- dp.load(git);
- DiffPreferencesInfo allUserPrefs = new DiffPreferencesInfo();
- loadSection(
- dp.getConfig(),
- UserConfigSections.DIFF,
- null,
- allUserPrefs,
- DiffPreferencesInfo.defaults(),
- in);
- return updateDefaults(allUserPrefs);
- }
-
- private static DiffPreferencesInfo updateDefaults(DiffPreferencesInfo input) {
- DiffPreferencesInfo result = DiffPreferencesInfo.defaults();
- try {
- for (Field field : input.getClass().getDeclaredFields()) {
- if (skipField(field)) {
- continue;
- }
- Object newVal = field.get(input);
- if (newVal != null) {
- field.set(result, newVal);
- }
- }
- } catch (IllegalAccessException e) {
- log.warn("Cannot get default diff preferences from All-Users", e);
- return DiffPreferencesInfo.defaults();
- }
- return result;
+ return accountCache.get(id).getDiffPreferences();
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/GetEditPreferences.java b/java/com/google/gerrit/server/restapi/account/GetEditPreferences.java
index 95a6e7c..a51da10 100644
--- a/java/com/google/gerrit/server/restapi/account/GetEditPreferences.java
+++ b/java/com/google/gerrit/server/restapi/account/GetEditPreferences.java
@@ -14,18 +14,13 @@
package com.google.gerrit.server.restapi.account;
-import static com.google.gerrit.server.config.ConfigUtil.loadSection;
-
import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResource;
-import com.google.gerrit.server.account.VersionedAccountPreferences;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.UserConfigSections;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -34,26 +29,19 @@
import com.google.inject.Singleton;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Repository;
@Singleton
public class GetEditPreferences implements RestReadView<AccountResource> {
private final Provider<CurrentUser> self;
private final PermissionBackend permissionBackend;
- private final AllUsersName allUsersName;
- private final GitRepositoryManager gitMgr;
+ private final AccountCache accountCache;
@Inject
GetEditPreferences(
- Provider<CurrentUser> self,
- PermissionBackend permissionBackend,
- AllUsersName allUsersName,
- GitRepositoryManager gitMgr) {
+ Provider<CurrentUser> self, PermissionBackend permissionBackend, AccountCache accountCache) {
this.self = self;
this.permissionBackend = permissionBackend;
- this.allUsersName = allUsersName;
- this.gitMgr = gitMgr;
+ this.accountCache = accountCache;
}
@Override
@@ -63,23 +51,7 @@
permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT);
}
- return readFromGit(rsrc.getUser().getAccountId(), gitMgr, allUsersName, null);
- }
-
- static EditPreferencesInfo readFromGit(
- Account.Id id, GitRepositoryManager gitMgr, AllUsersName allUsersName, EditPreferencesInfo in)
- throws IOException, ConfigInvalidException, RepositoryNotFoundException {
- try (Repository git = gitMgr.openRepository(allUsersName)) {
- VersionedAccountPreferences p = VersionedAccountPreferences.forUser(id);
- p.load(git);
-
- return loadSection(
- p.getConfig(),
- UserConfigSections.EDIT,
- null,
- new EditPreferencesInfo(),
- EditPreferencesInfo.defaults(),
- in);
- }
+ Account.Id id = rsrc.getUser().getAccountId();
+ return accountCache.get(id).getEditPreferences();
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/GetWatchedProjects.java b/java/com/google/gerrit/server/restapi/account/GetWatchedProjects.java
index ffddc7c..460d072 100644
--- a/java/com/google/gerrit/server/restapi/account/GetWatchedProjects.java
+++ b/java/com/google/gerrit/server/restapi/account/GetWatchedProjects.java
@@ -16,6 +16,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ComparisonChain;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.client.ProjectWatchInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -40,7 +41,6 @@
import java.util.Comparator;
import java.util.List;
import java.util.Map;
-import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
@@ -66,12 +66,10 @@
}
Account.Id accountId = rsrc.getUser().getAccountId();
- AccountState account = accounts.get(accountId);
- if (account == null) {
- throw new ResourceNotFoundException();
- }
+ AccountState account = accounts.get(accountId).orElseThrow(ResourceNotFoundException::new);
List<ProjectWatchInfo> projectWatchInfos = new ArrayList<>();
- for (Map.Entry<ProjectWatchKey, Set<NotifyType>> e : account.getProjectWatches().entrySet()) {
+ for (Map.Entry<ProjectWatchKey, ImmutableSet<NotifyType>> e :
+ account.getProjectWatches().entrySet()) {
ProjectWatchInfo pwi = new ProjectWatchInfo();
pwi.filter = e.getKey().filter();
pwi.project = e.getKey().project().get();
diff --git a/java/com/google/gerrit/server/restapi/account/PutName.java b/java/com/google/gerrit/server/restapi/account/PutName.java
index 4981e7a..0c85608 100644
--- a/java/com/google/gerrit/server/restapi/account/PutName.java
+++ b/java/com/google/gerrit/server/restapi/account/PutName.java
@@ -22,10 +22,10 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResource;
+import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.permissions.GlobalPermission;
@@ -79,15 +79,13 @@
}
String newName = input.name;
- Account account =
+ AccountState accountState =
accountsUpdate
.create()
- .update("Set Full Name via API", user.getAccountId(), u -> u.setFullName(newName));
- if (account == null) {
- throw new ResourceNotFoundException("account not found");
- }
- return Strings.isNullOrEmpty(account.getFullName())
+ .update("Set Full Name via API", user.getAccountId(), u -> u.setFullName(newName))
+ .orElseThrow(() -> new ResourceNotFoundException("account not found"));
+ return Strings.isNullOrEmpty(accountState.getAccount().getFullName())
? Response.none()
- : Response.ok(account.getFullName());
+ : Response.ok(accountState.getAccount().getFullName());
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/PutPreferred.java b/java/com/google/gerrit/server/restapi/account/PutPreferred.java
index b66a611..5c22273 100644
--- a/java/com/google/gerrit/server/restapi/account/PutPreferred.java
+++ b/java/com/google/gerrit/server/restapi/account/PutPreferred.java
@@ -19,7 +19,6 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResource;
@@ -65,22 +64,19 @@
public Response<String> apply(IdentifiedUser user, String email)
throws ResourceNotFoundException, IOException, ConfigInvalidException, OrmException {
AtomicBoolean alreadyPreferred = new AtomicBoolean(false);
- Account account =
- accountsUpdate
- .create()
- .update(
- "Set Preferred Email via API",
- user.getAccountId(),
- (a, u) -> {
- if (email.equals(a.getPreferredEmail())) {
- alreadyPreferred.set(true);
- } else {
- u.setPreferredEmail(email);
- }
- });
- if (account == null) {
- throw new ResourceNotFoundException("account not found");
- }
+ accountsUpdate
+ .create()
+ .update(
+ "Set Preferred Email via API",
+ user.getAccountId(),
+ (a, u) -> {
+ if (email.equals(a.getAccount().getPreferredEmail())) {
+ alreadyPreferred.set(true);
+ } else {
+ u.setPreferredEmail(email);
+ }
+ })
+ .orElseThrow(() -> new ResourceNotFoundException("account not found"));
return alreadyPreferred.get() ? Response.ok("") : Response.created("");
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/PutStatus.java b/java/com/google/gerrit/server/restapi/account/PutStatus.java
index 23958a2..40ae90f 100644
--- a/java/com/google/gerrit/server/restapi/account/PutStatus.java
+++ b/java/com/google/gerrit/server/restapi/account/PutStatus.java
@@ -20,10 +20,10 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResource;
+import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -68,15 +68,13 @@
}
String newStatus = input.status;
- Account account =
+ AccountState accountState =
accountsUpdate
.create()
- .update("Set Status via API", user.getAccountId(), u -> u.setStatus(newStatus));
- if (account == null) {
- throw new ResourceNotFoundException("account not found");
- }
- return Strings.isNullOrEmpty(account.getStatus())
+ .update("Set Status via API", user.getAccountId(), u -> u.setStatus(newStatus))
+ .orElseThrow(() -> new ResourceNotFoundException("account not found"));
+ return Strings.isNullOrEmpty(accountState.getAccount().getStatus())
? Response.none()
- : Response.ok(account.getStatus());
+ : Response.ok(accountState.getAccount().getStatus());
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/SetDiffPreferences.java b/java/com/google/gerrit/server/restapi/account/SetDiffPreferences.java
index c16e0f2..4700b96 100644
--- a/java/com/google/gerrit/server/restapi/account/SetDiffPreferences.java
+++ b/java/com/google/gerrit/server/restapi/account/SetDiffPreferences.java
@@ -14,26 +14,19 @@
package com.google.gerrit.server.restapi.account;
-import static com.google.gerrit.server.config.ConfigUtil.loadSection;
-import static com.google.gerrit.server.config.ConfigUtil.storeSection;
-import static com.google.gerrit.server.restapi.account.GetDiffPreferences.readDefaultsFromGit;
-import static com.google.gerrit.server.restapi.account.GetDiffPreferences.readFromGit;
-
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResource;
-import com.google.gerrit.server.account.VersionedAccountPreferences;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.MetaDataUpdate;
-import com.google.gerrit.server.git.UserConfigSections;
+import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -44,52 +37,38 @@
@Singleton
public class SetDiffPreferences implements RestModifyView<AccountResource, DiffPreferencesInfo> {
private final Provider<CurrentUser> self;
- private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
- private final AllUsersName allUsersName;
private final PermissionBackend permissionBackend;
- private final GitRepositoryManager gitMgr;
+ private final AccountCache accountCache;
+ private final AccountsUpdate.User accountsUpdate;
@Inject
SetDiffPreferences(
Provider<CurrentUser> self,
- Provider<MetaDataUpdate.User> metaDataUpdateFactory,
- AllUsersName allUsersName,
PermissionBackend permissionBackend,
- GitRepositoryManager gitMgr) {
+ AccountCache accountCache,
+ AccountsUpdate.User accountsUpdate) {
this.self = self;
- this.metaDataUpdateFactory = metaDataUpdateFactory;
- this.allUsersName = allUsersName;
this.permissionBackend = permissionBackend;
- this.gitMgr = gitMgr;
+ this.accountCache = accountCache;
+ this.accountsUpdate = accountsUpdate;
}
@Override
- public DiffPreferencesInfo apply(AccountResource rsrc, DiffPreferencesInfo in)
+ public DiffPreferencesInfo apply(AccountResource rsrc, DiffPreferencesInfo input)
throws AuthException, BadRequestException, ConfigInvalidException,
- RepositoryNotFoundException, IOException, PermissionBackendException {
+ RepositoryNotFoundException, IOException, PermissionBackendException, OrmException {
if (self.get() != rsrc.getUser()) {
permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT);
}
- if (in == null) {
+ if (input == null) {
throw new BadRequestException("input must be provided");
}
Account.Id id = rsrc.getUser().getAccountId();
- return writeToGit(readFromGit(id, gitMgr, allUsersName, in), id);
- }
-
- private DiffPreferencesInfo writeToGit(DiffPreferencesInfo in, Account.Id userId)
- throws RepositoryNotFoundException, IOException, ConfigInvalidException {
- DiffPreferencesInfo out = new DiffPreferencesInfo();
- try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
- DiffPreferencesInfo allUserPrefs = readDefaultsFromGit(md.getRepository(), null);
- VersionedAccountPreferences prefs = VersionedAccountPreferences.forUser(userId);
- prefs.load(md);
- storeSection(prefs.getConfig(), UserConfigSections.DIFF, null, in, allUserPrefs);
- prefs.commit(md);
- loadSection(prefs.getConfig(), UserConfigSections.DIFF, null, out, allUserPrefs, null);
- }
- return out;
+ accountsUpdate
+ .create()
+ .update("Set Diff Preferences via API", id, u -> u.setDiffPreferences(input));
+ return accountCache.get(id).getDiffPreferences();
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/SetEditPreferences.java b/java/com/google/gerrit/server/restapi/account/SetEditPreferences.java
index 3574377..f16bb9e 100644
--- a/java/com/google/gerrit/server/restapi/account/SetEditPreferences.java
+++ b/java/com/google/gerrit/server/restapi/account/SetEditPreferences.java
@@ -14,25 +14,19 @@
package com.google.gerrit.server.restapi.account;
-import static com.google.gerrit.server.config.ConfigUtil.loadSection;
-import static com.google.gerrit.server.config.ConfigUtil.storeSection;
-import static com.google.gerrit.server.restapi.account.GetEditPreferences.readFromGit;
-
import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResource;
-import com.google.gerrit.server.account.VersionedAccountPreferences;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.MetaDataUpdate;
-import com.google.gerrit.server.git.UserConfigSections;
+import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -44,61 +38,38 @@
public class SetEditPreferences implements RestModifyView<AccountResource, EditPreferencesInfo> {
private final Provider<CurrentUser> self;
- private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
private final PermissionBackend permissionBackend;
- private final GitRepositoryManager gitMgr;
- private final AllUsersName allUsersName;
+ private final AccountCache accountCache;
+ private final AccountsUpdate.User accountsUpdate;
@Inject
SetEditPreferences(
Provider<CurrentUser> self,
- Provider<MetaDataUpdate.User> metaDataUpdateFactory,
PermissionBackend permissionBackend,
- GitRepositoryManager gitMgr,
- AllUsersName allUsersName) {
+ AccountCache accountCache,
+ AccountsUpdate.User accountsUpdate) {
this.self = self;
- this.metaDataUpdateFactory = metaDataUpdateFactory;
this.permissionBackend = permissionBackend;
- this.gitMgr = gitMgr;
- this.allUsersName = allUsersName;
+ this.accountCache = accountCache;
+ this.accountsUpdate = accountsUpdate;
}
@Override
- public EditPreferencesInfo apply(AccountResource rsrc, EditPreferencesInfo in)
+ public EditPreferencesInfo apply(AccountResource rsrc, EditPreferencesInfo input)
throws AuthException, BadRequestException, RepositoryNotFoundException, IOException,
- ConfigInvalidException, PermissionBackendException {
+ ConfigInvalidException, PermissionBackendException, OrmException {
if (self.get() != rsrc.getUser()) {
permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT);
}
- if (in == null) {
+ if (input == null) {
throw new BadRequestException("input must be provided");
}
- Account.Id accountId = rsrc.getUser().getAccountId();
-
- VersionedAccountPreferences prefs;
- EditPreferencesInfo out = new EditPreferencesInfo();
- try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
- prefs = VersionedAccountPreferences.forUser(accountId);
- prefs.load(md);
- storeSection(
- prefs.getConfig(),
- UserConfigSections.EDIT,
- null,
- readFromGit(accountId, gitMgr, allUsersName, in),
- EditPreferencesInfo.defaults());
- prefs.commit(md);
- out =
- loadSection(
- prefs.getConfig(),
- UserConfigSections.EDIT,
- null,
- out,
- EditPreferencesInfo.defaults(),
- null);
- }
-
- return out;
+ Account.Id id = rsrc.getUser().getAccountId();
+ accountsUpdate
+ .create()
+ .update("Set Diff Preferences via API", id, u -> u.setEditPreferences(input));
+ return accountCache.get(id).getEditPreferences();
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/SetPreferences.java b/java/com/google/gerrit/server/restapi/account/SetPreferences.java
index 9b2b231..8131748 100644
--- a/java/com/google/gerrit/server/restapi/account/SetPreferences.java
+++ b/java/com/google/gerrit/server/restapi/account/SetPreferences.java
@@ -73,7 +73,7 @@
accountsUpdate
.create()
- .update("Set Preferences via API", id, u -> u.setGeneralPreferences(input));
+ .update("Set General Preferences via API", id, u -> u.setGeneralPreferences(input));
return cache.get(id).getGeneralPreferences();
}
diff --git a/java/com/google/gerrit/server/restapi/account/StarredChanges.java b/java/com/google/gerrit/server/restapi/account/StarredChanges.java
index f908d9f..1d3fae0 100644
--- a/java/com/google/gerrit/server/restapi/account/StarredChanges.java
+++ b/java/com/google/gerrit/server/restapi/account/StarredChanges.java
@@ -73,7 +73,7 @@
@Override
public AccountResource.StarredChange parse(AccountResource parent, IdString id)
- throws RestApiException, OrmException, PermissionBackendException {
+ throws RestApiException, OrmException, PermissionBackendException, IOException {
IdentifiedUser user = parent.getUser();
ChangeResource change = changes.parse(TopLevelResource.INSTANCE, id);
if (starredChangesUtil
@@ -109,7 +109,7 @@
return createProvider.get().setChange(changes.parse(TopLevelResource.INSTANCE, id));
} catch (ResourceNotFoundException e) {
throw new UnprocessableEntityException(String.format("change %s not found", id.get()));
- } catch (OrmException | PermissionBackendException e) {
+ } catch (OrmException | PermissionBackendException | IOException e) {
log.error("cannot resolve change", e);
throw new UnprocessableEntityException("internal server error");
}
diff --git a/java/com/google/gerrit/server/restapi/account/Stars.java b/java/com/google/gerrit/server/restapi/account/Stars.java
index 2ee6aef..6862e8a 100644
--- a/java/com/google/gerrit/server/restapi/account/Stars.java
+++ b/java/com/google/gerrit/server/restapi/account/Stars.java
@@ -40,6 +40,7 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import java.io.IOException;
import java.util.Collection;
import java.util.List;
import java.util.Set;
@@ -67,7 +68,7 @@
@Override
public Star parse(AccountResource parent, IdString id)
- throws RestApiException, OrmException, PermissionBackendException {
+ throws RestApiException, OrmException, PermissionBackendException, IOException {
IdentifiedUser user = parent.getUser();
ChangeResource change = changes.parse(TopLevelResource.INSTANCE, id);
Set<String> labels = starredChangesUtil.getLabels(user.getAccountId(), change.getId());
diff --git a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
index 9203134..b8b411f 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
@@ -24,6 +24,7 @@
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeFinder;
import com.google.gerrit.server.CurrentUser;
@@ -32,10 +33,13 @@
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import java.io.IOException;
import java.util.List;
@Singleton
@@ -49,6 +53,7 @@
private final CreateChange createChange;
private final ChangeResource.Factory changeResourceFactory;
private final PermissionBackend permissionBackend;
+ private final ProjectCache projectCache;
@Inject
ChangesCollection(
@@ -59,7 +64,8 @@
ChangeFinder changeFinder,
CreateChange createChange,
ChangeResource.Factory changeResourceFactory,
- PermissionBackend permissionBackend) {
+ PermissionBackend permissionBackend,
+ ProjectCache projectCache) {
this.db = db;
this.user = user;
this.queryFactory = queryFactory;
@@ -68,6 +74,7 @@
this.createChange = createChange;
this.changeResourceFactory = changeResourceFactory;
this.permissionBackend = permissionBackend;
+ this.projectCache = projectCache;
}
@Override
@@ -82,7 +89,7 @@
@Override
public ChangeResource parse(TopLevelResource root, IdString id)
- throws RestApiException, OrmException, PermissionBackendException {
+ throws RestApiException, OrmException, PermissionBackendException, IOException {
List<ChangeNotes> notes = changeFinder.find(id.encoded(), true);
if (notes.isEmpty()) {
throw new ResourceNotFoundException(id);
@@ -94,11 +101,12 @@
if (!canRead(change)) {
throw new ResourceNotFoundException(id);
}
+ checkProjectStatePermitsRead(change.getProjectName());
return changeResourceFactory.create(change, user.get());
}
public ChangeResource parse(Change.Id id)
- throws ResourceNotFoundException, OrmException, PermissionBackendException {
+ throws RestApiException, OrmException, PermissionBackendException, IOException {
List<ChangeNotes> notes = changeFinder.find(id);
if (notes.isEmpty()) {
throw new ResourceNotFoundException(toIdString(id));
@@ -110,6 +118,7 @@
if (!canRead(change)) {
throw new ResourceNotFoundException(toIdString(id));
}
+ checkProjectStatePermitsRead(change.getProjectName());
return changeResourceFactory.create(change, user.get());
}
@@ -134,4 +143,13 @@
return false;
}
}
+
+ private void checkProjectStatePermitsRead(Project.NameKey project)
+ throws IOException, RestApiException {
+ ProjectState projectState = projectCache.checkedGet(project);
+ if (projectState == null) {
+ throw new ResourceNotFoundException("project not found: " + project.get());
+ }
+ projectState.checkStatePermitsRead();
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java
index aac16660..85ad78f 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java
@@ -40,7 +40,6 @@
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RemoveReviewerControl;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -121,7 +120,7 @@
@Override
public boolean updateChange(ChangeContext ctx)
throws AuthException, ResourceNotFoundException, OrmException, PermissionBackendException,
- IOException, NoSuchProjectException {
+ IOException {
Account.Id reviewerId = reviewer.getId();
// Check of removing this reviewer (even if there is no vote processed by the loop below) is OK
removeReviewerControl.checkRemoveReviewer(ctx.getNotes(), ctx.getUser(), reviewerId);
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
index 268425e..e0ae92d3 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
@@ -44,7 +44,6 @@
import com.google.gerrit.server.mail.send.DeleteVoteSender;
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.RemoveReviewerControl;
@@ -166,7 +165,7 @@
@Override
public boolean updateChange(ChangeContext ctx)
throws OrmException, AuthException, ResourceNotFoundException, IOException,
- PermissionBackendException, NoSuchProjectException {
+ PermissionBackendException {
change = ctx.getChange();
PatchSet.Id psId = change.currentPatchSetId();
ps = psUtil.current(db.get(), ctx.getNotes());
diff --git a/java/com/google/gerrit/server/restapi/config/ConfigRestModule.java b/java/com/google/gerrit/server/restapi/config/ConfigRestModule.java
index 71b2f9c..90fa5c4 100644
--- a/java/com/google/gerrit/server/restapi/config/ConfigRestModule.java
+++ b/java/com/google/gerrit/server/restapi/config/ConfigRestModule.java
@@ -41,6 +41,8 @@
put(CONFIG_KIND, "preferences").to(SetPreferences.class);
get(CONFIG_KIND, "preferences.diff").to(GetDiffPreferences.class);
put(CONFIG_KIND, "preferences.diff").to(SetDiffPreferences.class);
+ get(CONFIG_KIND, "preferences.edit").to(GetEditPreferences.class);
+ put(CONFIG_KIND, "preferences.edit").to(SetEditPreferences.class);
put(CONFIG_KIND, "email.confirm").to(ConfirmEmail.class);
}
}
diff --git a/java/com/google/gerrit/server/restapi/config/GetDiffPreferences.java b/java/com/google/gerrit/server/restapi/config/GetDiffPreferences.java
index cc264db..222f292 100644
--- a/java/com/google/gerrit/server/restapi/config/GetDiffPreferences.java
+++ b/java/com/google/gerrit/server/restapi/config/GetDiffPreferences.java
@@ -14,22 +14,18 @@
package com.google.gerrit.server.restapi.config;
-import static com.google.gerrit.server.config.ConfigUtil.loadSection;
-
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.account.VersionedAccountPreferences;
+import com.google.gerrit.server.account.PreferencesConfig;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.UserConfigSections;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Repository;
@Singleton
@@ -47,25 +43,8 @@
@Override
public DiffPreferencesInfo apply(ConfigResource configResource)
throws BadRequestException, ResourceConflictException, IOException, ConfigInvalidException {
- return readFromGit(gitManager, allUsersName, null);
- }
-
- static DiffPreferencesInfo readFromGit(
- GitRepositoryManager gitMgr, AllUsersName allUsersName, DiffPreferencesInfo in)
- throws IOException, ConfigInvalidException, RepositoryNotFoundException {
- try (Repository git = gitMgr.openRepository(allUsersName)) {
- // Load all users prefs.
- VersionedAccountPreferences dp = VersionedAccountPreferences.forDefault();
- dp.load(git);
- DiffPreferencesInfo allUserPrefs = new DiffPreferencesInfo();
- loadSection(
- dp.getConfig(),
- UserConfigSections.DIFF,
- null,
- allUserPrefs,
- DiffPreferencesInfo.defaults(),
- in);
- return allUserPrefs;
+ try (Repository git = gitManager.openRepository(allUsersName)) {
+ return PreferencesConfig.readDefaultDiffPreferences(git);
}
}
}
diff --git a/java/com/google/gerrit/server/restapi/config/GetEditPreferences.java b/java/com/google/gerrit/server/restapi/config/GetEditPreferences.java
new file mode 100644
index 0000000..bd8174e
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/config/GetEditPreferences.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2018 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.config;
+
+import com.google.gerrit.extensions.client.EditPreferencesInfo;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.account.PreferencesConfig;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.ConfigResource;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Repository;
+
+@Singleton
+public class GetEditPreferences implements RestReadView<ConfigResource> {
+ private final AllUsersName allUsersName;
+ private final GitRepositoryManager gitManager;
+
+ @Inject
+ GetEditPreferences(GitRepositoryManager gitManager, AllUsersName allUsersName) {
+ this.allUsersName = allUsersName;
+ this.gitManager = gitManager;
+ }
+
+ @Override
+ public EditPreferencesInfo apply(ConfigResource configResource)
+ throws BadRequestException, ResourceConflictException, IOException, ConfigInvalidException {
+ try (Repository git = gitManager.openRepository(allUsersName)) {
+ return PreferencesConfig.readDefaultEditPreferences(git);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/config/GetPreferences.java b/java/com/google/gerrit/server/restapi/config/GetPreferences.java
index 3c7453c..e81ce24 100644
--- a/java/com/google/gerrit/server/restapi/config/GetPreferences.java
+++ b/java/com/google/gerrit/server/restapi/config/GetPreferences.java
@@ -41,7 +41,7 @@
public GeneralPreferencesInfo apply(ConfigResource rsrc)
throws IOException, ConfigInvalidException {
try (Repository git = gitMgr.openRepository(allUsersName)) {
- return PreferencesConfig.readDefaultPreferences(git);
+ return PreferencesConfig.readDefaultGeneralPreferences(git);
}
}
}
diff --git a/java/com/google/gerrit/server/restapi/config/SetDiffPreferences.java b/java/com/google/gerrit/server/restapi/config/SetDiffPreferences.java
index e4f8782..ca7b739 100644
--- a/java/com/google/gerrit/server/restapi/config/SetDiffPreferences.java
+++ b/java/com/google/gerrit/server/restapi/config/SetDiffPreferences.java
@@ -14,28 +14,24 @@
package com.google.gerrit.server.restapi.config;
-import static com.google.gerrit.server.config.ConfigUtil.loadSection;
import static com.google.gerrit.server.config.ConfigUtil.skipField;
-import static com.google.gerrit.server.config.ConfigUtil.storeSection;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.server.account.VersionedAccountPreferences;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.PreferencesConfig;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.ConfigResource;
-import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
-import com.google.gerrit.server.git.UserConfigSections;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.lang.reflect.Field;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -46,48 +42,33 @@
private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
private final AllUsersName allUsersName;
- private final GitRepositoryManager gitManager;
+ private final AccountCache accountCache;
@Inject
SetDiffPreferences(
- GitRepositoryManager gitManager,
Provider<MetaDataUpdate.User> metaDataUpdateFactory,
- AllUsersName allUsersName) {
- this.gitManager = gitManager;
+ AllUsersName allUsersName,
+ AccountCache accountCache) {
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.allUsersName = allUsersName;
+ this.accountCache = accountCache;
}
@Override
- public DiffPreferencesInfo apply(ConfigResource configResource, DiffPreferencesInfo in)
+ public DiffPreferencesInfo apply(ConfigResource configResource, DiffPreferencesInfo input)
throws BadRequestException, IOException, ConfigInvalidException {
- if (in == null) {
+ if (input == null) {
throw new BadRequestException("input must be provided");
}
- if (!hasSetFields(in)) {
+ if (!hasSetFields(input)) {
throw new BadRequestException("unsupported option");
}
- return writeToGit(GetDiffPreferences.readFromGit(gitManager, allUsersName, in));
- }
- private DiffPreferencesInfo writeToGit(DiffPreferencesInfo in)
- throws RepositoryNotFoundException, IOException, ConfigInvalidException {
- DiffPreferencesInfo out = new DiffPreferencesInfo();
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
- VersionedAccountPreferences prefs = VersionedAccountPreferences.forDefault();
- prefs.load(md);
- DiffPreferencesInfo defaults = DiffPreferencesInfo.defaults();
- storeSection(prefs.getConfig(), UserConfigSections.DIFF, null, in, defaults);
- prefs.commit(md);
- loadSection(
- prefs.getConfig(),
- UserConfigSections.DIFF,
- null,
- out,
- DiffPreferencesInfo.defaults(),
- null);
+ DiffPreferencesInfo updatedPrefs = PreferencesConfig.updateDefaultDiffPreferences(md, input);
+ accountCache.evictAllNoReindex();
+ return updatedPrefs;
}
- return out;
}
private static boolean hasSetFields(DiffPreferencesInfo in) {
diff --git a/java/com/google/gerrit/server/restapi/config/SetEditPreferences.java b/java/com/google/gerrit/server/restapi/config/SetEditPreferences.java
new file mode 100644
index 0000000..f17881c
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/config/SetEditPreferences.java
@@ -0,0 +1,89 @@
+// Copyright (C) 2018 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.config;
+
+import static com.google.gerrit.server.config.ConfigUtil.skipField;
+
+import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.extensions.annotations.RequiresCapability;
+import com.google.gerrit.extensions.client.EditPreferencesInfo;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.PreferencesConfig;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.ConfigResource;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
+@Singleton
+public class SetEditPreferences implements RestModifyView<ConfigResource, EditPreferencesInfo> {
+ private static final Logger log = LoggerFactory.getLogger(SetDiffPreferences.class);
+
+ private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
+ private final AllUsersName allUsersName;
+ private final AccountCache accountCache;
+
+ @Inject
+ SetEditPreferences(
+ Provider<MetaDataUpdate.User> metaDataUpdateFactory,
+ AllUsersName allUsersName,
+ AccountCache accountCache) {
+ this.metaDataUpdateFactory = metaDataUpdateFactory;
+ this.allUsersName = allUsersName;
+ this.accountCache = accountCache;
+ }
+
+ @Override
+ public EditPreferencesInfo apply(ConfigResource configResource, EditPreferencesInfo input)
+ throws BadRequestException, IOException, ConfigInvalidException {
+ if (input == null) {
+ throw new BadRequestException("input must be provided");
+ }
+ if (!hasSetFields(input)) {
+ throw new BadRequestException("unsupported option");
+ }
+
+ try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
+ EditPreferencesInfo updatedPrefs = PreferencesConfig.updateDefaultEditPreferences(md, input);
+ accountCache.evictAllNoReindex();
+ return updatedPrefs;
+ }
+ }
+
+ private static boolean hasSetFields(EditPreferencesInfo in) {
+ try {
+ for (Field field : in.getClass().getDeclaredFields()) {
+ if (skipField(field)) {
+ continue;
+ }
+ if (field.get(in) != null) {
+ return true;
+ }
+ }
+ } catch (IllegalAccessException e) {
+ log.warn("Unable to verify input", e);
+ }
+ return false;
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/config/SetPreferences.java b/java/com/google/gerrit/server/restapi/config/SetPreferences.java
index be990e2..84ef7fe 100644
--- a/java/com/google/gerrit/server/restapi/config/SetPreferences.java
+++ b/java/com/google/gerrit/server/restapi/config/SetPreferences.java
@@ -62,7 +62,8 @@
}
PreferencesConfig.validateMy(input.my);
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
- GeneralPreferencesInfo updatedPrefs = PreferencesConfig.updateDefaultPreferences(md, input);
+ GeneralPreferencesInfo updatedPrefs =
+ PreferencesConfig.updateDefaultGeneralPreferences(md, input);
accountCache.evictAllNoReindex();
return updatedPrefs;
}
diff --git a/java/com/google/gerrit/server/restapi/project/BranchesCollection.java b/java/com/google/gerrit/server/restapi/project/BranchesCollection.java
index b90bd9c..b10a90a 100644
--- a/java/com/google/gerrit/server/restapi/project/BranchesCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/BranchesCollection.java
@@ -20,6 +20,7 @@
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -71,7 +72,8 @@
@Override
public BranchResource parse(ProjectResource parent, IdString id)
- throws ResourceNotFoundException, IOException, PermissionBackendException {
+ throws RestApiException, IOException, PermissionBackendException {
+ parent.getProjectState().checkStatePermitsRead();
Project.NameKey project = parent.getNameKey();
try (Repository repo = repoManager.openRepository(project)) {
Ref ref = repo.exactRef(RefNames.fullName(id.get()));
diff --git a/java/com/google/gerrit/server/restapi/project/ChildProjectsCollection.java b/java/com/google/gerrit/server/restapi/project/ChildProjectsCollection.java
index ee572fe..9f6676d 100644
--- a/java/com/google/gerrit/server/restapi/project/ChildProjectsCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/ChildProjectsCollection.java
@@ -56,6 +56,7 @@
@Override
public ChildProjectResource parse(ProjectResource parent, IdString id)
throws RestApiException, IOException, PermissionBackendException {
+ parent.getProjectState().checkStatePermitsRead();
ProjectResource p = projectsCollection.parse(TopLevelResource.INSTANCE, id);
for (ProjectState pp : p.getProjectState().parents()) {
if (parent.getNameKey().equals(pp.getProject().getNameKey())) {
diff --git a/java/com/google/gerrit/server/restapi/project/CommitsCollection.java b/java/com/google/gerrit/server/restapi/project/CommitsCollection.java
index 6aa639f..3eb0e26 100644
--- a/java/com/google/gerrit/server/restapi/project/CommitsCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/CommitsCollection.java
@@ -18,6 +18,7 @@
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -70,7 +71,8 @@
@Override
public CommitResource parse(ProjectResource parent, IdString id)
- throws ResourceNotFoundException, IOException {
+ throws RestApiException, IOException {
+ parent.getProjectState().checkStatePermitsRead();
ObjectId objectId;
try {
objectId = ObjectId.fromString(id.get());
diff --git a/java/com/google/gerrit/server/restapi/project/DashboardsCollection.java b/java/com/google/gerrit/server/restapi/project/DashboardsCollection.java
index e3ffa4d..2f2ace8 100644
--- a/java/com/google/gerrit/server/restapi/project/DashboardsCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/DashboardsCollection.java
@@ -99,6 +99,7 @@
@Override
public RestModifyView<ProjectResource, ?> create(ProjectResource parent, IdString id)
throws RestApiException {
+ parent.getProjectState().checkStatePermitsWrite();
if (isDefaultDashboard(id)) {
return createDefault.get();
}
@@ -107,8 +108,8 @@
@Override
public DashboardResource parse(ProjectResource parent, IdString id)
- throws ResourceNotFoundException, IOException, ConfigInvalidException,
- PermissionBackendException {
+ throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
+ parent.getProjectState().checkStatePermitsRead();
if (isDefaultDashboard(id)) {
return DashboardResource.projectDefault(parent.getProjectState(), parent.getUser());
}
diff --git a/java/com/google/gerrit/server/restapi/project/GetDashboard.java b/java/com/google/gerrit/server/restapi/project/GetDashboard.java
index dde77e5..2ec67e7 100644
--- a/java/com/google/gerrit/server/restapi/project/GetDashboard.java
+++ b/java/com/google/gerrit/server/restapi/project/GetDashboard.java
@@ -81,8 +81,7 @@
}
private DashboardResource defaultOf(ProjectState projectState, CurrentUser user)
- throws ResourceNotFoundException, IOException, ConfigInvalidException,
- PermissionBackendException {
+ throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
String id = projectState.getProject().getLocalDefaultDashboard();
if (Strings.isNullOrEmpty(id)) {
id = projectState.getProject().getDefaultDashboard();
@@ -107,8 +106,7 @@
}
private DashboardResource parse(ProjectState projectState, CurrentUser user, String id)
- throws ResourceNotFoundException, IOException, ConfigInvalidException,
- PermissionBackendException {
+ throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
List<String> p = Lists.newArrayList(Splitter.on(':').limit(2).split(id));
String ref = Url.encode(p.get(0));
String path = Url.encode(p.get(1));
diff --git a/java/com/google/gerrit/server/restapi/project/ListBranches.java b/java/com/google/gerrit/server/restapi/project/ListBranches.java
index 416a5c2..05b6def 100644
--- a/java/com/google/gerrit/server/restapi/project/ListBranches.java
+++ b/java/com/google/gerrit/server/restapi/project/ListBranches.java
@@ -22,8 +22,8 @@
import com.google.gerrit.extensions.common.ActionInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.webui.UiAction;
@@ -134,8 +134,8 @@
@Override
public List<BranchInfo> apply(ProjectResource rsrc)
- throws ResourceNotFoundException, IOException, BadRequestException,
- PermissionBackendException {
+ throws RestApiException, IOException, PermissionBackendException {
+ rsrc.getProjectState().checkStatePermitsRead();
return new RefFilter<BranchInfo>(Constants.R_HEADS)
.subString(matchSubstring)
.regex(matchRegex)
diff --git a/java/com/google/gerrit/server/restapi/project/ListChildProjects.java b/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
index c514f90..5727c6b 100644
--- a/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
@@ -17,6 +17,7 @@
import static java.util.stream.Collectors.toList;
import com.google.gerrit.extensions.common.ProjectInfo;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
@@ -69,7 +70,9 @@
}
@Override
- public List<ProjectInfo> apply(ProjectResource rsrc) throws PermissionBackendException {
+ public List<ProjectInfo> apply(ProjectResource rsrc)
+ throws PermissionBackendException, ResourceConflictException {
+ rsrc.getProjectState().checkStatePermitsRead();
if (recursive) {
return childProjects.list(rsrc.getNameKey());
}
diff --git a/java/com/google/gerrit/server/restapi/project/ListTags.java b/java/com/google/gerrit/server/restapi/project/ListTags.java
index af4586c..a038bfe 100644
--- a/java/com/google/gerrit/server/restapi/project/ListTags.java
+++ b/java/com/google/gerrit/server/restapi/project/ListTags.java
@@ -18,9 +18,9 @@
import com.google.gerrit.extensions.api.projects.ProjectApi.ListRefsRequest;
import com.google.gerrit.extensions.api.projects.TagInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
-import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CommonConverters;
@@ -130,7 +130,9 @@
@Override
public List<TagInfo> apply(ProjectResource resource)
- throws IOException, ResourceNotFoundException, BadRequestException {
+ throws IOException, ResourceNotFoundException, RestApiException {
+ resource.getProjectState().checkStatePermitsRead();
+
List<TagInfo> tags = new ArrayList<>();
PermissionBackend.ForProject perm = permissionBackend.user(user).project(resource.getNameKey());
diff --git a/java/com/google/gerrit/server/restapi/project/TagsCollection.java b/java/com/google/gerrit/server/restapi/project/TagsCollection.java
index a1b0395..c503094 100644
--- a/java/com/google/gerrit/server/restapi/project/TagsCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/TagsCollection.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.TagResource;
@@ -50,9 +51,10 @@
}
@Override
- public TagResource parse(ProjectResource rsrc, IdString id)
- throws ResourceNotFoundException, IOException {
- return new TagResource(rsrc.getProjectState(), rsrc.getUser(), list.get().get(rsrc, id));
+ public TagResource parse(ProjectResource parent, IdString id)
+ throws RestApiException, IOException {
+ parent.getProjectState().checkStatePermitsRead();
+ return new TagResource(parent.getProjectState(), parent.getUser(), list.get().get(parent, id));
}
@Override
diff --git a/java/com/google/gerrit/sshd/ChangeArgumentParser.java b/java/com/google/gerrit/sshd/ChangeArgumentParser.java
index f442032..961860c 100644
--- a/java/com/google/gerrit/sshd/ChangeArgumentParser.java
+++ b/java/com/google/gerrit/sshd/ChangeArgumentParser.java
@@ -15,7 +15,7 @@
package com.google.gerrit.sshd;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -32,6 +32,7 @@
import com.google.gerrit.sshd.BaseCommand.UnloggedFailure;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -62,13 +63,13 @@
}
public void addChange(String id, Map<Change.Id, ChangeResource> changes)
- throws UnloggedFailure, OrmException, PermissionBackendException {
+ throws UnloggedFailure, OrmException, PermissionBackendException, IOException {
addChange(id, changes, null);
}
public void addChange(
String id, Map<Change.Id, ChangeResource> changes, ProjectState projectState)
- throws UnloggedFailure, OrmException, PermissionBackendException {
+ throws UnloggedFailure, OrmException, PermissionBackendException, IOException {
addChange(id, changes, projectState, true);
}
@@ -77,7 +78,7 @@
Map<Change.Id, ChangeResource> changes,
ProjectState projectState,
boolean useIndex)
- throws UnloggedFailure, OrmException, PermissionBackendException {
+ throws UnloggedFailure, OrmException, PermissionBackendException, IOException {
List<ChangeNotes> matched = useIndex ? changeFinder.find(id) : changeFromNotesFactory(id);
List<ChangeNotes> toAdd = new ArrayList<>(changes.size());
boolean canMaintainServer;
@@ -109,7 +110,7 @@
ChangeResource changeResource;
try {
changeResource = changesCollection.parse(cId);
- } catch (ResourceNotFoundException e) {
+ } catch (RestApiException e) {
throw new UnloggedFailure(1, "\"" + id + "\" no such change");
}
changes.put(cId, changeResource);
diff --git a/java/com/google/gerrit/sshd/commands/AdminSetParent.java b/java/com/google/gerrit/sshd/commands/AdminSetParent.java
index d9a0a37..fce4337 100644
--- a/java/com/google/gerrit/sshd/commands/AdminSetParent.java
+++ b/java/com/google/gerrit/sshd/commands/AdminSetParent.java
@@ -19,6 +19,7 @@
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.common.ProjectInfo;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.git.MetaDataUpdate;
@@ -133,6 +134,8 @@
childProjects.addAll(getChildrenForReparenting(oldParent));
} catch (PermissionBackendException e) {
throw new Failure(1, "permissions unavailable", e);
+ } catch (RestApiException e) {
+ throw new Failure(1, "failure in request", e);
}
}
@@ -196,7 +199,7 @@
* reparenting.
*/
private List<Project.NameKey> getChildrenForReparenting(ProjectState parent)
- throws PermissionBackendException {
+ throws PermissionBackendException, RestApiException {
final List<Project.NameKey> childProjects = new ArrayList<>();
final List<Project.NameKey> excluded = new ArrayList<>(excludedChildren.size());
for (ProjectState excludedChild : excludedChildren) {
diff --git a/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java b/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
index 3ed856b..9a98257 100644
--- a/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
+++ b/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
@@ -24,6 +24,7 @@
import com.google.gerrit.sshd.SshCommand;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import java.io.IOException;
import java.util.LinkedHashMap;
import java.util.Map;
import org.kohsuke.args4j.Argument;
@@ -44,7 +45,7 @@
void addChange(String token) {
try {
changeArgumentParser.addChange(token, changes, null, false);
- } catch (UnloggedFailure | OrmException | PermissionBackendException e) {
+ } catch (UnloggedFailure | OrmException | PermissionBackendException | IOException e) {
writeError("warning", e.getMessage());
}
}
diff --git a/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java b/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
index 4c7d59d..c777afa 100644
--- a/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
+++ b/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
@@ -30,6 +30,7 @@
import com.google.gerrit.sshd.SshCommand;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.LinkedHashMap;
@@ -76,7 +77,7 @@
void addChange(String token) {
try {
changeArgumentParser.addChange(token, changes, projectState);
- } catch (UnloggedFailure e) {
+ } catch (IOException | UnloggedFailure e) {
throw new IllegalArgumentException(e.getMessage(), e);
} catch (OrmException e) {
throw new IllegalArgumentException("database is down", e);
diff --git a/java/com/google/gerrit/sshd/commands/UploadArchive.java b/java/com/google/gerrit/sshd/commands/UploadArchive.java
index 95dbb40..acb2eca 100644
--- a/java/com/google/gerrit/sshd/commands/UploadArchive.java
+++ b/java/com/google/gerrit/sshd/commands/UploadArchive.java
@@ -203,15 +203,16 @@
} catch (GitAPIException e) {
throw new Failure(7, "fatal: git api exception, " + e);
}
- } catch (Failure f) {
- // Report the error in ERROR sideband channel
+ } catch (Throwable t) {
+ // Report the error in ERROR sideband channel. Catch Throwable too so we can also catch
+ // NoClassDefFound.
try (SideBandOutputStream sidebandError =
new SideBandOutputStream(
SideBandOutputStream.CH_ERROR, SideBandOutputStream.MAX_BUF, out)) {
- sidebandError.write(f.getMessage().getBytes(UTF_8));
+ sidebandError.write(t.getMessage().getBytes(UTF_8));
sidebandError.flush();
}
- throw f;
+ throw t;
} finally {
// In any case, cleanly close the packetOut channel
packetOut.end();
diff --git a/java/com/google/gerrit/testing/FakeAccountCache.java b/java/com/google/gerrit/testing/FakeAccountCache.java
index 4a94997..58cc48f 100644
--- a/java/com/google/gerrit/testing/FakeAccountCache.java
+++ b/java/com/google/gerrit/testing/FakeAccountCache.java
@@ -14,10 +14,8 @@
package com.google.gerrit.testing;
-import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
@@ -79,11 +77,6 @@
}
private static AccountState newState(Account account) {
- return new AccountState(
- new AllUsersName(AllUsersNameProvider.DEFAULT),
- account,
- ImmutableSet.of(),
- new HashMap<>(),
- GeneralPreferencesInfo.defaults());
+ return AccountState.forAccount(new AllUsersName(AllUsersNameProvider.DEFAULT), account);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 4b1b5d0..62d07c1 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.acceptance.GitUtil.deleteRef;
import static com.google.gerrit.acceptance.GitUtil.fetch;
import static com.google.gerrit.gpg.PublicKeyStore.REFS_GPG_KEYS;
@@ -46,6 +47,7 @@
import com.google.common.collect.Iterables;
import com.google.common.io.BaseEncoding;
import com.google.common.util.concurrent.AtomicLongMap;
+import com.google.common.util.concurrent.Runnables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AccountCreator;
import com.google.gerrit.acceptance.GerritConfig;
@@ -372,14 +374,14 @@
Account.Id accountId = new Account.Id(seq.nextAccountId());
String fullName = "Foo";
ExternalId extId = ExternalId.createEmail(accountId, "foo@example.com");
- Account account =
+ AccountState accountState =
accountsUpdate
.create()
.insert(
"Create Account Atomically",
accountId,
u -> u.setFullName(fullName).addExternalId(extId));
- assertThat(account.getFullName()).isEqualTo(fullName);
+ assertThat(accountState.getAccount().getFullName()).isEqualTo(fullName);
AccountInfo info = gApi.accounts().id(accountId.get()).get();
assertThat(info.name).isEqualTo(fullName);
@@ -400,12 +402,12 @@
public void updateNonExistingAccount() throws Exception {
Account.Id nonExistingAccountId = new Account.Id(999999);
AtomicBoolean consumerCalled = new AtomicBoolean();
- Account account =
+ Optional<AccountState> accountState =
accountsUpdate
.create()
.update(
"Update Non-Existing Account", nonExistingAccountId, a -> consumerCalled.set(true));
- assertThat(account).isNull();
+ assertThat(accountState).isEmpty();
assertThat(consumerCalled.get()).isFalse();
}
@@ -415,11 +417,12 @@
assertUserBranchWithoutAccountConfig(anonymousCoward.getId());
String status = "OOO";
- Account account =
+ Optional<AccountState> accountState =
accountsUpdate
.create()
.update("Set status", anonymousCoward.getId(), u -> u.setStatus(status));
- assertThat(account).isNotNull();
+ assertThat(accountState).isPresent();
+ Account account = accountState.get().getAccount();
assertThat(account.getFullName()).isNull();
assertThat(account.getStatus()).isEqualTo(status);
assertUserBranch(anonymousCoward.getId(), null, status);
@@ -1935,18 +1938,21 @@
@Test
public void checkMetaId() throws Exception {
// metaId is set when account is loaded
- assertThat(accounts.get(admin.getId()).getAccount().getMetaId())
+ assertThat(accounts.get(admin.getId()).get().getAccount().getMetaId())
.isEqualTo(getMetaId(admin.getId()));
// metaId is set when account is created
AccountsUpdate au = accountsUpdate.create();
Account.Id accountId = new Account.Id(seq.nextAccountId());
- Account account = au.insert("Create Test Account", accountId, u -> {});
- assertThat(account.getMetaId()).isEqualTo(getMetaId(accountId));
+ AccountState accountState = au.insert("Create Test Account", accountId, u -> {});
+ assertThat(accountState.getAccount().getMetaId()).isEqualTo(getMetaId(accountId));
// metaId is set when account is updated
- Account updatedAccount = au.update("Set Full Name", accountId, u -> u.setFullName("foo"));
- assertThat(account.getMetaId()).isNotEqualTo(updatedAccount.getMetaId());
+ Optional<AccountState> updatedAccountState =
+ au.update("Set Full Name", accountId, u -> u.setFullName("foo"));
+ assertThat(updatedAccountState.isPresent()).isTrue();
+ Account updatedAccount = updatedAccountState.get().getAccount();
+ assertThat(accountState.getAccount().getMetaId()).isNotEqualTo(updatedAccount.getMetaId());
assertThat(updatedAccount.getMetaId()).isEqualTo(getMetaId(accountId));
}
@@ -2021,6 +2027,7 @@
gitReferenceUpdated,
null,
allUsers,
+ externalIds,
metaDataUpdateInternalFactory,
new RetryHelper(
cfg,
@@ -2040,15 +2047,19 @@
// Ignore, the successful update of the account is asserted later
}
}
- });
+ },
+ Runnables.doNothing());
assertThat(doneBgUpdate.get()).isFalse();
AccountInfo accountInfo = gApi.accounts().id(admin.id.get()).get();
assertThat(accountInfo.status).isNull();
assertThat(accountInfo.name).isNotEqualTo(fullName);
- Account updatedAccount = update.update("Set Full Name", admin.id, u -> u.setFullName(fullName));
+ Optional<AccountState> updatedAccountState =
+ update.update("Set Full Name", admin.id, u -> u.setFullName(fullName));
assertThat(doneBgUpdate.get()).isTrue();
+ assertThat(updatedAccountState.isPresent()).isTrue();
+ Account updatedAccount = updatedAccountState.get().getAccount();
assertThat(updatedAccount.getStatus()).isEqualTo(status);
assertThat(updatedAccount.getFullName()).isEqualTo(fullName);
@@ -2069,6 +2080,7 @@
gitReferenceUpdated,
null,
allUsers,
+ externalIds,
metaDataUpdateInternalFactory,
new RetryHelper(
cfg,
@@ -2093,7 +2105,8 @@
} catch (IOException | ConfigInvalidException | OrmException e) {
// Ignore, the expected exception is asserted later
}
- });
+ },
+ Runnables.doNothing());
assertThat(bgCounter.get()).isEqualTo(0);
AccountInfo accountInfo = gApi.accounts().id(admin.id.get()).get();
assertThat(accountInfo.status).isNull();
@@ -2107,7 +2120,7 @@
}
assertThat(bgCounter.get()).isEqualTo(status.size());
- Account updatedAccount = accounts.get(admin.id).getAccount();
+ Account updatedAccount = accounts.get(admin.id).get().getAccount();
assertThat(updatedAccount.getStatus()).isEqualTo(Iterables.getLast(status));
assertThat(updatedAccount.getFullName()).isEqualTo(admin.fullName);
@@ -2117,6 +2130,156 @@
}
@Test
+ public void atomicReadMofifyWrite() throws Exception {
+ gApi.accounts().id(admin.id.get()).setStatus("A-1");
+
+ AtomicInteger bgCounterA1 = new AtomicInteger(0);
+ AtomicInteger bgCounterA2 = new AtomicInteger(0);
+ PersonIdent ident = serverIdent.get();
+ AccountsUpdate update =
+ new AccountsUpdate(
+ repoManager,
+ gitReferenceUpdated,
+ null,
+ allUsers,
+ externalIds,
+ metaDataUpdateInternalFactory,
+ new RetryHelper(
+ cfg,
+ retryMetrics,
+ null,
+ null,
+ null,
+ r -> r.withBlockStrategy(noSleepBlockStrategy)),
+ extIdNotesFactory,
+ ident,
+ ident,
+ Runnables.doNothing(),
+ () -> {
+ try {
+ accountsUpdate.create().update("Set Status", admin.id, u -> u.setStatus("A-2"));
+ } catch (IOException | ConfigInvalidException | OrmException e) {
+ // Ignore, the expected exception is asserted later
+ }
+ });
+ assertThat(bgCounterA1.get()).isEqualTo(0);
+ assertThat(bgCounterA2.get()).isEqualTo(0);
+ assertThat(gApi.accounts().id(admin.id.get()).get().status).isEqualTo("A-1");
+
+ Optional<AccountState> updatedAccountState =
+ update.update(
+ "Set Status",
+ admin.id,
+ (a, u) -> {
+ if ("A-1".equals(a.getAccount().getStatus())) {
+ bgCounterA1.getAndIncrement();
+ u.setStatus("B-1");
+ }
+
+ if ("A-2".equals(a.getAccount().getStatus())) {
+ bgCounterA2.getAndIncrement();
+ u.setStatus("B-2");
+ }
+ });
+
+ assertThat(bgCounterA1.get()).isEqualTo(1);
+ assertThat(bgCounterA2.get()).isEqualTo(1);
+
+ assertThat(updatedAccountState.isPresent()).isTrue();
+ assertThat(updatedAccountState.get().getAccount().getStatus()).isEqualTo("B-2");
+ assertThat(accounts.get(admin.id).get().getAccount().getStatus()).isEqualTo("B-2");
+ assertThat(gApi.accounts().id(admin.id.get()).get().status).isEqualTo("B-2");
+ }
+
+ @Test
+ public void atomicReadMofifyWriteExternalIds() throws Exception {
+ allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
+
+ Account.Id accountId = new Account.Id(seq.nextAccountId());
+ ExternalId extIdA1 = ExternalId.create("foo", "A-1", accountId);
+ accountsUpdate.create().insert("Create Test Account", accountId, u -> u.addExternalId(extIdA1));
+
+ AtomicInteger bgCounterA1 = new AtomicInteger(0);
+ AtomicInteger bgCounterA2 = new AtomicInteger(0);
+ PersonIdent ident = serverIdent.get();
+ ExternalId extIdA2 = ExternalId.create("foo", "A-2", accountId);
+ AccountsUpdate update =
+ new AccountsUpdate(
+ repoManager,
+ gitReferenceUpdated,
+ null,
+ allUsers,
+ externalIds,
+ metaDataUpdateInternalFactory,
+ new RetryHelper(
+ cfg,
+ retryMetrics,
+ null,
+ null,
+ null,
+ r -> r.withBlockStrategy(noSleepBlockStrategy)),
+ extIdNotesFactory,
+ ident,
+ ident,
+ Runnables.doNothing(),
+ () -> {
+ try {
+ accountsUpdate
+ .create()
+ .update(
+ "Update External ID",
+ accountId,
+ u -> u.replaceExternalId(extIdA1, extIdA2));
+ } catch (IOException | ConfigInvalidException | OrmException e) {
+ // Ignore, the expected exception is asserted later
+ }
+ });
+ assertThat(bgCounterA1.get()).isEqualTo(0);
+ assertThat(bgCounterA2.get()).isEqualTo(0);
+ assertThat(
+ gApi.accounts()
+ .id(accountId.get())
+ .getExternalIds()
+ .stream()
+ .map(i -> i.identity)
+ .collect(toSet()))
+ .containsExactly(extIdA1.key().get());
+
+ ExternalId extIdB1 = ExternalId.create("foo", "B-1", accountId);
+ ExternalId extIdB2 = ExternalId.create("foo", "B-2", accountId);
+ Optional<AccountState> updatedAccount =
+ update.update(
+ "Update External ID",
+ accountId,
+ (a, u) -> {
+ if (a.getExternalIds().contains(extIdA1)) {
+ bgCounterA1.getAndIncrement();
+ u.replaceExternalId(extIdA1, extIdB1);
+ }
+
+ if (a.getExternalIds().contains(extIdA2)) {
+ bgCounterA2.getAndIncrement();
+ u.replaceExternalId(extIdA2, extIdB2);
+ }
+ });
+
+ assertThat(bgCounterA1.get()).isEqualTo(1);
+ assertThat(bgCounterA2.get()).isEqualTo(1);
+
+ assertThat(updatedAccount.isPresent()).isTrue();
+ assertThat(updatedAccount.get().getExternalIds()).containsExactly(extIdB2);
+ assertThat(accounts.get(accountId).get().getExternalIds()).containsExactly(extIdB2);
+ assertThat(
+ gApi.accounts()
+ .id(accountId.get())
+ .getExternalIds()
+ .stream()
+ .map(i -> i.identity)
+ .collect(toSet()))
+ .containsExactly(extIdB2.key().get());
+ }
+
+ @Test
public void stalenessChecker() throws Exception {
// Newly created account is not stale.
AccountInfo accountInfo = gApi.accounts().create(name("foo")).get();
diff --git a/javatests/com/google/gerrit/acceptance/api/config/EditPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/config/EditPreferencesIT.java
new file mode 100644
index 0000000..e89aa3d
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/config/EditPreferencesIT.java
@@ -0,0 +1,47 @@
+// Copyright (C) 2018 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.api.config;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.AssertUtil.assertPrefs;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.extensions.client.EditPreferencesInfo;
+import org.junit.Test;
+
+@NoHttpd
+public class EditPreferencesIT extends AbstractDaemonTest {
+
+ @Test
+ public void getEditPreferences() throws Exception {
+ EditPreferencesInfo result = gApi.config().server().getDefaultEditPreferences();
+ assertPrefs(result, EditPreferencesInfo.defaults());
+ }
+
+ @Test
+ public void setEditPreferences() throws Exception {
+ int newLineLength = EditPreferencesInfo.defaults().lineLength + 10;
+ EditPreferencesInfo update = new EditPreferencesInfo();
+ update.lineLength = newLineLength;
+ EditPreferencesInfo result = gApi.config().server().setDefaultEditPreferences(update);
+ assertThat(result.lineLength).named("lineLength").isEqualTo(newLineLength);
+
+ result = gApi.config().server().getDefaultEditPreferences();
+ EditPreferencesInfo expected = EditPreferencesInfo.defaults();
+ expected.lineLength = newLineLength;
+ assertPrefs(result, expected);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/ssh/BUILD b/javatests/com/google/gerrit/acceptance/ssh/BUILD
index ec386a5..87b2920 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/BUILD
+++ b/javatests/com/google/gerrit/acceptance/ssh/BUILD
@@ -4,5 +4,6 @@
srcs = glob(["*IT.java"]),
group = "ssh",
labels = ["ssh"],
+ vm_args = ["-Xmx512m"],
deps = ["//lib/commons:compress"],
)
diff --git a/javatests/com/google/gerrit/acceptance/ssh/UploadArchiveIT.java b/javatests/com/google/gerrit/acceptance/ssh/UploadArchiveIT.java
index f4f81f8..aa7a987 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/UploadArchiveIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/UploadArchiveIT.java
@@ -51,7 +51,7 @@
@Test
@GerritConfig(name = "download.archive", value = "off")
public void archiveFeatureOff() throws Exception {
- archiveNotPermitted();
+ assertArchiveNotPermitted();
}
@Test
@@ -60,14 +60,14 @@
values = {"tar", "tbz2", "tgz", "txz"}
)
public void zipFormatDisabled() throws Exception {
- archiveNotPermitted();
+ assertArchiveNotPermitted();
}
@Test
public void zipFormat() throws Exception {
PushOneCommit.Result r = createChange();
String abbreviated = r.getCommit().abbreviate(8).name();
- String c = command(r, abbreviated);
+ String c = command(r, "zip", abbreviated);
InputStream out =
adminSshSession.exec2("git-upload-archive " + project.get(), argumentsToInputStream(c));
@@ -99,23 +99,49 @@
.inOrder();
}
- private String command(PushOneCommit.Result r, String abbreviated) {
+ // Make sure we have coverage for the dependency on xz.
+ @Test
+ public void txzFormat() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String abbreviated = r.getCommit().abbreviate(8).name();
+ String c = command(r, "tar.xz", abbreviated);
+
+ try (InputStream out =
+ adminSshSession.exec2("git-upload-archive " + project.get(), argumentsToInputStream(c))) {
+
+ // Wrap with PacketLineIn to read ACK bytes from output stream
+ PacketLineIn in = new PacketLineIn(out);
+ String packet = in.readString();
+ assertThat(packet).isEqualTo("ACK");
+
+ // Discard first bit of data, which should be empty.
+ packet = in.readString();
+ assertThat(packet).isEmpty();
+
+ // Make sure the next one is not on the error channel
+ packet = in.readString();
+
+ // 1 = DATA. It would be nicer to parse the OutputStream with SideBandInputStream from JGit, but
+ // that is currently not public.
+ char channel = packet.charAt(0);
+ if (channel != 1) {
+ fail("got packet on channel " + (int) channel, packet);
+ }
+ }
+ }
+
+ private String command(PushOneCommit.Result r, String format, String abbreviated) {
String c =
- "-f=zip "
- + "-9 "
- + "--prefix="
- + abbreviated
- + "/ "
- + r.getCommit().name()
- + " "
- + PushOneCommit.FILE_NAME;
+ String.format(
+ "-f=%s --prefix=%s/ %s %s",
+ format, abbreviated, r.getCommit().name(), PushOneCommit.FILE_NAME);
return c;
}
- private void archiveNotPermitted() throws Exception {
+ private void assertArchiveNotPermitted() throws Exception {
PushOneCommit.Result r = createChange();
String abbreviated = r.getCommit().abbreviate(8).name();
- String c = command(r, abbreviated);
+ String c = command(r, "zip", abbreviated);
InputStream out =
adminSshSession.exec2("git-upload-archive " + project.get(), argumentsToInputStream(c));
diff --git a/javatests/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/javatests/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
index d130c20..1e5ca4f 100644
--- a/javatests/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
+++ b/javatests/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
@@ -40,7 +40,6 @@
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.externalids.ExternalId;
-import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -55,7 +54,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
-import java.util.Set;
import org.bouncycastle.openpgp.PGPPublicKey;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
@@ -85,8 +83,6 @@
@Inject private ThreadLocalRequestContext requestContext;
- @Inject private ExternalIds externalIds;
-
private LifecycleManager lifecycle;
private ReviewDb db;
private Account.Id userId;
@@ -222,10 +218,12 @@
@Test
public void noExternalIds() throws Exception {
- Set<ExternalId> extIds = externalIds.byAccount(user.getAccountId());
accountsUpdate
.create()
- .update("Delete External IDs", user.getAccountId(), u -> u.deleteExternalIds(extIds));
+ .update(
+ "Delete External IDs",
+ user.getAccountId(),
+ (a, u) -> u.deleteExternalIds(a.getExternalIds()));
reloadUser();
TestKey key = validKeyWithSecondUserId();
diff --git a/javatests/com/google/gerrit/server/account/WatchConfigTest.java b/javatests/com/google/gerrit/server/account/WatchConfigTest.java
index cf61de2..fd2c108 100644
--- a/javatests/com/google/gerrit/server/account/WatchConfigTest.java
+++ b/javatests/com/google/gerrit/server/account/WatchConfigTest.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.account.WatchConfig.NotifyType;
@@ -52,7 +53,7 @@
+ "[project \"otherProject\"]\n"
+ " notify = [NEW_PATCHSETS]\n"
+ " notify = * [NEW_PATCHSETS, ALL_COMMENTS]\n");
- Map<ProjectWatchKey, Set<NotifyType>> projectWatches =
+ Map<ProjectWatchKey, ImmutableSet<NotifyType>> projectWatches =
WatchConfig.parse(new Account.Id(1000000), cfg, this);
assertThat(validationErrors).isEmpty();
diff --git a/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java b/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java
index 1542fe5..f0230d5 100644
--- a/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java
+++ b/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java
@@ -18,12 +18,10 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.AccountState;
@@ -43,14 +41,7 @@
String metaId = "0e39795bb25dc914118224995c53c5c36923a461";
account.setMetaId(metaId);
List<String> values =
- toStrings(
- AccountField.REF_STATE.get(
- new AccountState(
- allUsersName,
- account,
- ImmutableSet.of(),
- ImmutableMap.of(),
- GeneralPreferencesInfo.defaults())));
+ toStrings(AccountField.REF_STATE.get(AccountState.forAccount(allUsersName, account)));
assertThat(values).hasSize(1);
String expectedValue =
allUsersName.get() + ":" + RefNames.refsUsers(account.getId()) + ":" + metaId;
@@ -78,12 +69,7 @@
List<String> values =
toStrings(
AccountField.EXTERNAL_ID_STATE.get(
- new AccountState(
- null,
- account,
- ImmutableSet.of(extId1, extId2),
- ImmutableMap.of(),
- GeneralPreferencesInfo.defaults())));
+ AccountState.forAccount(null, account, ImmutableSet.of(extId1, extId2))));
String expectedValue1 = extId1.key().sha1().name() + ":" + extId1.blobId().name();
String expectedValue2 = extId2.key().sha1().name() + ":" + extId2.blobId().name();
assertThat(values).containsExactly(expectedValue1, expectedValue2);
diff --git a/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java b/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java
index 01e8225..ae01caf 100644
--- a/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java
+++ b/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java
@@ -22,7 +22,6 @@
import static org.easymock.EasyMock.verify;
import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
@@ -30,8 +29,6 @@
import com.google.gerrit.server.config.AllUsersNameProvider;
import com.google.gerrit.server.mail.Address;
import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
import java.util.List;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
@@ -385,11 +382,6 @@
final Account account = new Account(userId, TimeUtil.nowTs());
account.setFullName(name);
account.setPreferredEmail(email);
- return new AccountState(
- new AllUsersName(AllUsersNameProvider.DEFAULT),
- account,
- Collections.emptySet(),
- new HashMap<>(),
- GeneralPreferencesInfo.defaults());
+ return AccountState.forAccount(new AllUsersName(AllUsersNameProvider.DEFAULT), account);
}
}
diff --git a/javatests/com/google/gerrit/server/project/RefControlTest.java b/javatests/com/google/gerrit/server/project/RefControlTest.java
index 8892a50..643519b 100644
--- a/javatests/com/google/gerrit/server/project/RefControlTest.java
+++ b/javatests/com/google/gerrit/server/project/RefControlTest.java
@@ -208,7 +208,6 @@
@Inject private SingleVersionListener singleVersionListener;
@Inject private InMemoryDatabase schemaFactory;
@Inject private ThreadLocalRequestContext requestContext;
- @Inject private ProjectControl.Factory projectControlFactory;
@Before
public void setUp() throws Exception {
@@ -856,7 +855,6 @@
projectCache,
allProjectsName,
allUsersName,
- projectControlFactory,
envFactory,
repoManager,
rulesCache,
diff --git a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html
index 14662dd..350df7d 100644
--- a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html
+++ b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html
@@ -21,6 +21,7 @@
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
+<link rel="import" href="../../shared/gr-icons/gr-icons.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-permission/gr-permission.html">
@@ -36,22 +37,31 @@
fieldset {
border: 1px solid #d1d2d3;
}
+ .name {
+ align-items: center;
+ display: flex;
+ }
.header,
.editingRef .editContainer,
#deletedContainer {
- align-items: baseline;
+ align-items: center;
background: #f6f6f6;
border-bottom: 1px dotted #d1d2d3;
display: flex;
justify-content: space-between;
- padding: .7em .7em;
+ min-height: 3em;
+ padding: 0 .7em;
}
#deletedContainer {
border-bottom: 0;
}
+ #editRefInput {
+ width: 70%;
+ }
.sectionContent {
padding: .7em;
}
+ #editBtn,
#deletedContainer,
.deleted #mainContainer,
.global,
@@ -61,6 +71,9 @@
.editContainer {
display: none;
}
+ .editing #editBtn {
+ display: flex;
+ }
/* TODO @beckysiegel add back when editing allowed */
/* .deleted #deletedContainer,
#mainContainer,
@@ -77,15 +90,18 @@
class$="gr-form-styles [[_computeSectionClass(editing, _editingRef, _deleted)]]">
<div id="mainContainer">
<div class="header">
- <span class="name">
+ <div class="name">
<h3>[[_computeSectionName(section.id)]]</h3>
- </span>
- <div id="updateBtns">
<gr-button
id="editBtn"
+ link
class$="[[_computeEditBtnClass(section.id)]]"
- on-tap="_handleEditReference">Edit Reference</gr-button>
- <gr-button
+ on-tap="_handleEditReference">
+ <iron-icon id="icon" icon="gr-icons:create"></iron-icon>
+ </gr-button>
+ </div>
+ <div id="updateBtns">
+ <gr-button
id="deleteBtn"
on-tap="_handleRemoveReference">Remove</gr-button>
</div><!-- end updateBtns -->
@@ -95,10 +111,8 @@
id="editRefInput"
bind-value="{{section.id}}"
is="iron-input"
- type="text">
- <gr-button
- id="undoEdit"
- on-tap="_undoReferenceEdit">Undo</gr-button>
+ type="text"
+ on-input="_handleValueChange">
</div><!-- end editContainer -->
<div class="sectionContent">
<template
diff --git a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.js b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.js
index 07a7a62..c603828 100644
--- a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.js
+++ b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.js
@@ -31,13 +31,14 @@
section: {
type: Object,
notify: true,
- observer: '_sectionChanged',
+ observer: '_updateSection',
},
groups: Object,
labels: Object,
editing: {
type: Boolean,
value: false,
+ observer: '_handleEditingChanged',
},
_originalId: String,
_editingRef: {
@@ -55,11 +56,40 @@
Gerrit.AccessBehavior,
],
- _sectionChanged(section) {
+ listeners: {
+ 'access-saved': '_handleAccessSaved',
+ },
+
+ _updateSection(section) {
this._permissions = this.toSortedArray(section.value.permissions);
this._originalId = section.id;
},
+ _handleAccessSaved() {
+ // Set a new 'original' value to keep track of after the value has been
+ // saved.
+ this._updateSection(this.section);
+ },
+
+ _handleValueChange() {
+ this.section.value.modified = this.section.id !== this._originalId;
+ this.section.value.updatedId = this.section.id;
+
+ // Allows overall access page to know a change has been made.
+ this.dispatchEvent(new CustomEvent('access-modified', {bubbles: true}));
+ },
+
+ _handleEditingChanged(editing, editingOld) {
+ // Ignore when editing gets set initially.
+ if (!editingOld) { return; }
+ // Restore original values if no longer editing.
+ if (!editing) {
+ this._editingRef = false;
+ // Restore section ref.
+ this.set(['section', 'id'], this._originalId);
+ }
+ },
+
_computePermissions(name, capabilities, labels) {
let allPermissions;
if (name === GLOBAL_NAME) {
@@ -141,11 +171,6 @@
this._editingRef = true;
},
- _undoReferenceEdit() {
- this._editingRef = false;
- this.set('section.id', this._originalId);
- },
-
_computeSectionClass(editing, editingRef, deleted) {
const classList = [];
if (editing) {
diff --git a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section_test.html b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section_test.html
index 38c5d67..fb07f7a 100644
--- a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section_test.html
@@ -88,12 +88,12 @@
default_value: 0,
},
};
- element._sectionChanged(element.section);
+ element._updateSection(element.section);
flushAsynchronousOperations();
});
- test('_sectionChanged', () => {
- // _sectionChanged was called in setup, so just make assertions.
+ test('_updateSection', () => {
+ // _updateSection was called in setup, so just make assertions.
const expectedPermissions = [
{
id: 'read',
@@ -128,6 +128,13 @@
expectedLabelOptions);
});
+ test('_handleAccessSaved', () => {
+ assert.equal(element._originalId, 'refs/*');
+ element.section.id = 'refs/for/bar';
+ element._handleAccessSaved();
+ assert.equal(element._originalId, 'refs/for/bar');
+ });
+
test('_computePermissions', () => {
sandbox.stub(element, 'toSortedArray').returns(
[{
@@ -262,15 +269,6 @@
assert.isTrue(element._editingRef);
});
- test('_undoReferenceEdit', () => {
- element._originalId = 'refs/for/old';
- element.section.id = 'refs/for/new';
- element.editing = true;
- element._undoReferenceEdit();
- assert.isFalse(element._editingRef);
- assert.equal(element.section.id, 'refs/for/old');
- });
-
test('_computeSectionClass', () => {
let editingRef = false;
let editing = false;
@@ -348,7 +346,7 @@
name: 'Create Account',
},
};
- element._sectionChanged(element.section);
+ element._updateSection(element.section);
flushAsynchronousOperations();
});
@@ -372,7 +370,7 @@
},
};
element.capabilities = {};
- element._sectionChanged(element.section);
+ element._updateSection(element.section);
flushAsynchronousOperations();
});
@@ -429,6 +427,7 @@
});
test('edit section reference', () => {
+ element.section = {id: 'refs/for/bar', value: {permissions: {}}};
assert.isFalse(element.$.section.classList.contains('editing'));
element.editing = true;
assert.isTrue(element.$.section.classList.contains('editing'));
@@ -439,11 +438,25 @@
assert.equal(element.section.id, 'new/ref');
assert.isTrue(element._editingRef);
assert.isTrue(element.$.section.classList.contains('editingRef'));
- MockInteractions.tap(element.$.undoEdit);
- flushAsynchronousOperations();
+ element.editing = false;
assert.isFalse(element._editingRef);
- assert.isFalse(element.$.section.classList.contains('editingRef'));
- assert.equal(element.section.id, 'refs/*');
+ assert.equal(element.section.id, 'refs/for/bar');
+ });
+
+ test('_handleValueChange', () => {
+ const modifiedHandler = sandbox.stub();
+ element.section = {id: 'refs/for/bar', value: {permissions: {}}};
+ assert.notOk(element.section.value.updatedId);
+ element.section.id = 'refs/for/baz';
+ element.addEventListener('access-modified', modifiedHandler);
+ assert.isNotOk(element.section.value.modified);
+ element._handleValueChange();
+ assert.equal(element.section.value.updatedId, 'refs/for/baz');
+ assert.isTrue(element.section.value.modified);
+ assert.isTrue(modifiedHandler.called);
+ element.section.id = 'refs/for/bar';
+ element._handleValueChange();
+ assert.isFalse(element.section.value.modified);
});
test('remove section', () => {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
index d903404..bf5ad0b 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
@@ -16,6 +16,14 @@
const Defs = {};
+ const NOTHING_TO_SAVE = 'No changes to save.';
+
+ /**
+ * Fired when save is a no-op
+ *
+ * @event show-alert
+ */
+
/**
* @typedef {{
* value: !Object,
@@ -42,14 +50,14 @@
/**
* Can be an empty object or consist of permissions.
*
- * @typedef {Object<string, Defs.permissions>}
+ * @typedef {!Object<string, Defs.permissions>}
*/
Defs.sections;
/**
* @typedef {{
- * remove: Defs.sections,
- * add: Defs.sections,
+ * remove: !Defs.sections,
+ * add: !Defs.sections,
* }}
*/
Defs.projectAccessInput;
@@ -156,13 +164,12 @@
for (const item of path) {
if (!curPos[item]) {
if (item === path[path.length - 1] && type === 'remove') {
- // TODO(beckysiegel) This if statement should be removed when
- // https://gerrit-review.googlesource.com/c/gerrit/+/150851
- // is live.
if (path[path.length - 2] === 'permissions') {
curPos[item] = {rules: {}};
+ } else if (path.length === 1) {
+ curPos[item] = {permissions: {}};
} else {
- curPos[item] = null;
+ curPos[item] = {};
}
} else if (item === path[path.length - 1] && type === 'add') {
curPos[item] = opt_value;
@@ -175,6 +182,22 @@
return addRemoveObj;
},
+ /**
+ * Used to recursively remove any objects with a 'deleted' bit.
+ */
+ _recursivelyRemoveDeleted(obj) {
+ for (const k in obj) {
+ if (!obj.hasOwnProperty(k)) { return; }
+ if (typeof obj[k] == 'object') {
+ if (obj[k].deleted) {
+ delete obj[k];
+ return;
+ }
+ this._recursivelyRemoveDeleted(obj[k]);
+ }
+ }
+ },
+
_recursivelyUpdateAddRemoveObj(obj, addRemoveObj, path = []) {
for (const k in obj) {
if (!obj.hasOwnProperty(k)) { return; }
@@ -186,11 +209,24 @@
} else if (obj[k].modified) {
this._updateAddRemoveObj(addRemoveObj,
path.concat(k), 'remove');
- this._updateAddRemoveObj(addRemoveObj,
- path.concat(k), 'add', obj[k]);
+
+ const updatedId = obj[k].updatedId;
+ const ref = updatedId ? updatedId : k;
+ this._updateAddRemoveObj(addRemoveObj, path.concat(ref), 'add',
+ obj[k]);
+ /* Special case for ref changes because they need to be added and
+ removed in a different way. The new ref needs to include all
+ changes but also the initial state. To do this, instead of
+ continuing with the same recursion, just remove anything that is
+ deleted in the current state. */
+ if (updatedId && updatedId !== k) {
+ this._recursivelyRemoveDeleted(addRemoveObj.add[updatedId]);
+ }
+ continue;
} else if (obj[k].added) {
this._updateAddRemoveObj(addRemoveObj,
path.concat(k), 'add', obj[k]);
+ continue;
}
this._recursivelyUpdateAddRemoveObj(obj[k], addRemoveObj,
path.concat(k));
@@ -215,9 +251,15 @@
},
_handleSaveForReview() {
- // Use saving rather than editing here because rules have to handle
- // save prior to toggling editing.
const addRemoveObj = this._computeAddAndRemove();
+
+ // If there are no changes, don't actually save.
+ if (!Object.keys(addRemoveObj.add).length &&
+ !Object.keys(addRemoveObj.remove).length) {
+ this.dispatchEvent(new CustomEvent('show-alert',
+ {detail: {message: NOTHING_TO_SAVE}, bubbles: true}));
+ return;
+ }
return this.$.restAPI.setProjectAccessRightsForReview(this.repo, {
add: addRemoveObj.add,
remove: addRemoveObj.remove,
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
index b26121c..12d2e4a 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
@@ -227,7 +227,52 @@
assert.isTrue(element._handleAccessModified.called);
});
- test('_computeAddAndRemove rules', () => {
+ test('_handleAccessModified called with event fired', () => {
+ const saveStub =
+ sandbox.stub(element.$.restAPI, 'setProjectAccessRightsForReview');
+ sandbox.stub(element, '_computeAddAndRemove').returns({
+ add: {},
+ remove: {},
+ });
+ element._handleSaveForReview();
+ assert.isFalse(saveStub.called);
+ });
+
+ test('_recursivelyRemoveDeleted', () => {
+ const obj = {
+ 'refs/*': {
+ permissions: {
+ owner: {
+ rules: {
+ 234: {action: 'ALLOW'},
+ 123: {action: 'DENY', deleted: true},
+ },
+ },
+ read: {
+ deleted: true,
+ rules: {
+ 234: {action: 'ALLOW'},
+ },
+ },
+ },
+ },
+ };
+ const expectedResult = {
+ 'refs/*': {
+ permissions: {
+ owner: {
+ rules: {
+ 234: {action: 'ALLOW'},
+ },
+ },
+ },
+ },
+ };
+ element._recursivelyRemoveDeleted(obj);
+ assert.deepEqual(obj, expectedResult);
+ });
+
+ test('_handleSaveForReview with no changes', () => {
element._local = JSON.parse(JSON.stringify(accessRes.local));
assert.deepEqual(element._computeAddAndRemove(), {add: {}, remove: {}});
element._local['refs/*'].permissions.owner.rules[123].deleted = true;
@@ -238,7 +283,7 @@
permissions: {
owner: {
rules: {
- 123: null,
+ 123: {},
},
},
},
@@ -265,7 +310,7 @@
permissions: {
owner: {
rules: {
- 123: null,
+ 123: {},
},
},
},
@@ -317,6 +362,40 @@
assert.deepEqual(element._computeAddAndRemove(), expectedInput);
});
+ test('_computeAddAndRemove sections', () => {
+ element._local = JSON.parse(JSON.stringify(accessRes.local));
+ assert.deepEqual(element._computeAddAndRemove(), {add: {}, remove: {}});
+ element._local['refs/*'].updatedId = 'refs/for/bar';
+ element._local['refs/*'].modified = true;
+ const expectedInput = {
+ add: {
+ 'refs/for/bar': {
+ modified: true,
+ updatedId: 'refs/for/bar',
+ permissions: {
+ owner: {
+ rules: {
+ 234: {action: 'ALLOW'},
+ 123: {action: 'DENY'},
+ },
+ },
+ read: {
+ rules: {
+ 234: {action: 'ALLOW'},
+ },
+ },
+ },
+ },
+ },
+ remove: {
+ 'refs/*': {
+ permissions: {},
+ },
+ },
+ };
+ assert.deepEqual(element._computeAddAndRemove(), expectedInput);
+ });
+
test('_computeAddAndRemove combinations', () => {
// Modify rule and delete permission that it is inside of.
element._local = JSON.parse(JSON.stringify(accessRes.local));
@@ -393,6 +472,34 @@
},
};
assert.deepEqual(element._computeAddAndRemove(), expectedInput);
+
+ // Change one of the refs
+ element._local['refs/*'].updatedId = 'refs/for/bar';
+ element._local['refs/*'].modified = true;
+
+ expectedInput = {
+ add: {
+ 'refs/for/bar': {
+ modified: true,
+ updatedId: 'refs/for/bar',
+ permissions: {
+ read: {
+ exclusive: true,
+ modified: true,
+ rules: {
+ 234: {action: 'ALLOW'},
+ },
+ },
+ },
+ },
+ },
+ remove: {
+ 'refs/*': {
+ permissions: {},
+ },
+ },
+ };
+ assert.deepEqual(element._computeAddAndRemove(), expectedInput);
});
test('_handleSaveForReview', done => {
@@ -413,7 +520,7 @@
permissions: {
owner: {
rules: {
- 123: null,
+ 123: {},
},
},
},
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 6370374..5786264 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -94,6 +94,10 @@
},
};
sandbox = sinon.sandbox.create();
+ sandbox.stub(element.$.confirmCherrypick.$.restAPI,
+ 'getRepoBranches').returns(Promise.resolve([]));
+ sandbox.stub(element.$.confirmMove.$.restAPI,
+ 'getRepoBranches').returns(Promise.resolve([]));
return element.reload();
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 5f86514..cec91d5 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -134,7 +134,7 @@
max-width: var(--commit-message-max-width, 72ch);;
}
.commitMessage gr-linked-text {
- word-break: break-all;
+ word-break: break-word;
}
#commitMessageEditor {
min-width: 72ch;
@@ -374,7 +374,7 @@
<div class="changeInfo-column changeMetadata hideOnMobileOverlay">
<gr-change-metadata
change="{{_change}}"
- revision="[[_currentRevision]]"
+ revision="[[_selectedRevision]]"
commit-info="[[_commitInfo]]"
server-config="[[_serverConfig]]"
missing-labels="[[_missingLabels]]"
@@ -510,7 +510,7 @@
<gr-endpoint-decorator name="change-view-integration">
<gr-endpoint-param name="change" value="[[_change]]">
</gr-endpoint-param>
- <gr-endpoint-param name="revision" value="[[_currentRevision]]">
+ <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
</gr-endpoint-param>
</gr-endpoint-decorator>
<gr-messages-list id="messageList"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 1800175..0914fc8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -152,7 +152,8 @@
type: Object,
},
_filesExpanded: String,
- _currentRevision: Object,
+ _basePatchNum: String,
+ _selectedRevision: Object,
_currentRevisionActions: Object,
_allPatchSets: {
type: Array,
@@ -236,6 +237,7 @@
observers: [
'_labelsChanged(_change.labels.*)',
'_paramsAndChangeChanged(params, _change)',
+ '_patchNumChanged(_patchRange.patchNum)',
],
keyBindings: {
@@ -1004,7 +1006,6 @@
parseInt(lineHeight.slice(0, lineHeight.length - 2), 10);
this._change = change;
- this._currentRevision = currentRevision;
if (!this._patchRange || !this._patchRange.patchNum ||
this.patchNumEquals(this._patchRange.patchNum,
currentRevision._number)) {
@@ -1015,7 +1016,13 @@
this._commitInfo = currentRevision.commit;
this._currentRevisionActions =
this._updateRebaseAction(currentRevision.actions);
- // TODO: Fetch and process files.
+ this._selectedRevision = currentRevision;
+ // TODO: Fetch and process files.
+ } else {
+ this._selectedRevision =
+ Object.values(this._change.revisions).find(
+ revision => revision._number ===
+ parseInt(this._patchRange.patchNum, 10));
}
});
},
@@ -1371,5 +1378,17 @@
_computeCommitMessageKey(number, revision) {
return `c${number}_rev${revision}`;
},
+
+ _patchNumChanged(patchNumStr) {
+ if (!this._selectedRevision) {
+ return;
+ }
+ const patchNum = parseInt(patchNumStr, 10);
+ if (patchNum === this._selectedRevision._number) {
+ return;
+ }
+ this._selectedRevision = Object.values(this._change.revisions).find(
+ revision => revision._number === patchNum);
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 7d4a54b..7555a5a 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -1437,10 +1437,34 @@
assert.isTrue(Gerrit.Nav.navigateToRelativeUrl.called);
});
+ test('_selectedRevision updates when patchNum is changed', () => {
+ const revision1 = {_number: 1, commit: {}};
+ const revision2 = {_number: 2, commit: {}};
+ sandbox.stub(element.$.restAPI, 'getChangeDetail').returns(
+ Promise.resolve({
+ revisions: {
+ aaa: revision1,
+ bbb: revision2,
+ },
+ labels: {},
+ actions: {},
+ current_revision: 'bbb',
+ change_id: 'loremipsumdolorsitamet',
+ }));
+ sandbox.stub(element, '_getEdit').returns(Promise.resolve());
+ element._patchRange = {patchNum: '2'};
+ return element._getChangeDetail().then(() => {
+ assert.strictEqual(element._selectedRevision, revision2);
+
+ element.set('_patchRange.patchNum', '1');
+ assert.strictEqual(element._selectedRevision, revision1);
+ });
+ });
+
suite('plugin endpoints', () => {
test('endpoint params', done => {
element._change = {labels: {}};
- element._currentRevision = {};
+ element._selectedRevision = {};
let hookEl;
let plugin;
Gerrit.install(
@@ -1454,7 +1478,7 @@
flush(() => {
assert.strictEqual(hookEl.plugin, plugin);
assert.strictEqual(hookEl.change, element._change);
- assert.strictEqual(hookEl.revision, element._currentRevision);
+ assert.strictEqual(hookEl.revision, element._selectedRevision);
done();
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
index 1b5d6f9..6ce6a1e 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
@@ -55,11 +55,7 @@
}
</style>
<template is="dom-repeat" items="[[_computeFilesFromComments(comments)]]" as="file">
- <div class="file">
- <a href$="[[_computeFileDiffURL(file, changeNum, patchNum)]]">
- [[computeDisplayPath(file)]]
- </a>:
- </div>
+ <div class="file">[[computeDisplayPath(file)]]:</div>
<template is="dom-repeat"
items="[[_computeCommentsForFile(comments, file)]]" as="comment">
<div class="container">
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
index 7fa8c7a..b089ae5 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
@@ -36,11 +36,6 @@
return arr.sort(this.specialFilePathCompare);
},
- _computeFileDiffURL(file, changeNum, patchNum) {
- return Gerrit.Nav.getUrlForDiffById(this.changeNum, this.projectName,
- file, patchNum);
- },
-
_isOnParent(comment) {
return comment.side === 'PARENT';
},
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 8ce0e9d..b1d7b2e 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -58,6 +58,12 @@
:host(.editLoaded) .showOnEdit {
display: initial;
}
+ .controlRow {
+ align-items: center;
+ display: flex;
+ height: 2.25em;
+ justify-content: center;
+ }
.reviewed,
.status {
align-items: center;
@@ -114,7 +120,7 @@
min-width: 7em;
}
.invisible {
- visibility: hidden;
+ display: none;
}
.row:not(.header) .stats,
.total-stats {
@@ -383,25 +389,25 @@
</span>
</div>
</div>
- <gr-button
- class="fileListButton"
- id="incrementButton"
- hidden$="[[_computeFileListButtonHidden(numFilesShown, _files)]]"
- link on-tap="_incrementNumFilesShown">
- [[_computeIncrementText(numFilesShown, _files)]]
- </gr-button>
- <gr-tooltip-content
- has-tooltip="[[_computeWarnShowAll(_files)]]"
- show-icon="[[_computeWarnShowAll(_files)]]"
- title$="[[_computeShowAllWarning(_files)]]">
+ <div class$="row controlRow [[_computeFileListControlClass(numFilesShown, _files)]]">
<gr-button
class="fileListButton"
- id="showAllButton"
- hidden$="[[_computeFileListButtonHidden(numFilesShown, _files)]]"
- link on-tap="_showAllFiles">
- [[_computeShowAllText(_files)]]
- </gr-button><!--
- --></gr-tooltip-content>
+ id="incrementButton"
+ link on-tap="_incrementNumFilesShown">
+ [[_computeIncrementText(numFilesShown, _files)]]
+ </gr-button>
+ <gr-tooltip-content
+ has-tooltip="[[_computeWarnShowAll(_files)]]"
+ show-icon="[[_computeWarnShowAll(_files)]]"
+ title$="[[_computeShowAllWarning(_files)]]">
+ <gr-button
+ class="fileListButton"
+ id="showAllButton"
+ link on-tap="_showAllFiles">
+ [[_computeShowAllText(_files)]]
+ </gr-button><!--
+ --></gr-tooltip-content>
+ </div>
<gr-diff-preferences
id="diffPreferences"
prefs="{{diffPrefs}}"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 66e3db7..70cafe9 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -747,8 +747,8 @@
this.numFilesShown += this.fileListIncrement;
},
- _computeFileListButtonHidden(numFilesShown, files) {
- return numFilesShown >= files.length;
+ _computeFileListControlClass(numFilesShown, files) {
+ return numFilesShown >= files.length ? 'invisible' : '';
},
_computeIncrementText(numFilesShown, files) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 381a304..d04098c 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -96,13 +96,28 @@
});
test('correct number of files are shown', () => {
+ element.fileListIncrement = 300;
element._files = _.times(500, i => {
return {__path: '/file' + i, lines_inserted: 9};
});
flushAsynchronousOperations();
+
assert.equal(
Polymer.dom(element.root).querySelectorAll('.file-row').length,
element.numFilesShown);
+ const controlRow = element.$$('.controlRow');
+ assert.isFalse(controlRow.classList.contains('invisible'));
+ assert.equal(element.$.incrementButton.textContent.trim(),
+ 'Show 300 more');
+ assert.equal(element.$.showAllButton.textContent.trim(),
+ 'Show all 500 files');
+
+ MockInteractions.tap(element.$.showAllButton);
+ flushAsynchronousOperations();
+
+ assert.equal(element.numFilesShown, 500);
+ assert.equal(element._shownFiles.length, 500);
+ assert.isTrue(controlRow.classList.contains('invisible'));
});
test('calculate totals for patch number', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 9ae8ff2..e45702c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -298,7 +298,6 @@
_computeSaveDisabled(draft, comment, resolved) {
// If resolved state has changed and a msg exists, save should be enabled.
if (comment.unresolved === resolved && draft) { return false; }
- if (comment.message) { return draft === comment.message; }
return !draft || draft.trim() === '';
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index 8a2886c..c09e289 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -520,9 +520,9 @@
const msgComment = {message: 'test', unresolved: true};
assert.equal(element._computeSaveDisabled('', comment, false), true);
assert.equal(element._computeSaveDisabled('test', comment, false), false);
- assert.equal(element._computeSaveDisabled('', msgComment, false), false);
+ assert.equal(element._computeSaveDisabled('', msgComment, false), true);
assert.equal(
- element._computeSaveDisabled('test', msgComment, false), true);
+ element._computeSaveDisabled('test', msgComment, false), false);
assert.equal(
element._computeSaveDisabled('test2', msgComment, false), false);
assert.equal(element._computeSaveDisabled('test', comment, true), false);
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
index b43e994..847b49b 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
@@ -23,6 +23,8 @@
<g id="search"><path d="M15.5 14h-.79l-.28-.27C15.41 12.59 16 11.11 16 9.5 16 5.91 13.09 3 9.5 3S3 5.91 3 9.5 5.91 16 9.5 16c1.61 0 3.09-.59 4.23-1.57l.27.28v.79l5 4.99L20.49 19l-4.99-5zm-6 0C7.01 14 5 11.99 5 9.5S7.01 5 9.5 5 14 7.01 14 9.5 11.99 14 9.5 14z"/></g>
<!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.html -->
<g id="settings"><path d="M19.43 12.98c.04-.32.07-.64.07-.98s-.03-.66-.07-.98l2.11-1.65c.19-.15.24-.42.12-.64l-2-3.46c-.12-.22-.39-.3-.61-.22l-2.49 1c-.52-.4-1.08-.73-1.69-.98l-.38-2.65C14.46 2.18 14.25 2 14 2h-4c-.25 0-.46.18-.49.42l-.38 2.65c-.61.25-1.17.59-1.69.98l-2.49-1c-.23-.09-.49 0-.61.22l-2 3.46c-.13.22-.07.49.12.64l2.11 1.65c-.04.32-.07.65-.07.98s.03.66.07.98l-2.11 1.65c-.19.15-.24.42-.12.64l2 3.46c.12.22.39.3.61.22l2.49-1c.52.4 1.08.73 1.69.98l.38 2.65c.03.24.24.42.49.42h4c.25 0 .46-.18.49-.42l.38-2.65c.61-.25 1.17-.59 1.69-.98l2.49 1c.23.09.49 0 .61-.22l2-3.46c.12-.22.07-.49-.12-.64l-2.11-1.65zM12 15.5c-1.93 0-3.5-1.57-3.5-3.5s1.57-3.5 3.5-3.5 3.5 1.57 3.5 3.5-1.57 3.5-3.5 3.5z"/></g>
+ <!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.html -->
+ <g id="create"><path d="M3 17.25V21h3.75L17.81 9.94l-3.75-3.75L3 17.25zM20.71 7.04c.39-.39.39-1.02 0-1.41l-2.34-2.34c-.39-.39-1.02-.39-1.41 0l-1.83 1.83 3.75 3.75 1.83-1.83z"/></g>
<!-- This is a custom PolyGerrit SVG -->
<g id="side-by-side"><path d="M17.1578947,10.8888889 L2.84210526,10.8888889 C2.37894737,10.8888889 2,11.2888889 2,11.7777778 L2,17.1111111 C2,17.6 2.37894737,18 2.84210526,18 L17.1578947,18 C17.6210526,18 18,17.6 18,17.1111111 L18,11.7777778 C18,11.2888889 17.6210526,10.8888889 17.1578947,10.8888889 Z M17.1578947,2 L2.84210526,2 C2.37894737,2 2,2.4 2,2.88888889 L2,8.22222222 C2,8.71111111 2.37894737,9.11111111 2.84210526,9.11111111 L17.1578947,9.11111111 C17.6210526,9.11111111 18,8.71111111 18,8.22222222 L18,2.88888889 C18,2.4 17.6210526,2 17.1578947,2 Z M16.1973628,2 L2.78874238,2 C2.35493407,2 2,2.4 2,2.88888889 L2,8.22222222 C2,8.71111111 2.35493407,9.11111111 2.78874238,9.11111111 L16.1973628,9.11111111 C16.6311711,9.11111111 16.9861052,8.71111111 16.9861052,8.22222222 L16.9861052,2.88888889 C16.9861052,2.4 16.6311711,2 16.1973628,2 Z" id="Shape" transform="scale(1.2) translate(10.000000, 10.000000) rotate(-90.000000) translate(-10.000000, -10.000000)"/></g>
<!-- This is a custom PolyGerrit SVG -->
