diff --git a/java/com/google/gerrit/server/account/AccountManager.java b/java/com/google/gerrit/server/account/AccountManager.java index e2194cca7e..d0bd06994f 100644 --- a/java/com/google/gerrit/server/account/AccountManager.java +++ b/java/com/google/gerrit/server/account/AccountManager.java @@ -248,10 +248,16 @@ public class AccountManager { } } - if (!realm.allowsEdit(AccountFieldName.FULL_NAME) - && !Strings.isNullOrEmpty(who.getDisplayName()) + if (!Strings.isNullOrEmpty(who.getDisplayName()) && !Objects.equals(user.getAccount().getFullName(), who.getDisplayName())) { accountUpdates.add(u -> u.setFullName(who.getDisplayName())); + if (realm.allowsEdit(AccountFieldName.FULL_NAME)) { + accountUpdates.add(a -> a.setFullName(who.getDisplayName())); + } else { + logger.atWarning().log( + "Not changing already set display name '%s' to '%s'", + user.getAccount().getFullName(), who.getDisplayName()); + } } if (!realm.allowsEdit(AccountFieldName.USER_NAME) diff --git a/java/com/google/gerrit/server/restapi/account/GetPreferences.java b/java/com/google/gerrit/server/restapi/account/GetPreferences.java index a185898486..3d2064232f 100644 --- a/java/com/google/gerrit/server/restapi/account/GetPreferences.java +++ b/java/com/google/gerrit/server/restapi/account/GetPreferences.java @@ -15,6 +15,9 @@ package com.google.gerrit.server.restapi.account; import com.google.gerrit.extensions.client.GeneralPreferencesInfo; +import com.google.gerrit.extensions.config.DownloadScheme; +import com.google.gerrit.extensions.registration.DynamicMap; +import com.google.gerrit.extensions.registration.Extension; import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestApiException; @@ -36,13 +39,18 @@ public class GetPreferences implements RestReadView { private final Provider self; private final PermissionBackend permissionBackend; private final AccountCache accountCache; + private final DynamicMap downloadSchemes; @Inject GetPreferences( - Provider self, PermissionBackend permissionBackend, AccountCache accountCache) { + Provider self, + PermissionBackend permissionBackend, + AccountCache accountCache, + DynamicMap downloadSchemes) { this.self = self; this.permissionBackend = permissionBackend; this.accountCache = accountCache; + this.downloadSchemes = downloadSchemes; } @Override @@ -53,9 +61,28 @@ public class GetPreferences implements RestReadView { } Account.Id id = rsrc.getUser().getAccountId(); - return accountCache - .get(id) - .map(AccountState::getGeneralPreferences) - .orElseThrow(() -> new ResourceNotFoundException(IdString.fromDecoded(id.toString()))); + GeneralPreferencesInfo preferencesInfo = + accountCache + .get(id) + .map(AccountState::getGeneralPreferences) + .orElseThrow(() -> new ResourceNotFoundException(IdString.fromDecoded(id.toString()))); + return unsetDownloadSchemeIfUnsupported(preferencesInfo); + } + + private GeneralPreferencesInfo unsetDownloadSchemeIfUnsupported( + GeneralPreferencesInfo preferencesInfo) { + if (preferencesInfo.downloadScheme == null) { + return preferencesInfo; + } + + for (Extension e : downloadSchemes) { + if (e.getExportName().equals(preferencesInfo.downloadScheme) + && e.getProvider().get().isEnabled()) { + return preferencesInfo; + } + } + + preferencesInfo.downloadScheme = null; + return preferencesInfo; } } diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java index f04cefc361..5e46a03126 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -103,9 +103,11 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.Sequences; import com.google.gerrit.server.ServerInitiated; +import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.AccountProperties; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.AccountsUpdate; +import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.account.Emails; import com.google.gerrit.server.account.ProjectWatches; import com.google.gerrit.server.account.ProjectWatches.NotifyType; @@ -229,6 +231,8 @@ public class AccountIT extends AbstractDaemonTest { @Inject private DynamicSet accountActivationValidationListeners; + @Inject private AccountManager accountManager; + private AccountIndexedCounter accountIndexedCounter; private RegistrationHandle accountIndexEventCounterHandle; private RefUpdateCounter refUpdateCounter; @@ -2716,6 +2720,18 @@ public class AccountIT extends AbstractDaemonTest { } } + @Test + public void updateDisplayName() throws Exception { + String name = name("test"); + gApi.accounts().create(name); + AuthRequest who = AuthRequest.forUser(name); + accountManager.authenticate(who); + assertThat(gApi.accounts().id(name).get().name).isEqualTo(name); + who.setDisplayName("Something Else"); + accountManager.authenticate(who); + assertThat(gApi.accounts().id(name).get().name).isEqualTo("Something Else"); + } + private void createDraft(PushOneCommit.Result r, String path, String message) throws Exception { DraftInput in = new DraftInput(); in.path = path; diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java index 946e15c4e0..24040a4136 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java @@ -30,7 +30,13 @@ import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy; import com.google.gerrit.extensions.client.GeneralPreferencesInfo.ReviewCategoryStrategy; import com.google.gerrit.extensions.client.GeneralPreferencesInfo.TimeFormat; import com.google.gerrit.extensions.client.MenuItem; +import com.google.gerrit.extensions.config.DownloadScheme; +import com.google.gerrit.extensions.registration.DynamicMap; +import com.google.gerrit.extensions.registration.PrivateInternals_DynamicMapImpl; +import com.google.gerrit.extensions.registration.RegistrationHandle; import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.inject.Inject; +import com.google.inject.util.Providers; import java.util.ArrayList; import java.util.HashMap; import org.junit.Before; @@ -38,6 +44,8 @@ import org.junit.Test; @NoHttpd public class GeneralPreferencesIT extends AbstractDaemonTest { + @Inject private DynamicMap downloadSchemes; + private TestAccount user42; @Before @@ -177,4 +185,66 @@ public class GeneralPreferencesIT extends AbstractDaemonTest { GeneralPreferencesInfo o = gApi.accounts().id(user42.getId().toString()).setPreferences(i); assertThat(o.my).containsExactly(new MenuItem("name", "url", "_blank", "id")); } + + @Test + public void rejectUnsupportedDownloadScheme() throws Exception { + GeneralPreferencesInfo i = GeneralPreferencesInfo.defaults(); + i.downloadScheme = "foo"; + + exception.expect(BadRequestException.class); + exception.expectMessage("Unsupported download scheme: " + i.downloadScheme); + gApi.accounts().id(user42.getId().toString()).setPreferences(i); + } + + @Test + public void setDownloadScheme() throws Exception { + String schemeName = "foo"; + RegistrationHandle registrationHandle = + ((PrivateInternals_DynamicMapImpl) downloadSchemes) + .put("myPlugin", schemeName, Providers.of(new TestDownloadScheme())); + try { + GeneralPreferencesInfo i = GeneralPreferencesInfo.defaults(); + i.downloadScheme = schemeName; + + GeneralPreferencesInfo o = gApi.accounts().id(user42.getId().toString()).setPreferences(i); + assertThat(o.downloadScheme).isEqualTo(schemeName); + + o = gApi.accounts().id(user42.getId().toString()).getPreferences(); + assertThat(o.downloadScheme).isEqualTo(schemeName); + } finally { + registrationHandle.remove(); + } + } + + @Test + public void unsupportedDownloadSchemeIsNotReturned() throws Exception { + // Set a download scheme and unregister the plugin that provides this download scheme so that it + // becomes unsupported. + setDownloadScheme(); + + GeneralPreferencesInfo o = gApi.accounts().id(user42.getId().toString()).getPreferences(); + assertThat(o.downloadScheme).isNull(); + } + + private static class TestDownloadScheme extends DownloadScheme { + @Override + public String getUrl(String project) { + return "http://foo/" + project; + } + + @Override + public boolean isAuthRequired() { + return false; + } + + @Override + public boolean isAuthSupported() { + return false; + } + + @Override + public boolean isEnabled() { + return true; + } + } }