opendev/gerrit
Code Issues Proposed changes

Merge "GetPreferences: Don't return unsupported download scheme" into stable-2.16

Browse Source
This commit is contained in:
David Pursehouse 2018-10-25 23:04:56 +00:00 committed by Gerrit Code Review
commit 028798059c
2 changed files with 102 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
java/com/google/gerrit/server/restapi/account/GetPreferences.java
View File

@ -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<AccountResource> {
private final Provider<CurrentUser> self;
private final PermissionBackend permissionBackend;
private final AccountCache accountCache;
private final DynamicMap<DownloadScheme> downloadSchemes;
@Inject
GetPreferences(
Provider<CurrentUser> self, PermissionBackend permissionBackend, AccountCache accountCache) {
Provider<CurrentUser> self,
PermissionBackend permissionBackend,
AccountCache accountCache,
DynamicMap<DownloadScheme> downloadSchemes) {
this.self = self;
this.permissionBackend = permissionBackend;
this.accountCache = accountCache;
this.downloadSchemes = downloadSchemes;
}
@Override
@ -53,9 +61,28 @@ public class GetPreferences implements RestReadView<AccountResource> {
}
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<DownloadScheme> e : downloadSchemes) {
if (e.getExportName().equals(preferencesInfo.downloadScheme)
&& e.getProvider().get().isEnabled()) {
return preferencesInfo;
}
}
preferencesInfo.downloadScheme = null;
return preferencesInfo;
}
}

70
javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
View File

@ -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<DownloadScheme> 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<DownloadScheme>) 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;
}
}
}