Merge "Add a reviewer-deleted event"
diff --git a/Documentation/pgm-init.txt b/Documentation/pgm-init.txt
index 715d589..22f9109 100644
--- a/Documentation/pgm-init.txt
+++ b/Documentation/pgm-init.txt
@@ -1,7 +1,7 @@
= init
== NAME
-init - Initialize a new Gerrit server installation
+init - Initialize/Upgrade a Gerrit server installation
== SYNOPSIS
--
@@ -25,13 +25,14 @@
into a newly created `$site_path`.
If run in an existing `$site_path`, init will upgrade some resources
-as necessary.
+(e.g. DB schema, plugins) as necessary.
== OPTIONS
--batch::
- Run in batch mode, skipping interactive prompts. Reasonable
- configuration defaults are chosen based on the whims of
- the Gerrit developers.
+ Run in batch mode, skipping interactive prompts. For a fresh
+ install, reasonable configuration defaults are chosen based
+ on the whims of the Gerrit developers. On upgrades, the settings
+ in gerrit.config are respected.
+
If during a schema migration unused objects (e.g. tables, columns)
are detected they are *not* automatically dropped, but only a list of
diff --git a/Documentation/project-configuration.txt b/Documentation/project-configuration.txt
index 6d7c6d0..2b0db70b 100644
--- a/Documentation/project-configuration.txt
+++ b/Documentation/project-configuration.txt
@@ -53,7 +53,9 @@
The method Gerrit uses to submit a change to a project can be
modified by any project owner through the project console, `Projects` >
-`List` > my/project. The following methods are supported:
+`List` > my/project. In general, a submitted change is only merged if all
+its dependencies are also submitted, with exceptions documented below.
+The following submit types are supported:
[[fast_forward_only]]
* Fast Forward Only
@@ -96,10 +98,12 @@
is also set to the submitter, while the author header retains the
original patch set author.
+
-Note that Gerrit ignores patch set dependencies when operating in
-cherry-pick mode. Submitters must remember to submit changes in
-the right order since inter-change dependencies will not be
-enforced for them.
+Note that Gerrit ignores dependencies between changes when using this
+submit type unless
+link:config-gerrit.html#change.submitWholeTopic[`change.submitWholeTopic`]
+is enabled and depending changes share the same topic. So generally
+submitters must remember to submit changes in the right order when using this
+submit type.
[[rebase_if_necessary]]
* Rebase If Necessary
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 7bbf2be..ae42477 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -641,6 +641,9 @@
The SSH public key must be provided as raw content in the request body.
+Trying to add an SSH key that already exists succeeds, but no new SSH
+key is persisted.
+
.Request
----
POST /accounts/self/sshkeys HTTP/1.0
@@ -1105,11 +1108,13 @@
"changes_per_page": 25,
"show_site_header": true,
"use_flash_clipboard": true,
+ "download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
+ "diff_view": "SIDE_BY_SIDE",
"size_bar_in_change_table": true,
"review_category_strategy": "ABBREV",
- "diff_view": "SIDE_BY_SIDE",
+ "mute_common_path_prefixes": true,
"my": [
{
"url": "#/dashboard/self",
@@ -1159,11 +1164,13 @@
"changes_per_page": 50,
"show_site_header": true,
"use_flash_clipboard": true,
+ "download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
"size_bar_in_change_table": true,
"review_category_strategy": "NAME",
"diff_view": "SIDE_BY_SIDE",
+ "mute_common_path_prefixes": true,
"my": [
{
"url": "#/dashboard/self",
@@ -1207,11 +1214,13 @@
"changes_per_page": 50,
"show_site_header": true,
"use_flash_clipboard": true,
+ "download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
"size_bar_in_change_table": true,
"review_category_strategy": "NAME",
"diff_view": "SIDE_BY_SIDE",
+ "mute_common_path_prefixes": true,
"my": [
{
"url": "#/dashboard/self",
@@ -1944,7 +1953,7 @@
Whether the site header should be shown.
|`use_flash_clipboard` |not set if `false`|
Whether to use the flash clipboard widget.
-|`download_scheme` ||
+|`download_scheme` |optional|
The type of download URL the user prefers to use. May be any key from
the `schemes` map in
link:rest-api-config.html#download-info[DownloadInfo].
@@ -1981,7 +1990,7 @@
|`url_aliases` |optional|
A map of URL path pairs, where the first URL path is an alias for the
second URL path.
-|`email_notifications` ||
+|`email_strategy` ||
The type of email strategy to use. On `ENABLED`, the user will receive emails
from Gerrit. On `CC_ON_OWN_COMMENTS` the user will also receive emails for
their own comments. On `DISABLED` the user will not receive any email
@@ -2039,6 +2048,12 @@
|`url_aliases` |optional|
A map of URL path pairs, where the first URL path is an alias for the
second URL path.
+|`email_strategy` |optional|
+The type of email strategy to use. On `ENABLED`, the user will receive emails
+from Gerrit. On `CC_ON_OWN_COMMENTS` the user will also receive emails for
+their own comments. On `DISABLED` the user will not receive any email
+notifications from Gerrit.
+Allowed values are `ENABLED`, `CC_ON_OWN_COMMENTS`, `DISABLED`.
|============================================
[[query-limit-info]]
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
index ee0688b..3632502 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
@@ -22,13 +22,12 @@
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
-import com.google.gerrit.reviewdb.client.AccountSshKey;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountByEmailCache;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.ssh.SshKeyCache;
-import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -47,18 +46,23 @@
public class AccountCreator {
private final Map<String, TestAccount> accounts;
- private SchemaFactory<ReviewDb> reviewDbProvider;
- private GroupCache groupCache;
- private SshKeyCache sshKeyCache;
- private AccountCache accountCache;
- private AccountByEmailCache byEmailCache;
+ private final SchemaFactory<ReviewDb> reviewDbProvider;
+ private final VersionedAuthorizedKeys.Accessor authorizedKeys;
+ private final GroupCache groupCache;
+ private final SshKeyCache sshKeyCache;
+ private final AccountCache accountCache;
+ private final AccountByEmailCache byEmailCache;
@Inject
- AccountCreator(SchemaFactory<ReviewDb> schema, GroupCache groupCache,
- SshKeyCache sshKeyCache, AccountCache accountCache,
+ AccountCreator(SchemaFactory<ReviewDb> schema,
+ VersionedAuthorizedKeys.Accessor authorizedKeys,
+ GroupCache groupCache,
+ SshKeyCache sshKeyCache,
+ AccountCache accountCache,
AccountByEmailCache byEmailCache) {
accounts = new HashMap<>();
reviewDbProvider = schema;
+ this.authorizedKeys = authorizedKeys;
this.groupCache = groupCache;
this.sshKeyCache = sshKeyCache;
this.accountCache = accountCache;
@@ -66,17 +70,14 @@
}
public synchronized TestAccount create(String username, String email,
- String fullName, String... groups)
- throws OrmException, UnsupportedEncodingException, JSchException {
+ String fullName, String... groups) throws Exception {
TestAccount account = accounts.get(username);
if (account != null) {
return account;
}
try (ReviewDb db = reviewDbProvider.open()) {
Account.Id id = new Account.Id(db.nextAccountId());
- KeyPair sshKey = genSshKey();
- AccountSshKey key =
- new AccountSshKey(new AccountSshKey.Id(id, 1), publicKey(sshKey, email));
+
AccountExternalId extUser =
new AccountExternalId(id, new AccountExternalId.Key(
AccountExternalId.SCHEME_USERNAME, username));
@@ -95,8 +96,6 @@
a.setPreferredEmail(email);
db.accounts().insert(Collections.singleton(a));
- db.accountSshKeys().insert(Collections.singleton(key));
-
if (groups != null) {
for (String n : groups) {
AccountGroup.NameKey k = new AccountGroup.NameKey(n);
@@ -107,7 +106,10 @@
}
}
+ KeyPair sshKey = genSshKey();
+ authorizedKeys.addKey(id, publicKey(sshKey, email));
sshKeyCache.evict(username);
+
accountCache.evictByUsername(username);
byEmailCache.evict(email);
@@ -118,35 +120,29 @@
}
}
- public TestAccount create(String username, String group)
- throws OrmException, UnsupportedEncodingException, JSchException {
+ public TestAccount create(String username, String group) throws Exception {
return create(username, null, username, group);
}
- public TestAccount create(String username)
- throws UnsupportedEncodingException, OrmException, JSchException {
+ public TestAccount create(String username) throws Exception {
return create(username, null, username, (String[]) null);
}
- public TestAccount admin()
- throws UnsupportedEncodingException, OrmException, JSchException {
+ public TestAccount admin() throws Exception {
return create("admin", "admin@example.com", "Administrator",
"Administrators");
}
- public TestAccount admin2()
- throws UnsupportedEncodingException, OrmException, JSchException {
+ public TestAccount admin2() throws Exception {
return create("admin2", "admin2@example.com", "Administrator2",
"Administrators");
}
- public TestAccount user()
- throws UnsupportedEncodingException, OrmException, JSchException {
+ public TestAccount user() throws Exception {
return create("user", "user@example.com", "User");
}
- public TestAccount user2()
- throws UnsupportedEncodingException, OrmException, JSchException {
+ public TestAccount user2() throws Exception {
return create("user2", "user2@example.com", "User2");
}
@@ -169,6 +165,6 @@
throws UnsupportedEncodingException {
ByteArrayOutputStream out = new ByteArrayOutputStream();
sshKey.writePublicKey(out, comment);
- return out.toString(US_ASCII.name());
+ return out.toString(US_ASCII.name()).trim();
}
}
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InProcessProtocol.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InProcessProtocol.java
index c16eed7..14188bd 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InProcessProtocol.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InProcessProtocol.java
@@ -259,11 +259,8 @@
UploadPack up = new UploadPack(repo);
up.setPackConfig(transferConfig.getPackConfig());
up.setTimeout(transferConfig.getTimeout());
-
- if (!ctl.allRefsAreVisible()) {
- up.setAdvertiseRefsHook(new VisibleRefFilter(
- tagCache, changeCache, repo, ctl, dbProvider.get(), true));
- }
+ up.setAdvertiseRefsHook(new VisibleRefFilter(
+ tagCache, changeCache, repo, ctl, dbProvider.get(), true));
List<PreUploadHook> hooks = Lists.newArrayList(preUploadHooks);
hooks.add(uploadValidatorsFactory.create(
ctl.getProject(), repo, "localhost-test"));
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 59f0150..0c2e0c6 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -340,6 +340,7 @@
// The test account should initially have exactly one ssh key
List<SshKeyInfo> info = gApi.accounts().self().listSshKeys();
assertThat(info).hasSize(1);
+ assertSequenceNumbers(info);
SshKeyInfo key = info.get(0);
String inital = AccountCreator.publicKey(admin.sshKey, admin.email);
assertThat(key.sshPublicKey).isEqualTo(inital);
@@ -350,11 +351,35 @@
gApi.accounts().self().addSshKey(newKey);
info = gApi.accounts().self().listSshKeys();
assertThat(info).hasSize(2);
+ assertSequenceNumbers(info);
- // Add an existing key again
+ // Add an existing key (the request succeeds, but the key isn't added again)
gApi.accounts().self().addSshKey(inital);
info = gApi.accounts().self().listSshKeys();
+ assertThat(info).hasSize(2);
+ assertSequenceNumbers(info);
+
+ // Add another new key
+ String newKey2 = AccountCreator.publicKey(
+ AccountCreator.genSshKey(), admin.email);
+ gApi.accounts().self().addSshKey(newKey2);
+ info = gApi.accounts().self().listSshKeys();
assertThat(info).hasSize(3);
+ assertSequenceNumbers(info);
+
+ // Delete second key
+ gApi.accounts().self().deleteSshKey(2);
+ info = gApi.accounts().self().listSshKeys();
+ assertThat(info).hasSize(2);
+ assertThat(info.get(0).seq).isEqualTo(1);
+ assertThat(info.get(1).seq).isEqualTo(3);
+ }
+
+ private void assertSequenceNumbers(List<SshKeyInfo> sshKeys) {
+ int seq = 1;
+ for (SshKeyInfo key : sshKeys) {
+ assertThat(key.seq).isEqualTo(seq++);
+ }
}
private PGPPublicKey getOnlyKeyFromStore(TestKey key) throws Exception {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index bc206b4..5f2af6e 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -32,7 +32,6 @@
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
-import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
@@ -47,14 +46,12 @@
import com.google.gerrit.testutil.TestTimeUtil;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.transport.PushResult;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import java.util.Collection;
-import java.util.List;
import java.util.Set;
public abstract class AbstractPushForReview extends AbstractDaemonTest {
@@ -493,35 +490,21 @@
}
@Test
- public void testPushSameCommitTwiceUsingMagicBranchBaseOption()
- throws Exception {
- grant(Permission.PUSH, project, "refs/heads/master");
- PushOneCommit.Result rBase = pushTo("refs/heads/master");
- rBase.assertOkStatus();
-
- gApi.projects()
- .name(project.get())
- .branch("foo")
- .create(new BranchInput());
+ public void testCreateNewChangeForAllNotInTarget() throws Exception {
+ ProjectConfig config = projectCache.checkedGet(project).getConfig();
+ config.getProject().setCreateNewChangeForAllNotInTarget(InheritableBoolean.TRUE);
+ saveProjectConfig(project, config);
PushOneCommit push =
pushFactory.create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT,
- "b.txt", "anotherContent");
-
+ "a.txt", "content");
PushOneCommit.Result r = push.to("refs/for/master");
r.assertOkStatus();
- PushResult pr = GitUtil.pushHead(
- testRepo, "refs/for/foo%base=" + rBase.getCommit().name(), false, false);
- assertThat(pr.getMessages()).contains("changes: new: 1, refs: 1, done");
-
- List<ChangeInfo> changes = query(r.getCommit().name());
- assertThat(changes).hasSize(2);
- ChangeInfo c1 = get(changes.get(0).id);
- ChangeInfo c2 = get(changes.get(1).id);
- assertThat(c1.project).isEqualTo(c2.project);
- assertThat(c1.branch).isNotEqualTo(c2.branch);
- assertThat(c1.changeId).isEqualTo(c2.changeId);
- assertThat(c1.currentRevision).isEqualTo(c2.currentRevision);
+ push =
+ pushFactory.create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT,
+ "b.txt", "anotherContent");
+ r = push.to("refs/for/master");
+ r.assertOkStatus();
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
index 8228a277..40ea296 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
@@ -692,6 +692,7 @@
PatchSet ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
psUtil.setGroups(ctx.getDb(), ctx.getUpdate(psId), ps,
ImmutableList.<String> of());
+ ctx.bumpLastUpdatedOn(false);
return true;
}
});
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/AccountService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/AccountService.java
index 75872f4..55073d5 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/AccountService.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/AccountService.java
@@ -16,7 +16,6 @@
import com.google.gerrit.common.audit.Audit;
import com.google.gerrit.common.auth.SignInRequired;
-import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gwtjsonrpc.common.AsyncCallback;
import com.google.gwtjsonrpc.common.RemoteJsonService;
@@ -29,11 +28,6 @@
@RpcImpl(version = Version.V2_0)
public interface AccountService extends RemoteJsonService {
- @Audit
- @SignInRequired
- void changeDiffPreferences(DiffPreferencesInfo diffPref,
- AsyncCallback<VoidResult> callback);
-
@SignInRequired
void myProjectWatch(AsyncCallback<List<AccountProjectWatchInfo>> callback);
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/AccountApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
index a45d8d3..a6e54b2 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
@@ -49,6 +49,7 @@
List<SshKeyInfo> listSshKeys() throws RestApiException;
SshKeyInfo addSshKey(String key) throws RestApiException;
+ void deleteSshKey(int seq) throws RestApiException;
Map<String, GpgKeyInfo> listGpgKeys() throws RestApiException;
Map<String, GpgKeyInfo> putGpgKeys(List<String> add, List<String> remove)
@@ -129,6 +130,11 @@
}
@Override
+ public void deleteSshKey(int seq) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public Map<String, GpgKeyInfo> putGpgKeys(List<String> add,
List<String> remove) throws RestApiException {
throw new NotImplementedException();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ListenableAccountDiffPreference.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ListenableAccountDiffPreference.java
deleted file mode 100644
index 0c31f41..0000000
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ListenableAccountDiffPreference.java
+++ /dev/null
@@ -1,55 +0,0 @@
-// Copyright (C) 2010 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.client.ui;
-
-import com.google.gerrit.client.Gerrit;
-import com.google.gerrit.client.account.Util;
-import com.google.gerrit.client.rpc.GerritCallback;
-import com.google.gerrit.extensions.client.DiffPreferencesInfo;
-import com.google.gwtjsonrpc.common.VoidResult;
-
-public class ListenableAccountDiffPreference
- extends ListenableOldValue<DiffPreferencesInfo> {
-
- public ListenableAccountDiffPreference() {
- reset();
- }
-
- public void save(final GerritCallback<VoidResult> cb) {
- if (Gerrit.isSignedIn()) {
- Util.ACCOUNT_SVC.changeDiffPreferences(get(),
- new GerritCallback<VoidResult>() {
- @Override
- public void onSuccess(VoidResult result) {
- Gerrit.setDiffPreferences(get());
- cb.onSuccess(result);
- }
-
- @Override
- public void onFailure(Throwable caught) {
- cb.onFailure(caught);
- }
- });
- }
- }
-
- public void reset() {
- if (Gerrit.isSignedIn() && Gerrit.getDiffPreferences() != null) {
- set(Gerrit.getDiffPreferences());
- } else {
- set(DiffPreferencesInfo.defaults());
- }
- }
-}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java
index bf8fe01..063f22b 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -256,10 +256,8 @@
uploadValidatorsFactory.create(pc.getProject(), repo, request.getRemoteHost());
up.setPreUploadHook(PreUploadHookChain.newChain(
Lists.newArrayList(up.getPreUploadHook(), uploadValidators)));
- if (!pc.allRefsAreVisible()) {
- up.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache,
- repo, pc, db.get(), true));
- }
+ up.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache,
+ repo, pc, db.get(), true));
next.doFilter(request, response);
}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java
index 8d3947f..ce47161 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java
@@ -19,17 +19,12 @@
import com.google.gerrit.common.data.AgreementInfo;
import com.google.gerrit.common.errors.InvalidQueryException;
import com.google.gerrit.common.errors.NoSuchEntityException;
-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.httpd.rpc.BaseServiceImplementation;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountResource;
-import com.google.gerrit.server.account.SetDiffPreferences;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.query.QueryParseException;
@@ -41,9 +36,6 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-
-import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
@@ -55,41 +47,17 @@
private final ProjectControl.Factory projectControlFactory;
private final AgreementInfoFactory.Factory agreementInfoFactory;
private final Provider<ChangeQueryBuilder> queryBuilder;
- private final SetDiffPreferences setDiff;
@Inject
AccountServiceImpl(final Provider<ReviewDb> schema,
final Provider<IdentifiedUser> identifiedUser,
final ProjectControl.Factory projectControlFactory,
final AgreementInfoFactory.Factory agreementInfoFactory,
- final Provider<ChangeQueryBuilder> queryBuilder,
- SetDiffPreferences setDiff) {
+ final Provider<ChangeQueryBuilder> queryBuilder) {
super(schema, identifiedUser);
this.projectControlFactory = projectControlFactory;
this.agreementInfoFactory = agreementInfoFactory;
this.queryBuilder = queryBuilder;
- this.setDiff = setDiff;
- }
-
- @Override
- public void changeDiffPreferences(final DiffPreferencesInfo diffPref,
- AsyncCallback<VoidResult> callback) {
- run(callback, new Action<VoidResult>() {
- @Override
- public VoidResult run(ReviewDb db) throws OrmException {
- if (!getUser().isIdentifiedUser()) {
- throw new IllegalArgumentException("Not authenticated");
- }
- IdentifiedUser me = getUser().asIdentifiedUser();
- try {
- setDiff.apply(new AccountResource(me), diffPref);
- } catch (AuthException | BadRequestException | ConfigInvalidException
- | IOException e) {
- throw new OrmException("Cannot save diff preferences", e);
- }
- return VoidResult.INSTANCE;
- }
- });
}
@Override
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java
index 40a07b4..6cbd40d 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java
@@ -43,14 +43,17 @@
public class InitAdminUser implements InitStep {
private final ConsoleUI ui;
private final InitFlags flags;
+ private final VersionedAuthorizedKeysOnInit.Factory authorizedKeysFactory;
private SchemaFactory<ReviewDb> dbFactory;
@Inject
InitAdminUser(
InitFlags flags,
- ConsoleUI ui) {
+ ConsoleUI ui,
+ VersionedAuthorizedKeysOnInit.Factory authorizedKeysFactory) {
this.flags = flags;
this.ui = ui;
+ this.authorizedKeysFactory = authorizedKeysFactory;
}
@Override
@@ -110,7 +113,10 @@
db.accountGroupMembers().insert(Collections.singleton(m));
if (sshKey != null) {
- db.accountSshKeys().insert(Collections.singleton(sshKey));
+ VersionedAuthorizedKeysOnInit authorizedKeys =
+ authorizedKeysFactory.create(id).load();
+ authorizedKeys.addKey(sshKey.getSshPublicKey());
+ authorizedKeys.save("Added SSH key for initial admin user\n");
}
}
}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitModule.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitModule.java
index 57a1a30..b5aa625 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitModule.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitModule.java
@@ -42,6 +42,7 @@
bind(Libraries.class);
bind(LibraryDownloader.class);
factory(Section.Factory.class);
+ factory(VersionedAuthorizedKeysOnInit.Factory.class);
// Steps are executed in the order listed here.
//
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/VersionedAuthorizedKeysOnInit.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/VersionedAuthorizedKeysOnInit.java
new file mode 100644
index 0000000..9ef6956
--- /dev/null
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/VersionedAuthorizedKeysOnInit.java
@@ -0,0 +1,198 @@
+// Copyright (C) 2016 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.pgm.init;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Strings;
+import com.google.gerrit.pgm.init.api.InitFlags;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountSshKey;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.AuthorizedKeys;
+import com.google.gerrit.server.account.VersionedAuthorizedKeys;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.git.VersionedMetaData;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.RepositoryCache.FileKey;
+import org.eclipse.jgit.revwalk.RevTree;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.util.FS;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+
+public class VersionedAuthorizedKeysOnInit extends VersionedMetaData {
+ public static interface Factory {
+ VersionedAuthorizedKeysOnInit create(Account.Id accountId);
+ }
+
+ private final Account.Id accountId;
+ private final String ref;
+ private final String project;
+ private final SitePaths site;
+ private final InitFlags flags;
+
+ private List<Optional<AccountSshKey>> keys;
+ private ObjectId revision;
+
+ @Inject
+ public VersionedAuthorizedKeysOnInit(
+ AllUsersNameOnInitProvider allUsers,
+ SitePaths site,
+ InitFlags flags,
+ @Assisted Account.Id accountId) {
+
+ this.project = allUsers.get();
+ this.site = site;
+ this.flags = flags;
+ this.accountId = accountId;
+ this.ref = RefNames.refsUsers(accountId);
+ }
+
+ @Override
+ protected String getRefName() {
+ return ref;
+ }
+
+ public VersionedAuthorizedKeysOnInit load()
+ throws IOException, ConfigInvalidException {
+ File path = getPath();
+ if (path != null) {
+ try (Repository repo = new FileRepository(path)) {
+ load(repo);
+ }
+ }
+ return this;
+ }
+
+ private File getPath() {
+ Path basePath = site.resolve(flags.cfg.getString("gerrit", null, "basePath"));
+ if (basePath == null) {
+ throw new IllegalStateException("gerrit.basePath must be configured");
+ }
+ return FileKey.resolve(basePath.resolve(project).toFile(), FS.DETECTED);
+ }
+
+ @Override
+ protected void onLoad() throws IOException, ConfigInvalidException {
+ revision = getRevision();
+ keys = AuthorizedKeys.parse(accountId, readUTF8(AuthorizedKeys.FILE_NAME));
+ }
+
+ public AccountSshKey addKey(String pub) {
+ checkState(keys != null, "SSH keys not loaded yet");
+ int seq = keys.isEmpty() ? 1 : keys.size() + 1;
+ AccountSshKey.Id keyId = new AccountSshKey.Id(accountId, seq);
+ AccountSshKey key =
+ new VersionedAuthorizedKeys.SimpleSshKeyCreator().create(keyId, pub);
+ keys.add(Optional.of(key));
+ return key;
+ }
+
+ public void save(String message) throws IOException {
+ save(new PersonIdent("Gerrit Initialization", "init@gerrit"), message);
+ }
+
+ private void save(PersonIdent ident, String msg) throws IOException {
+ File path = getPath();
+ if (path == null) {
+ throw new IOException(project + " does not exist.");
+ }
+
+ try (Repository repo = new FileRepository(path);
+ ObjectInserter i = repo.newObjectInserter();
+ ObjectReader r = repo.newObjectReader();
+ RevWalk rw = new RevWalk(reader)) {
+ inserter = i;
+ reader = r;
+
+ RevTree srcTree = revision != null ? rw.parseTree(revision) : null;
+ newTree = readTree(srcTree);
+
+ CommitBuilder commit = new CommitBuilder();
+ commit.setAuthor(ident);
+ commit.setCommitter(ident);
+ commit.setMessage(msg);
+
+ onSave(commit);
+ ObjectId res = newTree.writeTree(inserter);
+ if (res.equals(srcTree)) {
+ return;
+ }
+
+ commit.setTreeId(res);
+ if (revision != null) {
+ commit.addParentId(revision);
+ }
+ ObjectId newRevision = inserter.insert(commit);
+ updateRef(repo, ident, newRevision, "commit: " + msg);
+ revision = newRevision;
+ } finally {
+ inserter = null;
+ reader = null;
+ }
+ }
+
+ @Override
+ protected boolean onSave(CommitBuilder commit) throws IOException {
+ if (Strings.isNullOrEmpty(commit.getMessage())) {
+ commit.setMessage("Updated SSH keys\n");
+ }
+
+ saveUTF8(AuthorizedKeys.FILE_NAME, AuthorizedKeys.serialize(keys));
+ return true;
+ }
+
+ private void updateRef(Repository repo, PersonIdent ident,
+ ObjectId newRevision, String refLogMsg) throws IOException {
+ RefUpdate ru = repo.updateRef(getRefName());
+ ru.setRefLogIdent(ident);
+ ru.setNewObjectId(newRevision);
+ ru.setExpectedOldObjectId(revision);
+ ru.setRefLogMessage(refLogMsg, false);
+ RefUpdate.Result r = ru.update();
+ switch(r) {
+ case FAST_FORWARD:
+ case NEW:
+ case NO_CHANGE:
+ break;
+ case FORCED:
+ case IO_FAILURE:
+ case LOCK_FAILURE:
+ case NOT_ATTEMPTED:
+ case REJECTED:
+ case REJECTED_CURRENT_BRANCH:
+ case RENAMED:
+ default:
+ throw new IOException("Failed to update " + getRefName() + " of "
+ + project + ": " + r.name());
+ }
+ }
+}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountSshKey.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountSshKey.java
index 8c4ac82..7b1e874 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountSshKey.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountSshKey.java
@@ -17,6 +17,8 @@
import com.google.gwtorm.client.Column;
import com.google.gwtorm.client.IntKey;
+import java.util.Objects;
+
/** An SSH key approved for use by an {@link Account}. */
public final class AccountSshKey {
public static class Id extends IntKey<Account.Id> {
@@ -129,4 +131,20 @@
public void setInvalid() {
valid = false;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (o instanceof AccountSshKey) {
+ AccountSshKey other = (AccountSshKey) o;
+ return Objects.equals(id, other.id)
+ && Objects.equals(sshPublicKey, other.sshPublicKey)
+ && Objects.equals(valid, other.valid);
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(id, sshPublicKey, valid);
+ }
}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountSshKeyAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountSshKeyAccess.java
deleted file mode 100644
index 6f71ba4..0000000
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountSshKeyAccess.java
+++ /dev/null
@@ -1,36 +0,0 @@
-// Copyright (C) 2008 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.reviewdb.server;
-
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountSshKey;
-import com.google.gwtorm.server.Access;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.PrimaryKey;
-import com.google.gwtorm.server.Query;
-import com.google.gwtorm.server.ResultSet;
-
-public interface AccountSshKeyAccess extends
- Access<AccountSshKey, AccountSshKey.Id> {
- @Override
- @PrimaryKey("id")
- AccountSshKey get(AccountSshKey.Id id) throws OrmException;
-
- @Query("WHERE id.accountId = ?")
- ResultSet<AccountSshKey> byAccount(Account.Id id) throws OrmException;
-
- @Query("WHERE id.accountId = ? ORDER BY id.seq DESC LIMIT 1")
- ResultSet<AccountSshKey> byAccountLast(Account.Id id) throws OrmException;
-}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java
index 7b28f1f..9f62dc2 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java
@@ -53,8 +53,7 @@
@Relation(id = 7)
AccountExternalIdAccess accountExternalIds();
- @Relation(id = 8)
- AccountSshKeyAccess accountSshKeys();
+ // Deleted @Relation(id = 8)
@Relation(id = 10)
AccountGroupAccess accountGroups();
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
index 05793b5..aa37974 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
@@ -89,11 +89,6 @@
}
@Override
- public AccountSshKeyAccess accountSshKeys() {
- return delegate.accountSshKeys();
- }
-
- @Override
public AccountGroupAccess accountGroups() {
return delegate.accountGroups();
}
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql
index 9242fe0..2110295 100644
--- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql
+++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql
@@ -48,11 +48,6 @@
-- *********************************************************************
--- AccountSshKeyAccess
--- @PrimaryKey covers: byAccount, valid
-
-
--- *********************************************************************
-- ApprovalCategoryAccess
-- too small to bother indexing
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql
index 6934eb2..334b6c4 100644
--- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql
+++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql
@@ -55,11 +55,6 @@
#
-- *********************************************************************
--- AccountSshKeyAccess
--- @PrimaryKey covers: byAccount, valid
-
-
--- *********************************************************************
-- ApprovalCategoryAccess
-- too small to bother indexing
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql
index f146785..d723667 100644
--- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql
+++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql
@@ -96,11 +96,6 @@
-- *********************************************************************
--- AccountSshKeyAccess
--- @PrimaryKey covers: byAccount, valid
-
-
--- *********************************************************************
-- ApprovalCategoryAccess
-- too small to bother indexing
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AddSshKey.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AddSshKey.java
index e91998e..216672c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AddSshKey.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AddSshKey.java
@@ -16,7 +16,6 @@
import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.common.collect.Iterables;
import com.google.common.io.ByteSource;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.common.errors.InvalidSshKeyException;
@@ -27,24 +26,22 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.AccountSshKey;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AddSshKey.Input;
import com.google.gerrit.server.mail.AddKeySender;
import com.google.gerrit.server.ssh.SshKeyCache;
import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.io.InputStream;
-import java.util.Collections;
@Singleton
public class AddSshKey implements RestModifyView<AccountResource, Input> {
@@ -55,22 +52,25 @@
}
private final Provider<CurrentUser> self;
- private final Provider<ReviewDb> dbProvider;
+ private final VersionedAuthorizedKeys.Accessor authorizedKeys;
private final SshKeyCache sshKeyCache;
private final AddKeySender.Factory addKeyFactory;
@Inject
- AddSshKey(Provider<CurrentUser> self, Provider<ReviewDb> dbProvider,
- SshKeyCache sshKeyCache, AddKeySender.Factory addKeyFactory) {
+ AddSshKey(Provider<CurrentUser> self,
+ VersionedAuthorizedKeys.Accessor authorizedKeys,
+ SshKeyCache sshKeyCache,
+ AddKeySender.Factory addKeyFactory) {
this.self = self;
- this.dbProvider = dbProvider;
+ this.authorizedKeys = authorizedKeys;
this.sshKeyCache = sshKeyCache;
this.addKeyFactory = addKeyFactory;
}
@Override
public Response<SshKeyInfo> apply(AccountResource rsrc, Input input)
- throws AuthException, BadRequestException, OrmException, IOException {
+ throws AuthException, BadRequestException, OrmException, IOException,
+ ConfigInvalidException {
if (self.get() != rsrc.getUser()
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to add SSH keys");
@@ -79,7 +79,8 @@
}
public Response<SshKeyInfo> apply(IdentifiedUser user, Input input)
- throws BadRequestException, OrmException, IOException {
+ throws BadRequestException, IOException,
+ ConfigInvalidException {
if (input == null) {
input = new Input();
}
@@ -87,11 +88,6 @@
throw new BadRequestException("SSH public key missing");
}
- ResultSet<AccountSshKey> byAccountLast =
- dbProvider.get().accountSshKeys().byAccountLast(user.getAccountId());
- AccountSshKey last = Iterables.getOnlyElement(byAccountLast, null);
- int max = last == null ? 0 : last.getKey().get();
-
final RawInput rawKey = input.raw;
String sshPublicKey = new ByteSource() {
@Override
@@ -102,15 +98,15 @@
try {
AccountSshKey sshKey =
- sshKeyCache.create(new AccountSshKey.Id(
- user.getAccountId(), max + 1), sshPublicKey);
- dbProvider.get().accountSshKeys().insert(Collections.singleton(sshKey));
+ authorizedKeys.addKey(user.getAccountId(), sshPublicKey);
+
try {
addKeyFactory.create(user, sshKey).send();
} catch (EmailException e) {
log.error("Cannot send SSH key added message to "
+ user.getAccount().getPreferredEmail(), e);
}
+
sshKeyCache.evict(user.getUserName());
return Response.<SshKeyInfo>created(GetSshKeys.newSshKeyInfo(sshKey));
} catch (InvalidSshKeyException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthorizedKeys.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthorizedKeys.java
new file mode 100644
index 0000000..0e8c051
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthorizedKeys.java
@@ -0,0 +1,78 @@
+// Copyright (C) 2016 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.account;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Optional;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountSshKey;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+public class AuthorizedKeys {
+ public static final String FILE_NAME = "authorized_keys";
+
+ @VisibleForTesting
+ public static final String INVALID_KEY_COMMENT_PREFIX = "# INVALID ";
+
+ @VisibleForTesting
+ public static final String DELETED_KEY_COMMENT = "# DELETED";
+
+ public static List<Optional<AccountSshKey>> parse(
+ Account.Id accountId, String s) {
+ List<Optional<AccountSshKey>> keys = new ArrayList<>();
+ int seq = 1;
+ for (String line : s.split("\\r?\\n")) {
+ line = line.trim();
+ if (line.isEmpty()) {
+ continue;
+ } else if (line.startsWith(INVALID_KEY_COMMENT_PREFIX)) {
+ String pub = line.substring(INVALID_KEY_COMMENT_PREFIX.length());
+ AccountSshKey key =
+ new AccountSshKey(new AccountSshKey.Id(accountId, seq++), pub);
+ key.setInvalid();
+ keys.add(Optional.of(key));
+ } else if (line.startsWith(DELETED_KEY_COMMENT)) {
+ keys.add(Optional.<AccountSshKey> absent());
+ seq++;
+ } else if (line.startsWith("#")) {
+ continue;
+ } else {
+ AccountSshKey key =
+ new AccountSshKey(new AccountSshKey.Id(accountId, seq++), line);
+ keys.add(Optional.of(key));
+ }
+ }
+ return keys;
+ }
+
+ public static String serialize(Collection<Optional<AccountSshKey>> keys) {
+ StringBuilder b = new StringBuilder();
+ for (Optional<AccountSshKey> key : keys) {
+ if (key.isPresent()) {
+ if (!key.get().isValid()) {
+ b.append(INVALID_KEY_COMMENT_PREFIX);
+ }
+ b.append(key.get().getSshPublicKey().trim());
+ } else {
+ b.append(DELETED_KEY_COMMENT);
+ }
+ b.append("\n");
+ }
+ return b.toString();
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
index 190db5f..bcdc071 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
@@ -34,7 +34,6 @@
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
-import com.google.gerrit.reviewdb.client.AccountSshKey;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.CreateAccount.Input;
@@ -48,7 +47,9 @@
import com.google.inject.assistedinject.Assisted;
import org.apache.commons.validator.routines.EmailValidator;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import java.io.IOException;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
@@ -73,37 +74,45 @@
private final ReviewDb db;
private final Provider<IdentifiedUser> currentUser;
private final GroupsCollection groupsCollection;
+ private final VersionedAuthorizedKeys.Accessor authorizedKeys;
private final SshKeyCache sshKeyCache;
private final AccountCache accountCache;
private final AccountByEmailCache byEmailCache;
private final AccountLoader.Factory infoLoader;
private final DynamicSet<AccountExternalIdCreator> externalIdCreators;
- private final String username;
private final AuditService auditService;
+ private final String username;
@Inject
- CreateAccount(ReviewDb db, Provider<IdentifiedUser> currentUser,
- GroupsCollection groupsCollection, SshKeyCache sshKeyCache,
- AccountCache accountCache, AccountByEmailCache byEmailCache,
+ CreateAccount(ReviewDb db,
+ Provider<IdentifiedUser> currentUser,
+ GroupsCollection groupsCollection,
+ VersionedAuthorizedKeys.Accessor authorizedKeys,
+ SshKeyCache sshKeyCache,
+ AccountCache accountCache,
+ AccountByEmailCache byEmailCache,
AccountLoader.Factory infoLoader,
DynamicSet<AccountExternalIdCreator> externalIdCreators,
- @Assisted String username, AuditService auditService) {
+ AuditService auditService,
+ @Assisted String username) {
this.db = db;
this.currentUser = currentUser;
this.groupsCollection = groupsCollection;
+ this.authorizedKeys = authorizedKeys;
this.sshKeyCache = sshKeyCache;
this.accountCache = accountCache;
this.byEmailCache = byEmailCache;
this.infoLoader = infoLoader;
this.externalIdCreators = externalIdCreators;
- this.username = username;
this.auditService = auditService;
+ this.username = username;
}
@Override
public Response<AccountInfo> apply(TopLevelResource rsrc, Input input)
throws BadRequestException, ResourceConflictException,
- UnprocessableEntityException, OrmException {
+ UnprocessableEntityException, OrmException, IOException,
+ ConfigInvalidException {
if (input == null) {
input = new Input();
}
@@ -119,7 +128,6 @@
Set<AccountGroup.Id> groups = parseGroups(input.groups);
Account.Id id = new Account.Id(db.nextAccountId());
- AccountSshKey key = createSshKey(id, input.sshKey);
AccountExternalId extUser =
new AccountExternalId(id, new AccountExternalId.Key(
@@ -178,10 +186,6 @@
a.setPreferredEmail(input.email);
db.accounts().insert(Collections.singleton(a));
- if (key != null) {
- db.accountSshKeys().insert(Collections.singleton(key));
- }
-
for (AccountGroup.Id groupId : groups) {
AccountGroupMember m =
new AccountGroupMember(new AccountGroupMember.Key(id, groupId));
@@ -190,7 +194,15 @@
db.accountGroupMembers().insert(Collections.singleton(m));
}
- sshKeyCache.evict(username);
+ if (input.sshKey != null) {
+ try {
+ authorizedKeys.addKey(id, input.sshKey);
+ sshKeyCache.evict(username);
+ } catch (InvalidSshKeyException e) {
+ throw new BadRequestException(e.getMessage());
+ }
+ }
+
accountCache.evictByUsername(username);
byEmailCache.evict(input.email);
@@ -212,18 +224,6 @@
return groupIds;
}
- private AccountSshKey createSshKey(Account.Id id, String sshKey)
- throws BadRequestException {
- if (sshKey == null) {
- return null;
- }
- try {
- return sshKeyCache.create(new AccountSshKey.Id(id, 1), sshKey.trim());
- } catch (InvalidSshKeyException e) {
- throw new BadRequestException(e.getMessage());
- }
- }
-
private AccountExternalId.Key getEmailKey(String email) {
return new AccountExternalId.Key(AccountExternalId.SCHEME_MAILTO, email);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteSshKey.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteSshKey.java
index 9066858..9212002 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteSshKey.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteSshKey.java
@@ -17,7 +17,6 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.DeleteSshKey.Input;
import com.google.gerrit.server.ssh.SshKeyCache;
@@ -26,7 +25,10 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
-import java.util.Collections;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+
+import java.io.IOException;
@Singleton
public class DeleteSshKey implements
@@ -35,28 +37,31 @@
}
private final Provider<CurrentUser> self;
- private final Provider<ReviewDb> dbProvider;
+ private final VersionedAuthorizedKeys.Accessor authorizedKeys;
private final SshKeyCache sshKeyCache;
@Inject
- DeleteSshKey(Provider<ReviewDb> dbProvider,
- Provider<CurrentUser> self,
+ DeleteSshKey(Provider<CurrentUser> self,
+ VersionedAuthorizedKeys.Accessor authorizedKeys,
SshKeyCache sshKeyCache) {
this.self = self;
- this.dbProvider = dbProvider;
+ this.authorizedKeys = authorizedKeys;
this.sshKeyCache = sshKeyCache;
}
@Override
public Response<?> apply(AccountResource.SshKey rsrc, Input input)
- throws AuthException, OrmException {
+ throws AuthException, OrmException, RepositoryNotFoundException,
+ IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser()
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to delete SSH keys");
}
- dbProvider.get().accountSshKeys()
- .deleteKeys(Collections.singleton(rsrc.getSshKey().getKey()));
+
+ authorizedKeys.deleteKey(rsrc.getUser().getAccountId(),
+ rsrc.getSshKey().getKey().get());
sshKeyCache.evict(rsrc.getUser().getUserName());
+
return Response.none();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetSshKeys.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetSshKeys.java
index e1668f2..bf1a3af 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetSshKeys.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetSshKeys.java
@@ -14,13 +14,13 @@
package com.google.gerrit.server.account;
+import com.google.common.base.Function;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.common.SshKeyInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.AccountSshKey;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gwtorm.server.OrmException;
@@ -28,23 +28,29 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+
+import java.io.IOException;
import java.util.List;
@Singleton
public class GetSshKeys implements RestReadView<AccountResource> {
private final Provider<CurrentUser> self;
- private final Provider<ReviewDb> dbProvider;
+ private final VersionedAuthorizedKeys.Accessor authorizedKeys;
@Inject
- GetSshKeys(Provider<CurrentUser> self, Provider<ReviewDb> dbProvider) {
+ GetSshKeys(Provider<CurrentUser> self,
+ VersionedAuthorizedKeys.Accessor authorizedKeys) {
this.self = self;
- this.dbProvider = dbProvider;
+ this.authorizedKeys = authorizedKeys;
}
@Override
- public List<SshKeyInfo> apply(AccountResource rsrc) throws AuthException,
- OrmException {
+ public List<SshKeyInfo> apply(AccountResource rsrc)
+ throws AuthException, OrmException, RepositoryNotFoundException,
+ IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser()
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("not allowed to get SSH keys");
@@ -52,13 +58,15 @@
return apply(rsrc.getUser());
}
- public List<SshKeyInfo> apply(IdentifiedUser user) throws OrmException {
- List<SshKeyInfo> sshKeys = Lists.newArrayList();
- for (AccountSshKey sshKey : dbProvider.get().accountSshKeys()
- .byAccount(user.getAccountId()).toList()) {
- sshKeys.add(newSshKeyInfo(sshKey));
- }
- return sshKeys;
+ public List<SshKeyInfo> apply(IdentifiedUser user)
+ throws RepositoryNotFoundException, IOException, ConfigInvalidException {
+ return Lists.transform(authorizedKeys.getKeys(user.getAccountId()),
+ new Function<AccountSshKey, SshKeyInfo>() {
+ @Override
+ public SshKeyInfo apply(AccountSshKey key) {
+ return newSshKeyInfo(key);
+ }
+ });
}
public static SshKeyInfo newSshKeyInfo(AccountSshKey sshKey) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
index b70cabd..eb32e5a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
@@ -140,7 +140,7 @@
}
}
- public static void storeUrlAliases(VersionedAccountPreferences prefs,
+ private static void storeUrlAliases(VersionedAccountPreferences prefs,
Map<String, String> urlAliases) {
if (urlAliases != null) {
Config cfg = prefs.getConfig();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SshKeys.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SshKeys.java
index b35c03e..44a3192 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SshKeys.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SshKeys.java
@@ -20,7 +20,6 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.AccountSshKey;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gwtorm.server.OrmException;
@@ -28,22 +27,26 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+
+import java.io.IOException;
+
@Singleton
public class SshKeys implements
ChildCollection<AccountResource, AccountResource.SshKey> {
private final DynamicMap<RestView<AccountResource.SshKey>> views;
private final GetSshKeys list;
private final Provider<CurrentUser> self;
- private final Provider<ReviewDb> dbProvider;
+ private final VersionedAuthorizedKeys.Accessor authorizedKeys;
@Inject
SshKeys(DynamicMap<RestView<AccountResource.SshKey>> views,
GetSshKeys list, Provider<CurrentUser> self,
- Provider<ReviewDb> dbProvider) {
+ VersionedAuthorizedKeys.Accessor authorizedKeys) {
this.views = views;
this.list = list;
this.self = self;
- this.dbProvider = dbProvider;
+ this.authorizedKeys = authorizedKeys;
}
@Override
@@ -53,7 +56,8 @@
@Override
public AccountResource.SshKey parse(AccountResource rsrc, IdString id)
- throws ResourceNotFoundException, OrmException {
+ throws ResourceNotFoundException, OrmException, IOException,
+ ConfigInvalidException {
if (self.get() != rsrc.getUser()
&& !self.get().getCapabilities().canModifyAccount()) {
throw new ResourceNotFoundException();
@@ -62,12 +66,10 @@
}
public AccountResource.SshKey parse(IdentifiedUser user, IdString id)
- throws ResourceNotFoundException, OrmException {
+ throws ResourceNotFoundException, IOException, ConfigInvalidException {
try {
int seq = Integer.parseInt(id.get(), 10);
- AccountSshKey sshKey =
- dbProvider.get().accountSshKeys()
- .get(new AccountSshKey.Id(user.getAccountId(), seq));
+ AccountSshKey sshKey = authorizedKeys.getKey(user.getAccountId(), seq);
if (sshKey == null) {
throw new ResourceNotFoundException(id);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAuthorizedKeys.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAuthorizedKeys.java
new file mode 100644
index 0000000..4662f87
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAuthorizedKeys.java
@@ -0,0 +1,327 @@
+// Copyright (C) 2016 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.account;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.base.Function;
+import com.google.common.base.Optional;
+import com.google.common.base.Strings;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Ordering;
+import com.google.gerrit.common.errors.InvalidSshKeyException;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountSshKey;
+import com.google.gerrit.reviewdb.client.AccountSshKey.Id;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.IdentifiedUser;
+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.VersionedMetaData;
+import com.google.gerrit.server.ssh.SshKeyCreator;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.assistedinject.Assisted;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Repository;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * 'authorized_keys' file in the refs/users/CD/ABCD branches of the All-Users
+ * repository.
+ *
+ * The `authorized_keys' files stores the public SSH keys of the user. The file
+ * format matches the standard SSH file format, which means that each key is
+ * stored on a separate line (see
+ * https://en.wikibooks.org/wiki/OpenSSH/Client_Configuration_Files#.7E.2F.ssh.2Fauthorized_keys).
+ *
+ * The order of the keys in the file determines the sequence numbers of the
+ * keys. The first line corresponds to sequence number 1.
+ *
+ * Invalid keys are marked with the prefix <code># INVALID</code>.
+ *
+ * To keep the sequence numbers intact when a key is deleted, a
+ * <code># DELETED</code> line is inserted at the position where the key was
+ * deleted.
+ *
+ * Other comment lines are ignored on read, and are not written back when the
+ * file is modified.
+ */
+public class VersionedAuthorizedKeys extends VersionedMetaData
+ implements AutoCloseable {
+ @Singleton
+ public static class Accessor {
+ private final GitRepositoryManager repoManager;
+ private final AllUsersName allUsersName;
+ private final VersionedAuthorizedKeys.Factory authorizedKeysFactory;
+ private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
+ private final IdentifiedUser.GenericFactory userFactory;
+
+ @Inject
+ Accessor(
+ GitRepositoryManager repoManager,
+ AllUsersName allUsersName,
+ VersionedAuthorizedKeys.Factory authorizedKeysFactory,
+ Provider<MetaDataUpdate.User> metaDataUpdateFactory,
+ IdentifiedUser.GenericFactory userFactory) {
+ this.repoManager = repoManager;
+ this.allUsersName = allUsersName;
+ this.authorizedKeysFactory = authorizedKeysFactory;
+ this.metaDataUpdateFactory = metaDataUpdateFactory;
+ this.userFactory = userFactory;
+ }
+
+ public List<AccountSshKey> getKeys(Account.Id accountId)
+ throws IOException, ConfigInvalidException {
+ return read(accountId).getKeys();
+ }
+
+ public AccountSshKey getKey(Account.Id accountId, int seq)
+ throws IOException, ConfigInvalidException {
+ return read(accountId).getKey(seq);
+ }
+
+ public AccountSshKey addKey(Account.Id accountId, String pub)
+ throws IOException, ConfigInvalidException, InvalidSshKeyException {
+ try (VersionedAuthorizedKeys authorizedKeys = open(accountId)) {
+ AccountSshKey key = authorizedKeys.addKey(pub);
+ commit(authorizedKeys);
+ return key;
+ }
+ }
+
+ public void deleteKey(Account.Id accountId, int seq)
+ throws IOException, ConfigInvalidException {
+ try (VersionedAuthorizedKeys authorizedKeys = open(accountId)) {
+ if (authorizedKeys.deleteKey(seq)) {
+ commit(authorizedKeys);
+ }
+ }
+ }
+
+ public void markKeyInvalid(Account.Id accountId, int seq)
+ throws IOException, ConfigInvalidException {
+ try (VersionedAuthorizedKeys authorizedKeys = open(accountId)) {
+ if (authorizedKeys.markKeyInvalid(seq)) {
+ commit(authorizedKeys);
+ }
+ }
+ }
+
+ private VersionedAuthorizedKeys read(Account.Id accountId)
+ throws IOException, ConfigInvalidException {
+ try (Repository git = repoManager.openRepository(allUsersName)) {
+ VersionedAuthorizedKeys authorizedKeys =
+ authorizedKeysFactory.create(accountId);
+ authorizedKeys.load(git);
+ return authorizedKeys;
+ }
+ }
+
+ private VersionedAuthorizedKeys open(Account.Id accountId)
+ throws IOException, ConfigInvalidException {
+ Repository git = repoManager.openRepository(allUsersName);
+ VersionedAuthorizedKeys authorizedKeys =
+ authorizedKeysFactory.create(accountId);
+ authorizedKeys.load(git);
+ return authorizedKeys;
+ }
+
+ private void commit(VersionedAuthorizedKeys authorizedKeys)
+ throws IOException {
+ try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName,
+ userFactory.create(authorizedKeys.accountId))) {
+ authorizedKeys.commit(md);
+ }
+ }
+ }
+
+ public static class SimpleSshKeyCreator implements SshKeyCreator {
+ @Override
+ public AccountSshKey create(Id id, String encoded) {
+ return new AccountSshKey(id, encoded);
+ }
+ }
+
+ public static interface Factory {
+ VersionedAuthorizedKeys create(Account.Id accountId);
+ }
+
+ private final SshKeyCreator sshKeyCreator;
+ private final Account.Id accountId;
+ private final String ref;
+ private Repository git;
+ private List<Optional<AccountSshKey>> keys;
+
+ @Inject
+ public VersionedAuthorizedKeys(
+ SshKeyCreator sshKeyCreator,
+ @Assisted Account.Id accountId) {
+ this.sshKeyCreator = sshKeyCreator;
+ this.accountId = accountId;
+ this.ref = RefNames.refsUsers(accountId);
+ }
+
+ @Override
+ protected String getRefName() {
+ return ref;
+ }
+
+ @Override
+ public void load(Repository git) throws IOException, ConfigInvalidException {
+ checkState(this.git == null);
+ this.git = git;
+ super.load(git);
+ }
+
+ @Override
+ protected void onLoad() throws IOException {
+ keys = AuthorizedKeys.parse(accountId, readUTF8(AuthorizedKeys.FILE_NAME));
+ }
+
+ @Override
+ protected boolean onSave(CommitBuilder commit) throws IOException {
+ if (Strings.isNullOrEmpty(commit.getMessage())) {
+ commit.setMessage("Updated SSH keys\n");
+ }
+
+ saveUTF8(AuthorizedKeys.FILE_NAME, AuthorizedKeys.serialize(keys));
+ return true;
+ }
+
+ /** Returns all SSH keys. */
+ private List<AccountSshKey> getKeys() {
+ checkLoaded();
+ return Lists.newArrayList(Optional.presentInstances(keys));
+ }
+
+ /**
+ * Returns the SSH key with the given sequence number.
+ *
+ * @param seq sequence number
+ * @return the SSH key, <code>null</code> if there is no SSH key with this
+ * sequence number, or if the SSH key with this sequence number has
+ * been deleted
+ */
+ private AccountSshKey getKey(int seq) {
+ checkLoaded();
+ Optional<AccountSshKey> key = keys.get(seq - 1);
+ return key.orNull();
+ }
+
+ /**
+ * Adds a new public SSH key.
+ *
+ * If the specified public key exists already, the existing key is returned.
+ *
+ * @param pub the public SSH key to be added
+ * @return the new SSH key
+ * @throws InvalidSshKeyException
+ */
+ private AccountSshKey addKey(String pub) throws InvalidSshKeyException {
+ checkLoaded();
+
+ for (Optional<AccountSshKey> key : keys) {
+ if (key.isPresent()
+ && key.get().getSshPublicKey().trim().equals(pub.trim())) {
+ return key.get();
+ }
+ }
+
+ int seq = keys.size() + 1;
+ AccountSshKey.Id keyId = new AccountSshKey.Id(accountId, seq);
+ AccountSshKey key = sshKeyCreator.create(keyId, pub);
+ keys.add(Optional.of(key));
+ return key;
+ }
+
+ /**
+ * Deletes the SSH key with the given sequence number.
+ *
+ * @param seq the sequence number
+ * @return <code>true</code> if a key with this sequence number was found and
+ * deleted, <code>false</code> if no key with the given sequence
+ * number exists
+ */
+ private boolean deleteKey(int seq) {
+ checkLoaded();
+ if (seq <= keys.size() && keys.get(seq - 1).isPresent()) {
+ keys.set(seq - 1, Optional.<AccountSshKey> absent());
+ return true;
+ }
+ return false;
+ }
+
+ /**
+ * Marks the SSH key with the given sequence number as invalid.
+ *
+ * @param seq the sequence number
+ * @return <code>true</code> if a key with this sequence number was found and
+ * marked as invalid, <code>false</code> if no key with the given
+ * sequence number exists or if the key was already marked as invalid
+ */
+ private boolean markKeyInvalid(int seq) {
+ checkLoaded();
+ AccountSshKey key = getKey(seq);
+ if (key != null && key.isValid()) {
+ key.setInvalid();
+ return true;
+ }
+ return false;
+ }
+
+ /**
+ * Sets new SSH keys.
+ *
+ * The existing SSH keys are overwritten.
+ *
+ * @param newKeys the new public SSH keys
+ */
+ public void setKeys(Collection<AccountSshKey> newKeys) {
+ Ordering<AccountSshKey> o =
+ Ordering.natural().onResultOf(new Function<AccountSshKey, Integer>() {
+ @Override
+ public Integer apply(AccountSshKey sshKey) {
+ return sshKey.getKey().get();
+ }
+ });
+ keys = new ArrayList<>(Collections.nCopies(o.max(newKeys).getKey().get(),
+ Optional.<AccountSshKey> absent()));
+ for (AccountSshKey key : newKeys) {
+ keys.set(key.getKey().get() - 1, Optional.of(key));
+ }
+ }
+
+ @Override
+ public void close() {
+ if (git != null) {
+ git.close();
+ }
+ }
+
+ private void checkLoaded() {
+ checkNotNull(keys, "SSH keys not loaded yet");
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
index 6c8e599..709020f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
@@ -33,6 +33,7 @@
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.AddSshKey;
import com.google.gerrit.server.account.CreateEmail;
+import com.google.gerrit.server.account.DeleteSshKey;
import com.google.gerrit.server.account.GetAvatar;
import com.google.gerrit.server.account.GetDiffPreferences;
import com.google.gerrit.server.account.GetEditPreferences;
@@ -41,6 +42,7 @@
import com.google.gerrit.server.account.SetDiffPreferences;
import com.google.gerrit.server.account.SetEditPreferences;
import com.google.gerrit.server.account.SetPreferences;
+import com.google.gerrit.server.account.SshKeys;
import com.google.gerrit.server.account.StarredChanges;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ChangesCollection;
@@ -75,6 +77,8 @@
private final GpgApiAdapter gpgApiAdapter;
private final GetSshKeys getSshKeys;
private final AddSshKey addSshKey;
+ private final DeleteSshKey deleteSshKey;
+ private final SshKeys sshKeys;
@Inject
AccountApiImpl(AccountLoader.Factory ailf,
@@ -92,6 +96,8 @@
GpgApiAdapter gpgApiAdapter,
GetSshKeys getSshKeys,
AddSshKey addSshKey,
+ DeleteSshKey deleteSshKey,
+ SshKeys sshKeys,
@Assisted AccountResource account) {
this.account = account;
this.accountLoaderFactory = ailf;
@@ -108,6 +114,8 @@
this.createEmailFactory = createEmailFactory;
this.getSshKeys = getSshKeys;
this.addSshKey = addSshKey;
+ this.deleteSshKey = deleteSshKey;
+ this.sshKeys = sshKeys;
this.gpgApiAdapter = gpgApiAdapter;
}
@@ -225,7 +233,7 @@
public List<SshKeyInfo> listSshKeys() throws RestApiException {
try {
return getSshKeys.apply(account);
- } catch (OrmException e) {
+ } catch (OrmException | IOException | ConfigInvalidException e) {
throw new RestApiException("Cannot list SSH keys", e);
}
}
@@ -236,12 +244,23 @@
in.raw = RawInputUtil.create(key);
try {
return addSshKey.apply(account, in).value();
- } catch (OrmException | IOException e) {
+ } catch (OrmException | IOException | ConfigInvalidException e) {
throw new RestApiException("Cannot add SSH key", e);
}
}
@Override
+ public void deleteSshKey(int seq) throws RestApiException {
+ try {
+ AccountResource.SshKey sshKeyRes =
+ sshKeys.parse(account, IdString.fromDecoded(Integer.toString(seq)));
+ deleteSshKey.apply(sshKeyRes, null);
+ } catch (OrmException | IOException | ConfigInvalidException e) {
+ throw new RestApiException("Cannot delete SSH key", e);
+ }
+ }
+
+ @Override
public Map<String, GpgKeyInfo> listGpgKeys() throws RestApiException {
try {
return gpgApiAdapter.listGpgKeys(account);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
index d8fbd3b..288dd83 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
@@ -132,7 +132,6 @@
patchSet = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
change.setStatus(Change.Status.ABANDONED);
change.setLastUpdatedOn(ctx.getWhen());
- ctx.saveChange();
update.setStatus(change.getStatus());
message = newMessage(ctx);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
index 6af3e79..44e55c8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -332,7 +332,6 @@
}
patchSet = psUtil.insert(ctx.getDb(), ctx.getRevWalk(), update, psId,
commit, draft, newGroups, null);
- ctx.saveChange();
/* TODO: fixStatus is used here because the tests
* (byStatusClosed() in AbstractQueryChangesTest)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
index 513a34f..cefbf8a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
@@ -286,7 +286,6 @@
changeMessage.setMessage(sb.toString());
cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId), changeMessage);
- ctx.saveChange(); // Bump lastUpdatedOn to match message.
return true;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java
index b991303..bfbc828 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java
@@ -127,6 +127,7 @@
comment, patchListCache, ctx.getChange(), ps);
plcUtil.putComments(
ctx.getDb(), ctx.getUpdate(psId), Collections.singleton(comment));
+ ctx.bumpLastUpdatedOn(false);
return true;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
index 2edff4b..c9f5aa3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
@@ -160,7 +160,6 @@
if (c.currentPatchSetId().equals(psId)) {
c.setCurrentPatchSet(previousPatchSetInfo(ctx));
}
- ctx.saveChange();
}
private boolean deletedOnlyPatchSet() {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
index 0ded3a2..8f597ac 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
@@ -168,7 +168,6 @@
if (psa == null) {
throw new ResourceNotFoundException();
}
- ctx.saveChange();
ctx.getDb().patchSetApprovals().update(Collections.singleton(psa));
if (msg.length() > 0) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java
index db5cc47..2139ec4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java
@@ -186,7 +186,6 @@
cmsg.setMessage(msgBuf.toString());
cmUtil.addChangeMessage(ctx.getDb(), update, cmsg);
- ctx.saveChange();
return true;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
index a70716a..3defdd7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -246,7 +246,6 @@
change.setStatus(Change.Status.NEW);
}
change.setCurrentPatchSet(patchSetInfo);
- ctx.saveChange();
if (copyApprovals) {
approvalCopier.copy(db, ctl, patchSet);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 961b0bf..dc84e49 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -370,9 +370,6 @@
dirty |= insertComments(ctx);
dirty |= updateLabels(ctx);
dirty |= insertMessage(ctx);
- if (dirty) {
- ctx.saveChange();
- }
return dirty;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
index d6e80f4..e3a8572 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
@@ -275,10 +275,12 @@
rsrc.getChange(),
reviewers.keySet());
- if (!added.isEmpty()) {
- patchSet = psUtil.current(dbProvider.get(), rsrc.getNotes());
+ if (added.isEmpty()) {
+ return false;
}
- return !added.isEmpty();
+ patchSet = psUtil.current(dbProvider.get(), rsrc.getNotes());
+ ctx.bumpLastUpdatedOn(false);
+ return true;
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
index 74cbd5f..ff5185e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
@@ -192,7 +192,6 @@
if (wasDraftChange) {
change.setStatus(Change.Status.NEW);
update.setStatus(change.getStatus());
- ctx.saveChange();
}
}
@@ -202,10 +201,6 @@
throw new ResourceConflictException("Patch set is not a draft");
}
psUtil.publish(ctx.getDb(), ctx.getUpdate(psId), patchSet);
- // Force ETag invalidation if not done already
- if (!wasDraftChange) {
- ctx.saveChange();
- }
}
private void addReviewers(ChangeContext ctx)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java
index 082798a..1d756f4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java
@@ -155,6 +155,7 @@
plcUtil.putComments(ctx.getDb(), update,
Collections.singleton(update(comment, in, ctx.getWhen())));
}
+ ctx.bumpLastUpdatedOn(false);
return true;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
index 61718ae..f0db81d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
@@ -114,7 +114,6 @@
}
change.setTopic(Strings.emptyToNull(newTopicName));
update.setTopic(change.getTopic());
- ctx.saveChange();
ChangeMessage cmsg = new ChangeMessage(
new ChangeMessage.Key(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
index 8696ed6..b03194e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
@@ -117,7 +117,6 @@
patchSet = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
change.setStatus(Status.NEW);
change.setLastUpdatedOn(ctx.getWhen());
- ctx.saveChange();
update.setStatus(change.getStatus());
message = newMessage(ctx);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
index f1ff452..b5b3b6a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -70,6 +70,7 @@
import com.google.gerrit.server.account.GroupDetailFactory;
import com.google.gerrit.server.account.GroupIncludeCacheImpl;
import com.google.gerrit.server.account.GroupMembers;
+import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.api.accounts.AccountExternalIdCreator;
import com.google.gerrit.server.auth.AuthBackend;
import com.google.gerrit.server.auth.UniversalAuthBackend;
@@ -332,6 +333,7 @@
factory(SubmoduleSectionParser.Factory.class);
factory(ReplaceOp.Factory.class);
factory(GitModules.Factory.class);
+ factory(VersionedAuthorizedKeys.Factory.class);
bind(AccountManager.class);
factory(ChangeUserName.Factory.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
index 4c5ab65..bbf2299 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
@@ -60,6 +60,8 @@
|| i.legacycidInChangeTable != null
|| i.muteCommonPathPrefixes != null
|| i.reviewCategoryStrategy != null
+ || i.signedOffBy != null
+ || i.urlAliases != null
|| i.emailStrategy != null) {
throw new BadRequestException("unsupported option");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
index db1e97e..86b269b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
@@ -179,7 +179,7 @@
private final ReviewDbWrapper dbWrapper;
private boolean deleted;
- private boolean saved;
+ private boolean bumpLastUpdatedOn = true;
private ChangeContext(ChangeControl ctl, ReviewDbWrapper dbWrapper) {
this.ctl = ctl;
@@ -223,13 +223,11 @@
return c;
}
- public void saveChange() {
- checkState(!deleted, "cannot both save and delete change");
- saved = true;
+ public void bumpLastUpdatedOn(boolean bump) {
+ bumpLastUpdatedOn = bump;
}
public void deleteChange() {
- checkState(!saved, "cannot both save and delete change");
deleted = true;
}
}
@@ -581,14 +579,14 @@
}
// Bump lastUpdatedOn or rowVersion and commit.
+ Iterable<Change> cs = changesToUpdate(ctx);
if (newChanges.containsKey(id)) {
- db.changes().insert(bumpLastUpdatedOn(ctx));
- } else if (ctx.saved) {
- db.changes().update(bumpLastUpdatedOn(ctx));
+ // Insert rather than upsert in case of a race on change IDs.
+ db.changes().insert(cs);
} else if (ctx.deleted) {
- db.changes().delete(bumpLastUpdatedOn(ctx));
+ db.changes().delete(cs);
} else {
- db.changes().update(bumpRowVersionNotLastUpdatedOn(ctx));
+ db.changes().update(cs);
}
db.commit();
} finally {
@@ -618,19 +616,14 @@
}
}
- private static Iterable<Change> bumpLastUpdatedOn(ChangeContext ctx) {
+ private static Iterable<Change> changesToUpdate(ChangeContext ctx) {
Change c = ctx.getChange();
- if (c.getLastUpdatedOn().before(ctx.getWhen())) {
+ if (ctx.bumpLastUpdatedOn && c.getLastUpdatedOn().before(ctx.getWhen())) {
c.setLastUpdatedOn(ctx.getWhen());
}
return Collections.singleton(c);
}
- private static Iterable<Change> bumpRowVersionNotLastUpdatedOn(
- ChangeContext ctx) {
- return Collections.singleton(ctx.getChange());
- }
-
private ChangeContext newChangeContext(Change.Id id) throws Exception {
Change c = newChanges.get(id);
if (c == null) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java
index 9042955..eb359e6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java
@@ -19,6 +19,8 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
+import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.util.SubmoduleSectionParser;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -27,9 +29,7 @@
import org.eclipse.jgit.lib.BlobBasedConfig;
import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -49,28 +49,29 @@
private static final Logger log = LoggerFactory.getLogger(GitModules.class);
public interface Factory {
- GitModules create(Branch.NameKey project, String submissionId);
+ GitModules create(Branch.NameKey project, String submissionId,
+ MergeOpRepoManager m);
}
private static final String GIT_MODULES = ".gitmodules";
private final String thisServer;
- private final GitRepositoryManager repoManager;
private final SubmoduleSectionParser.Factory subSecParserFactory;
private final Branch.NameKey branch;
private final String submissionId;
+ private final MergeOpRepoManager orm;
Set<SubmoduleSubscription> subscriptions;
@AssistedInject
GitModules(
@CanonicalWebUrl @Nullable String canonicalWebUrl,
- GitRepositoryManager repoManager,
SubmoduleSectionParser.Factory subSecParserFactory,
@Assisted Branch.NameKey branch,
- @Assisted String submissionId) throws SubmoduleException {
- this.repoManager = repoManager;
+ @Assisted String submissionId,
+ @Assisted MergeOpRepoManager orm) throws SubmoduleException {
this.subSecParserFactory = subSecParserFactory;
+ this.orm = orm;
this.branch = branch;
this.submissionId = submissionId;
try {
@@ -84,24 +85,27 @@
void load() throws IOException {
Project.NameKey project = branch.getParentKey();
logDebug("Loading .gitmodules of {} for project {}", branch, project);
- try (Repository repo = repoManager.openRepository(project);
- RevWalk rw = new RevWalk(repo)) {
+ try {
+ orm.openRepo(project, false);
+ } catch (NoSuchProjectException e) {
+ throw new IOException(e);
+ }
+ OpenRepo or = orm.getRepo(project);
- ObjectId id = repo.resolve(branch.get());
- if (id == null) {
- throw new IOException("Cannot open branch " + branch.get());
- }
- RevCommit commit = rw.parseCommit(id);
+ ObjectId id = or.repo.resolve(branch.get());
+ if (id == null) {
+ throw new IOException("Cannot open branch " + branch.get());
+ }
+ RevCommit commit = or.rw.parseCommit(id);
- TreeWalk tw = TreeWalk.forPath(repo, GIT_MODULES, commit.getTree());
- if (tw == null
- || (tw.getRawMode(0) & FileMode.TYPE_MASK) != FileMode.TYPE_FILE) {
- return;
- }
-
+ TreeWalk tw = TreeWalk.forPath(or.repo, GIT_MODULES, commit.getTree());
+ if (tw == null
+ || (tw.getRawMode(0) & FileMode.TYPE_MASK) != FileMode.TYPE_FILE) {
+ return;
+ }
+ try {
BlobBasedConfig bbc =
- new BlobBasedConfig(null, repo, commit, GIT_MODULES);
-
+ new BlobBasedConfig(null, or.repo, commit, GIT_MODULES);
subscriptions = subSecParserFactory.create(bbc, thisServer,
branch).parseAllSections();
} catch (ConfigInvalidException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index 0047260..26db045 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -28,7 +28,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Iterables;
-import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Ordering;
@@ -54,7 +53,8 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
-import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
+import com.google.gerrit.server.git.MergeOpRepoManager.OpenBranch;
+import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
import com.google.gerrit.server.git.strategy.SubmitStrategy;
import com.google.gerrit.server.git.strategy.SubmitStrategyFactory;
import com.google.gerrit.server.git.strategy.SubmitStrategyListener;
@@ -62,8 +62,6 @@
import com.google.gerrit.server.git.validators.MergeValidators;
import com.google.gerrit.server.project.ChangeControl;
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.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -72,17 +70,10 @@
import com.google.inject.Provider;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefUpdate;
-import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevFlag;
-import org.eclipse.jgit.revwalk.RevSort;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -97,7 +88,6 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
import java.util.Set;
/**
@@ -117,88 +107,6 @@
public class MergeOp implements AutoCloseable {
private static final Logger log = LoggerFactory.getLogger(MergeOp.class);
- private class OpenRepo {
- final Repository repo;
- final CodeReviewRevWalk rw;
- final RevFlag canMergeFlag;
- final ObjectInserter ins;
-
- ProjectState project;
- BatchUpdate update;
-
- private final ObjectReader reader;
- private final Map<Branch.NameKey, OpenBranch> branches;
-
- OpenRepo(Repository repo, ProjectState project) {
- this.repo = repo;
- this.project = project;
- ins = repo.newObjectInserter();
- reader = ins.newReader();
- rw = CodeReviewCommit.newRevWalk(reader);
- rw.sort(RevSort.TOPO);
- rw.sort(RevSort.COMMIT_TIME_DESC, true);
- rw.setRetainBody(false);
- canMergeFlag = rw.newFlag("CAN_MERGE");
- rw.retainOnReset(canMergeFlag);
-
- branches = Maps.newHashMapWithExpectedSize(1);
- }
-
- OpenBranch getBranch(Branch.NameKey branch) throws IntegrationException {
- OpenBranch ob = branches.get(branch);
- if (ob == null) {
- ob = new OpenBranch(this, branch);
- branches.put(branch, ob);
- }
- return ob;
- }
-
- Project.NameKey getProjectName() {
- return project.getProject().getNameKey();
- }
-
- BatchUpdate getUpdate() {
- if (update == null) {
- update = batchUpdateFactory.create(db, getProjectName(), caller, ts);
- update.setRepository(repo, rw, ins);
- }
- return update;
- }
-
- void close() {
- if (update != null) {
- update.close();
- }
- rw.close();
- reader.close();
- ins.close();
- repo.close();
- }
- }
-
- private static class OpenBranch {
- final RefUpdate update;
- final CodeReviewCommit oldTip;
- MergeTip mergeTip;
-
- OpenBranch(OpenRepo or, Branch.NameKey name) throws IntegrationException {
- try {
- update = or.repo.updateRef(name.get());
- if (update.getOldObjectId() != null) {
- oldTip = or.rw.parseCommit(update.getOldObjectId());
- } else if (Objects.equals(or.repo.getFullBranch(), name.get())) {
- oldTip = null;
- update.setExpectedOldObjectId(ObjectId.zeroId());
- } else {
- throw new IntegrationException("The destination branch "
- + name + " does not exist anymore.");
- }
- } catch (IOException e) {
- throw new IntegrationException("Cannot open branch " + name, e);
- }
- }
- }
-
public static class CommitStatus {
private final ImmutableMap<Change.Id, ChangeData> changes;
private final ImmutableSetMultimap<Branch.NameKey, Change.Id> byBranch;
@@ -307,16 +215,13 @@
private final ChangeMessagesUtil cmUtil;
private final BatchUpdate.Factory batchUpdateFactory;
- private final GitRepositoryManager repoManager;
private final InternalUser.Factory internalUserFactory;
private final MergeSuperSet mergeSuperSet;
private final MergeValidators.Factory mergeValidatorsFactory;
- private final ProjectCache projectCache;
private final InternalChangeQuery internalChangeQuery;
private final SubmitStrategyFactory submitStrategyFactory;
private final Provider<SubmoduleOp> subOpProvider;
-
- private final Map<Project.NameKey, OpenRepo> openRepos;
+ private final MergeOpRepoManager orm;
private static final String MACHINE_ID;
static {
@@ -339,56 +244,27 @@
@Inject
MergeOp(ChangeMessagesUtil cmUtil,
BatchUpdate.Factory batchUpdateFactory,
- GitRepositoryManager repoManager,
InternalUser.Factory internalUserFactory,
MergeSuperSet mergeSuperSet,
MergeValidators.Factory mergeValidatorsFactory,
- ProjectCache projectCache,
InternalChangeQuery internalChangeQuery,
SubmitStrategyFactory submitStrategyFactory,
- Provider<SubmoduleOp> subOpProvider) {
+ Provider<SubmoduleOp> subOpProvider,
+ MergeOpRepoManager orm) {
this.cmUtil = cmUtil;
this.batchUpdateFactory = batchUpdateFactory;
- this.repoManager = repoManager;
this.internalUserFactory = internalUserFactory;
this.mergeSuperSet = mergeSuperSet;
this.mergeValidatorsFactory = mergeValidatorsFactory;
- this.projectCache = projectCache;
this.internalChangeQuery = internalChangeQuery;
this.submitStrategyFactory = submitStrategyFactory;
this.subOpProvider = subOpProvider;
-
- openRepos = new HashMap<>();
- }
-
- private OpenRepo getRepo(Project.NameKey project) {
- OpenRepo or = openRepos.get(project);
- checkState(or != null, "repo not yet opened: %s", project);
- return or;
- }
-
- private void openRepo(Project.NameKey project)
- throws NoSuchProjectException, IOException {
- checkState(!openRepos.containsKey(project),
- "repo already opened: %s", project);
- ProjectState projectState = projectCache.get(project);
- if (projectState == null) {
- throw new NoSuchProjectException(project);
- }
- try {
- OpenRepo or =
- new OpenRepo(repoManager.openRepository(project), projectState);
- openRepos.put(project, or);
- } catch (RepositoryNotFoundException e) {
- throw new NoSuchProjectException(project);
- }
+ this.orm = orm;
}
@Override
public void close() {
- for (OpenRepo repo : openRepos.values()) {
- repo.close();
- }
+ orm.close();
}
private static Optional<SubmitRecord> findOkRecord(
@@ -544,6 +420,8 @@
this.caller = caller;
updateSubmissionId(change);
this.db = db;
+ orm.setContext(db, ts, caller);
+
logDebug("Beginning integration of {}", change);
try {
ChangeSet cs = mergeSuperSet.completeChangeSet(db, change, caller);
@@ -605,14 +483,14 @@
openRepos(projects);
for (Branch.NameKey branch : branches) {
- OpenRepo or = getRepo(branch.getParentKey());
+ OpenRepo or = orm.getRepo(branch.getParentKey());
toSubmit.put(branch, validateChangeList(or, cbb.get(branch)));
}
failFast(cs); // Done checks that don't involve running submit strategies.
List<SubmitStrategy> strategies = new ArrayList<>(branches.size());
for (Branch.NameKey branch : branches) {
- OpenRepo or = getRepo(branch.getParentKey());
+ OpenRepo or = orm.getRepo(branch.getParentKey());
OpenBranch ob = or.getBranch(branch);
BranchBatch submitting = toSubmit.get(branch);
checkNotNull(submitting.submitType(),
@@ -647,14 +525,13 @@
throw new IntegrationException(msg, e);
}
- SubmoduleOp subOp = subOpProvider.get();
- updateSuperProjects(subOp, br.values());
+ updateSuperProjects(br.values());
}
private List<BatchUpdate> batchUpdates(Collection<Project.NameKey> projects) {
List<BatchUpdate> updates = new ArrayList<>(projects.size());
for (Project.NameKey project : projects) {
- updates.add(getRepo(project).getUpdate());
+ updates.add(orm.getRepo(project).getUpdate());
}
return updates;
}
@@ -848,11 +725,11 @@
}
}
- private void updateSuperProjects(SubmoduleOp subOp,
- Collection<Branch.NameKey> branches) {
+ private void updateSuperProjects(Collection<Branch.NameKey> branches) {
logDebug("Updating superprojects");
+ SubmoduleOp subOp = subOpProvider.get();
try {
- subOp.updateSuperProjects(db, branches, submissionId);
+ subOp.updateSuperProjects(db, branches, submissionId, orm);
logDebug("Updating superprojects done");
} catch (SubmoduleException e) {
logError("The gitlinks were not updated according to the "
@@ -864,7 +741,7 @@
throws IntegrationException {
for (Project.NameKey project : projects) {
try {
- openRepo(project);
+ orm.openRepo(project, true);
} catch (NoSuchProjectException noProject) {
logWarn("Project " + noProject.project() + " no longer exists, "
+ "abandoning open changes");
@@ -899,7 +776,6 @@
cmUtil.addChangeMessage(ctx.getDb(),
ctx.getUpdate(change.currentPatchSetId()), msg);
- ctx.saveChange();
return true;
}
});
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOpRepoManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOpRepoManager.java
new file mode 100644
index 0000000..0482034
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOpRepoManager.java
@@ -0,0 +1,199 @@
+// Copyright (C) 2016 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.git;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.Maps;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
+import com.google.inject.Inject;
+
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevFlag;
+import org.eclipse.jgit.revwalk.RevSort;
+
+import java.io.IOException;
+import java.sql.Timestamp;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * This is a helper class for MergeOp and not intended for general use.
+ *
+ * Some database backends require to open a repository just once within
+ * a transaction of a submission, this caches open repositories to satisfy
+ * that requirement.
+ */
+class MergeOpRepoManager implements AutoCloseable {
+ public class OpenRepo {
+ final Repository repo;
+ final CodeReviewRevWalk rw;
+ final RevFlag canMergeFlag;
+ final ObjectInserter ins;
+
+ final ProjectState project;
+ BatchUpdate update;
+
+ private final ObjectReader reader;
+ private final Map<Branch.NameKey, OpenBranch> branches;
+
+ private OpenRepo(Repository repo, ProjectState project) {
+ this.repo = repo;
+ this.project = project;
+ ins = repo.newObjectInserter();
+ reader = ins.newReader();
+ rw = CodeReviewCommit.newRevWalk(reader);
+ rw.sort(RevSort.TOPO);
+ rw.sort(RevSort.COMMIT_TIME_DESC, true);
+ rw.setRetainBody(false);
+ canMergeFlag = rw.newFlag("CAN_MERGE");
+ rw.retainOnReset(canMergeFlag);
+
+ branches = Maps.newHashMapWithExpectedSize(1);
+ }
+
+ OpenBranch getBranch(Branch.NameKey branch) throws IntegrationException {
+ OpenBranch ob = branches.get(branch);
+ if (ob == null) {
+ ob = new OpenBranch(this, branch);
+ branches.put(branch, ob);
+ }
+ return ob;
+ }
+
+ Project.NameKey getProjectName() {
+ return project.getProject().getNameKey();
+ }
+
+ BatchUpdate getUpdate() {
+ checkState(db != null, "call setContext before getUpdate");
+ if (update == null) {
+ update = batchUpdateFactory.create(db, getProjectName(), caller, ts);
+ update.setRepository(repo, rw, ins);
+ }
+ return update;
+ }
+
+ void close() {
+ if (update != null) {
+ update.close();
+ }
+ rw.close();
+ reader.close();
+ ins.close();
+ repo.close();
+ }
+ }
+
+ public static class OpenBranch {
+ final RefUpdate update;
+ final CodeReviewCommit oldTip;
+ MergeTip mergeTip;
+
+ OpenBranch(OpenRepo or, Branch.NameKey name) throws IntegrationException {
+ try {
+ update = or.repo.updateRef(name.get());
+ if (update.getOldObjectId() != null) {
+ oldTip = or.rw.parseCommit(update.getOldObjectId());
+ } else if (Objects.equals(or.repo.getFullBranch(), name.get())) {
+ oldTip = null;
+ update.setExpectedOldObjectId(ObjectId.zeroId());
+ } else {
+ throw new IntegrationException("The destination branch "
+ + name + " does not exist anymore.");
+ }
+ } catch (IOException e) {
+ throw new IntegrationException("Cannot open branch " + name, e);
+ }
+ }
+ }
+
+
+ private final Map<Project.NameKey, OpenRepo> openRepos;
+ private final BatchUpdate.Factory batchUpdateFactory;
+ private final GitRepositoryManager repoManager;
+ private final ProjectCache projectCache;
+
+ private ReviewDb db;
+ private Timestamp ts;
+ private IdentifiedUser caller;
+
+ @Inject
+ MergeOpRepoManager(
+ GitRepositoryManager repoManager,
+ ProjectCache projectCache,
+ BatchUpdate.Factory batchUpdateFactory) {
+ this.repoManager = repoManager;
+ this.projectCache = projectCache;
+ this.batchUpdateFactory = batchUpdateFactory;
+
+ openRepos = new HashMap<>();
+ }
+
+ void setContext(ReviewDb db, Timestamp ts, IdentifiedUser caller) {
+ this.db = db;
+ this.ts = ts;
+ this.caller = caller;
+ }
+
+ public OpenRepo getRepo(Project.NameKey project) {
+ OpenRepo or = openRepos.get(project);
+ checkState(or != null, "repo not yet opened: %s", project);
+ return or;
+ }
+
+ public void openRepo(Project.NameKey project, boolean abortIfOpen)
+ throws NoSuchProjectException, IOException {
+ if (abortIfOpen) {
+ checkState(!openRepos.containsKey(project),
+ "repo already opened: %s", project);
+ } else {
+ if (openRepos.containsKey(project)) {
+ return;
+ }
+ }
+ ProjectState projectState = projectCache.get(project);
+ if (projectState == null) {
+ throw new NoSuchProjectException(project);
+ }
+ try {
+ OpenRepo or =
+ new OpenRepo(repoManager.openRepository(project), projectState);
+ openRepos.put(project, or);
+ } catch (RepositoryNotFoundException e) {
+ throw new NoSuchProjectException(project);
+ }
+ }
+
+ @Override
+ public void close() {
+ for (OpenRepo repo : openRepos.values()) {
+ repo.close();
+ }
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index dc3ff6e..83585c1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -341,6 +341,7 @@
private final Provider<SubmoduleOp> subOpProvider;
private final Provider<Submit> submitProvider;
private final Provider<MergeOp> mergeOpProvider;
+ private final Provider<MergeOpRepoManager> ormProvider;
private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
private final NotesMigration notesMigration;
private final ChangeEditUtil editUtil;
@@ -391,6 +392,7 @@
Provider<SubmoduleOp> subOpProvider,
Provider<Submit> submitProvider,
Provider<MergeOp> mergeOpProvider,
+ Provider<MergeOpRepoManager> ormProvider,
DynamicMap<ProjectConfigEntry> pluginConfigEntries,
NotesMigration notesMigration,
ChangeEditUtil editUtil,
@@ -440,6 +442,7 @@
this.subOpProvider = subOpProvider;
this.submitProvider = submitProvider;
this.mergeOpProvider = mergeOpProvider;
+ this.ormProvider = ormProvider;
this.pluginConfigEntries = pluginConfigEntries;
this.notesMigration = notesMigration;
@@ -474,9 +477,11 @@
});
if (!projectControl.allRefsAreVisible()) {
- rp.setCheckReferencedObjectsAreReachable(receiveConfig.checkReferencedObjectsAreReachable);
- rp.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache, repo, projectControl, db, false));
+ rp.setCheckReferencedObjectsAreReachable(
+ receiveConfig.checkReferencedObjectsAreReachable);
}
+ rp.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache, repo,
+ projectControl, db, false));
List<AdvertiseRefsHook> advHooks = new ArrayList<>(3);
advHooks.add(new AdvertiseRefsHook() {
@Override
@@ -673,8 +678,9 @@
}
// Update superproject gitlinks if required.
SubmoduleOp op = subOpProvider.get();
- try {
- op.updateSuperProjects(db, branches, "receiveID");
+ try (MergeOpRepoManager orm = ormProvider.get()) {
+ orm.setContext(db, TimeUtil.nowTs(), user);
+ op.updateSuperProjects(db, branches, "receiveID", orm);
} catch (SubmoduleException e) {
log.error("Can't update the superprojects", e);
}
@@ -1500,8 +1506,8 @@
private void selectNewAndReplacedChangesFromMagicBranch() {
newChanges = Lists.newArrayList();
- SetMultimap<ObjectId, Ref> existing = HashMultimap.create();
- GroupCollector groupCollector = GroupCollector.create(changeRefsById(), db, psUtil,
+ SetMultimap<ObjectId, Ref> existing = changeRefsById();
+ GroupCollector groupCollector = GroupCollector.create(refsById, db, psUtil,
notesFactory, project.getNameKey());
rp.getRevWalk().reset();
@@ -1522,7 +1528,6 @@
} else {
markHeadsAsUninteresting(
rp.getRevWalk(),
- existing,
magicBranch.ctl != null ? magicBranch.ctl.getRefName() : null);
}
@@ -1679,23 +1684,15 @@
}
}
- private void markHeadsAsUninteresting(
- final RevWalk walk,
- SetMultimap<ObjectId, Ref> existing,
- @Nullable String forRef) {
+ private void markHeadsAsUninteresting(RevWalk rw, @Nullable String forRef) {
for (Ref ref : allRefs.values()) {
- if (ref.getObjectId() == null) {
- continue;
- } else if (ref.getName().startsWith(REFS_CHANGES)) {
- existing.put(ref.getObjectId(), ref);
- } else if (ref.getName().startsWith(R_HEADS)
- || (forRef != null && forRef.equals(ref.getName()))) {
+ if ((ref.getName().startsWith(R_HEADS) || ref.getName().equals(forRef))
+ && ref.getObjectId() != null) {
try {
- walk.markUninteresting(walk.parseCommit(ref.getObjectId()));
+ rw.markUninteresting(rw.parseCommit(ref.getObjectId()));
} catch (IOException e) {
log.warn(String.format("Invalid ref %s in %s",
ref.getName(), project.getName()), e);
- continue;
}
}
}
@@ -2338,11 +2335,11 @@
if (!(parsedObject instanceof RevCommit)) {
return;
}
- SetMultimap<ObjectId, Ref> existing = HashMultimap.create();
walk.markStart((RevCommit)parsedObject);
- markHeadsAsUninteresting(walk, existing, cmd.getRefName());
+ markHeadsAsUninteresting(walk, cmd.getRefName());
+ Set<ObjectId> existing = changeRefsById().keySet();
for (RevCommit c; (c = walk.next()) != null;) {
- if (existing.keySet().contains(c)) {
+ if (existing.contains(c)) {
continue;
} else if (!validCommit(walk, ctl, cmd, c)) {
break;
@@ -2521,7 +2518,6 @@
if (change.getStatus().isOpen()) {
change.setCurrentPatchSet(info);
change.setStatus(Change.Status.MERGED);
- ctx.saveChange();
// we cannot reconstruct the submit records for when this change was
// submitted, this is why we must fix the status
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
index 9e51b3f..3469298 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
@@ -263,7 +263,6 @@
if (mergedIntoRef == null) {
resetChange(ctx, msg);
}
- ctx.saveChange();
return true;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
index e2ec146..63c71a1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
@@ -29,6 +29,8 @@
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
+import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
@@ -67,7 +69,6 @@
private final GitModules.Factory gitmodulesFactory;
private final PersonIdent myIdent;
- private final GitRepositoryManager repoManager;
private final GitReferenceUpdated gitRefUpdated;
private final ProjectCache projectCache;
private final Set<Branch.NameKey> updatedSubscribers;
@@ -82,14 +83,12 @@
GitModules.Factory gitmodulesFactory,
@GerritPersonIdent PersonIdent myIdent,
@GerritServerConfig Config cfg,
- GitRepositoryManager repoManager,
GitReferenceUpdated gitRefUpdated,
ProjectCache projectCache,
@Nullable Account account,
ChangeHooks changeHooks) {
this.gitmodulesFactory = gitmodulesFactory;
this.myIdent = myIdent;
- this.repoManager = repoManager;
this.gitRefUpdated = gitRefUpdated;
this.projectCache = projectCache;
this.account = account;
@@ -99,11 +98,10 @@
this.enableSuperProjectSubscriptions = cfg.getBoolean("submodule",
"enableSuperProjectSubscriptions", true);
updatedSubscribers = new HashSet<>();
-
}
public Collection<Branch.NameKey> getDestinationBranches(Branch.NameKey src,
- SubscribeSection s) throws IOException {
+ SubscribeSection s, MergeOpRepoManager orm) throws IOException {
Collection<Branch.NameKey> ret = new ArrayList<>();
logDebug("Inspecting SubscribeSection " + s);
for (RefSpec r : s.getRefSpecs()) {
@@ -111,11 +109,17 @@
if (r.matchSource(src.get())) {
if (r.getDestination() == null) {
// no need to care for wildcard, as we matched already
- try (Repository repo = repoManager.openRepository(s.getProject())) {
- for (Ref ref : repo.getRefDatabase().getRefs(
- RefNames.REFS_HEADS).values()) {
- ret.add(new Branch.NameKey(s.getProject(), ref.getName()));
- }
+ try {
+ orm.openRepo(s.getProject(), false);
+ } catch (NoSuchProjectException e) {
+ // A project listed a non existent project to be allowed
+ // to subscribe to it. Allow this for now.
+ continue;
+ }
+ OpenRepo or = orm.getRepo(s.getProject());
+ for (Ref ref : or.repo.getRefDatabase().getRefs(
+ RefNames.REFS_HEADS).values()) {
+ ret.add(new Branch.NameKey(s.getProject(), ref.getName()));
}
} else if (r.isWildcard()) {
// refs/heads/*:refs/heads/*
@@ -134,15 +138,16 @@
public Collection<SubmoduleSubscription>
superProjectSubscriptionsForSubmoduleBranch(
- Branch.NameKey branch) throws IOException {
+ Branch.NameKey branch, MergeOpRepoManager orm) throws IOException {
logDebug("Calculating possible superprojects for " + branch);
Collection<SubmoduleSubscription> ret = new ArrayList<>();
Project.NameKey project = branch.getParentKey();
ProjectConfig cfg = projectCache.get(project).getConfig();
for (SubscribeSection s : cfg.getSubscribeSections(branch)) {
- Collection<Branch.NameKey> branches = getDestinationBranches(branch, s);
+ Collection<Branch.NameKey> branches =
+ getDestinationBranches(branch, s, orm);
for (Branch.NameKey targetBranch : branches) {
- GitModules m = gitmodulesFactory.create(targetBranch, updateId);
+ GitModules m = gitmodulesFactory.create(targetBranch, updateId, orm);
m.load();
ret.addAll(m.subscribedTo(branch));
}
@@ -152,8 +157,8 @@
}
protected void updateSuperProjects(ReviewDb db,
- Collection<Branch.NameKey> updatedBranches, String updateId)
- throws SubmoduleException {
+ Collection<Branch.NameKey> updatedBranches, String updateId,
+ MergeOpRepoManager orm) throws SubmoduleException {
if (!enableSuperProjectSubscriptions) {
logDebug("Updating superprojects disabled");
return;
@@ -168,7 +173,7 @@
try {
for (Branch.NameKey updatedBranch : updatedBranches) {
for (SubmoduleSubscription sub :
- superProjectSubscriptionsForSubmoduleBranch(updatedBranch)) {
+ superProjectSubscriptionsForSubmoduleBranch(updatedBranch, orm)) {
targets.put(sub.getSuperProject(), sub);
}
}
@@ -182,7 +187,7 @@
if (!updatedSubscribers.add(dest)) {
log.error("Possible circular subscription involving " + dest);
} else {
- updateGitlinks(db, dest, targets.get(dest));
+ updateGitlinks(db, dest, targets.get(dest), orm);
}
} catch (SubmoduleException e) {
log.warn("Cannot update gitlinks for " + dest, e);
@@ -198,78 +203,92 @@
* @throws SubmoduleException
*/
private void updateGitlinks(ReviewDb db, Branch.NameKey subscriber,
- Collection<SubmoduleSubscription> updates) throws SubmoduleException {
+ Collection<SubmoduleSubscription> updates, MergeOpRepoManager orm)
+ throws SubmoduleException {
PersonIdent author = null;
StringBuilder msgbuf = new StringBuilder("Update git submodules\n\n");
boolean sameAuthorForAll = true;
- try (Repository pdb = repoManager.openRepository(subscriber.getParentKey())) {
- if (pdb.exactRef(subscriber.get()) == null) {
+ try {
+ orm.openRepo(subscriber.getParentKey(), false);
+ } catch (NoSuchProjectException | IOException e) {
+ throw new SubmoduleException("Cannot access superproject", e);
+ }
+ OpenRepo or = orm.getRepo(subscriber.getParentKey());
+ try {
+ Ref r = or.repo.exactRef(subscriber.get());
+ if (r == null) {
throw new SubmoduleException(
"The branch was probably deleted from the subscriber repository");
}
- DirCache dc = readTree(pdb, pdb.exactRef(subscriber.get()));
+ DirCache dc = readTree(r, or.rw);
DirCacheEditor ed = dc.editor();
for (SubmoduleSubscription s : updates) {
- try (Repository subrepo = repoManager.openRepository(
- s.getSubmodule().getParentKey());
- RevWalk rw = CodeReviewCommit.newRevWalk(subrepo)) {
- Ref ref = subrepo.getRefDatabase().exactRef(s.getSubmodule().get());
- if (ref == null) {
- ed.add(new DeletePath(s.getPath()));
+ try {
+ orm.openRepo(s.getSubmodule().getParentKey(), false);
+ } catch (NoSuchProjectException | IOException e) {
+ throw new SubmoduleException("Cannot access submodule", e);
+ }
+ OpenRepo subOr = orm.getRepo(s.getSubmodule().getParentKey());
+ Repository subrepo = subOr.repo;
+
+ Ref ref = subrepo.getRefDatabase().exactRef(s.getSubmodule().get());
+ if (ref == null) {
+ ed.add(new DeletePath(s.getPath()));
+ continue;
+ }
+
+ final ObjectId updateTo = ref.getObjectId();
+ RevCommit newCommit = subOr.rw.parseCommit(updateTo);
+
+ subOr.rw.parseBody(newCommit);
+ if (author == null) {
+ author = newCommit.getAuthorIdent();
+ } else if (!author.equals(newCommit.getAuthorIdent())) {
+ sameAuthorForAll = false;
+ }
+
+ DirCacheEntry dce = dc.getEntry(s.getPath());
+ ObjectId oldId;
+ if (dce != null) {
+ if (!dce.getFileMode().equals(FileMode.GITLINK)) {
+ log.error("Requested to update gitlink " + s.getPath() + " in "
+ + s.getSubmodule().getParentKey().get() + " but entry "
+ + "doesn't have gitlink file mode.");
continue;
}
+ oldId = dce.getObjectId();
+ } else {
+ // This submodule did not exist before. We do not want to add
+ // the full submodule history to the commit message, so omit it.
+ oldId = updateTo;
+ }
- final ObjectId updateTo = ref.getObjectId();
- RevCommit newCommit = rw.parseCommit(updateTo);
-
- if (author == null) {
- author = newCommit.getAuthorIdent();
- } else if (!author.equals(newCommit.getAuthorIdent())) {
- sameAuthorForAll = false;
+ ed.add(new PathEdit(s.getPath()) {
+ @Override
+ public void apply(DirCacheEntry ent) {
+ ent.setFileMode(FileMode.GITLINK);
+ ent.setObjectId(updateTo);
}
+ });
+ if (verboseSuperProject) {
+ msgbuf.append("Project: " + s.getSubmodule().getParentKey().get());
+ msgbuf.append(" " + s.getSubmodule().getShortName());
+ msgbuf.append(" " + updateTo.getName());
+ msgbuf.append("\n\n");
- DirCacheEntry dce = dc.getEntry(s.getPath());
- ObjectId oldId;
- if (dce != null) {
- if (!dce.getFileMode().equals(FileMode.GITLINK)) {
- log.error("Requested to update gitlink " + s.getPath() + " in "
- + s.getSubmodule().getParentKey().get() + " but entry "
- + "doesn't have gitlink file mode.");
- continue;
+ try {
+ subOr.rw.markStart(newCommit);
+ subOr.rw.markUninteresting(subOr.rw.parseCommit(oldId));
+ for (RevCommit c : subOr.rw) {
+ subOr.rw.parseBody(c);
+ msgbuf.append(c.getFullMessage() + "\n\n");
}
- oldId = dce.getObjectId();
- } else {
- // This submodule did not exist before. We do not want to add
- // the full submodule history to the commit message, so omit it.
- oldId = updateTo;
- }
-
- ed.add(new PathEdit(s.getPath()) {
- @Override
- public void apply(DirCacheEntry ent) {
- ent.setFileMode(FileMode.GITLINK);
- ent.setObjectId(updateTo);
- }
- });
- if (verboseSuperProject) {
- msgbuf.append("Project: " + s.getSubmodule().getParentKey().get());
- msgbuf.append(" " + s.getSubmodule().getShortName());
- msgbuf.append(" " + updateTo.getName());
- msgbuf.append("\n\n");
-
- try {
- rw.markStart(newCommit);
- rw.markUninteresting(rw.parseCommit(oldId));
- for (RevCommit c : rw) {
- msgbuf.append(c.getFullMessage() + "\n\n");
- }
- } catch (IOException e) {
- throw new SubmoduleException("Could not perform a revwalk to "
- + "create superproject commit message", e);
- }
+ } catch (IOException e) {
+ throw new SubmoduleException("Could not perform a revwalk to "
+ + "create superproject commit message", e);
}
}
}
@@ -279,11 +298,11 @@
author = myIdent;
}
- ObjectInserter oi = pdb.newObjectInserter();
+ ObjectInserter oi = or.repo.newObjectInserter();
ObjectId tree = dc.writeTree(oi);
ObjectId currentCommitId =
- pdb.exactRef(subscriber.get()).getObjectId();
+ or.repo.exactRef(subscriber.get()).getObjectId();
CommitBuilder commit = new CommitBuilder();
commit.setTreeId(tree);
@@ -296,7 +315,7 @@
ObjectId commitId = oi.idFor(Constants.OBJ_COMMIT, commit.build());
- final RefUpdate rfu = pdb.updateRef(subscriber.get());
+ final RefUpdate rfu = or.repo.updateRef(subscriber.get());
rfu.setForceUpdate(false);
rfu.setNewObjectId(commitId);
rfu.setExpectedOldObjectId(currentCommitId);
@@ -322,24 +341,23 @@
throw new IOException(rfu.getResult().name());
}
// Recursive call: update subscribers of the subscriber
- updateSuperProjects(db, Sets.newHashSet(subscriber), updateId);
+ updateSuperProjects(db, Sets.newHashSet(subscriber), updateId, orm);
} catch (IOException e) {
throw new SubmoduleException("Cannot update gitlinks for "
+ subscriber.get(), e);
}
}
- private static DirCache readTree(final Repository pdb, final Ref branch)
- throws MissingObjectException, IncorrectObjectTypeException, IOException {
- try (RevWalk rw = new RevWalk(pdb)) {
- final DirCache dc = DirCache.newInCore();
- final DirCacheBuilder b = dc.builder();
- b.addTree(new byte[0], // no prefix path
- DirCacheEntry.STAGE_0, // standard stage
- pdb.newObjectReader(), rw.parseTree(branch.getObjectId()));
- b.finish();
- return dc;
- }
+ private static DirCache readTree(final Ref branch, RevWalk rw)
+ throws MissingObjectException, IncorrectObjectTypeException,
+ IOException {
+ final DirCache dc = DirCache.newInCore();
+ final DirCacheBuilder b = dc.builder();
+ b.addTree(new byte[0], // no prefix path
+ DirCacheEntry.STAGE_0, // standard stage
+ rw.getObjectReader(), rw.parseTree(branch.getObjectId()));
+ b.finish();
+ return dc;
}
private void logDebug(String msg, Object... args) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
index 9a9e0b9..91d2cc7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
@@ -160,7 +160,6 @@
prevPs != null ? prevPs.getGroups() : ImmutableList.<String> of(),
null);
ctx.getChange().setCurrentPatchSet(patchSetInfo);
- ctx.saveChange();
// Don't copy approvals, as this is already taken care of by
// SubmitStrategyOp.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
index f886e49..f0dc239 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
@@ -484,7 +484,6 @@
logDebug("Setting change {} merged", c.getId());
c.setStatus(Change.Status.MERGED);
c.setSubmissionId(args.submissionId);
- ctx.saveChange();
// TODO(dborowitz): We need to be able to change the author of the message,
// which is not the user from the update context. addMergedMessage was able
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
index 5ca31d0..e67fba2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
@@ -32,7 +32,7 @@
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
- public static final Class<Schema_123> C = Schema_123.class;
+ public static final Class<Schema_124> C = Schema_124.class;
public static int getBinaryVersion() {
return guessVersion(C);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_124.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_124.java
new file mode 100644
index 0000000..957d683
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_124.java
@@ -0,0 +1,119 @@
+// Copyright (C) 2016 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.schema;
+
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountSshKey;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.account.VersionedAuthorizedKeys;
+import com.google.gerrit.server.account.VersionedAuthorizedKeys.SimpleSshKeyCreator;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gwtorm.jdbc.JdbcSchema;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.NullProgressMonitor;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+import java.io.IOException;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Collection;
+import java.util.Map;
+
+public class Schema_124 extends SchemaVersion {
+ private final GitRepositoryManager repoManager;
+ private final AllUsersName allUsersName;
+ private final PersonIdent serverUser;
+
+ @Inject
+ Schema_124(Provider<Schema_123> prior,
+ GitRepositoryManager repoManager,
+ AllUsersName allUsersName,
+ @GerritPersonIdent PersonIdent serverUser) {
+ super(prior);
+ this.repoManager = repoManager;
+ this.allUsersName = allUsersName;
+ this.serverUser = serverUser;
+ }
+
+ @Override
+ protected void migrateData(ReviewDb db, UpdateUI ui)
+ throws OrmException, SQLException {
+ Multimap<Account.Id, AccountSshKey> imports = ArrayListMultimap.create();
+ try (Statement stmt = ((JdbcSchema) db).getConnection().createStatement();
+ ResultSet rs = stmt.executeQuery(
+ "SELECT "
+ + "account_id, "
+ + "seq, "
+ + "ssh_public_key, "
+ + "valid "
+ + "FROM account_ssh_keys")) {
+ while (rs.next()) {
+ Account.Id accountId = new Account.Id(rs.getInt(1));
+ int seq = rs.getInt(2);
+ String sshPublicKey = rs.getString(3);
+ AccountSshKey key = new AccountSshKey(
+ new AccountSshKey.Id(accountId, seq), sshPublicKey);
+ boolean valid = rs.getBoolean(4);
+ if (!valid) {
+ key.setInvalid();
+ }
+ imports.put(accountId, key);
+ }
+ }
+
+ if (imports.isEmpty()) {
+ return;
+ }
+
+ try (Repository git = repoManager.openRepository(allUsersName);
+ RevWalk rw = new RevWalk(git)) {
+ BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate();
+
+ for (Map.Entry<Account.Id, Collection<AccountSshKey>> e : imports.asMap()
+ .entrySet()) {
+ try (MetaDataUpdate md = new MetaDataUpdate(
+ GitReferenceUpdated.DISABLED, allUsersName, git, bru);
+ VersionedAuthorizedKeys authorizedKeys =
+ new VersionedAuthorizedKeys(
+ new SimpleSshKeyCreator(), e.getKey())) {
+ md.getCommitBuilder().setAuthor(serverUser);
+ md.getCommitBuilder().setCommitter(serverUser);
+
+ authorizedKeys.load(md);
+ authorizedKeys.setKeys(e.getValue());
+ authorizedKeys.commit(md);
+ }
+ }
+
+ bru.execute(rw, NullProgressMonitor.INSTANCE);
+ } catch (ConfigInvalidException | IOException ex) {
+ throw new OrmException(ex);
+ }
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ssh/NoSshKeyCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/ssh/NoSshKeyCache.java
index 0957594..ed9b87c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ssh/NoSshKeyCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ssh/NoSshKeyCache.java
@@ -20,13 +20,14 @@
import com.google.inject.Module;
-public class NoSshKeyCache implements SshKeyCache {
+public class NoSshKeyCache implements SshKeyCache, SshKeyCreator {
public static Module module() {
return new AbstractModule() {
@Override
protected void configure() {
bind(SshKeyCache.class).to(NoSshKeyCache.class);
+ bind(SshKeyCreator.class).to(NoSshKeyCache.class);
}
};
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ssh/SshKeyCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/ssh/SshKeyCache.java
index a3c0a5a..fd8e1b9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ssh/SshKeyCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ssh/SshKeyCache.java
@@ -14,13 +14,7 @@
package com.google.gerrit.server.ssh;
-import com.google.gerrit.common.errors.InvalidSshKeyException;
-import com.google.gerrit.reviewdb.client.AccountSshKey;
-
/** Permits controlling the contents of the SSH key cache area. */
public interface SshKeyCache {
void evict(String username);
-
- AccountSshKey create(AccountSshKey.Id id, String encoded)
- throws InvalidSshKeyException;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ssh/SshKeyCreator.java b/gerrit-server/src/main/java/com/google/gerrit/server/ssh/SshKeyCreator.java
new file mode 100644
index 0000000..fd0c69c
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ssh/SshKeyCreator.java
@@ -0,0 +1,23 @@
+// Copyright (C) 2016 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.ssh;
+
+import com.google.gerrit.common.errors.InvalidSshKeyException;
+import com.google.gerrit.reviewdb.client.AccountSshKey;
+
+public interface SshKeyCreator {
+ AccountSshKey create(AccountSshKey.Id id, String encoded)
+ throws InvalidSshKeyException;
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/account/AuthorizedKeysTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/account/AuthorizedKeysTest.java
new file mode 100644
index 0000000..f5849c1
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/account/AuthorizedKeysTest.java
@@ -0,0 +1,174 @@
+// Copyright (C) 2016 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.account;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.base.Optional;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountSshKey;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class AuthorizedKeysTest {
+ private static final String KEY1 =
+ "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCgug5VyMXQGnem2H1KVC4/HcRcD4zzBqS"
+ + "uJBRWVonSSoz3RoAZ7bWXCVVGwchtXwUURD689wFYdiPecOrWOUgeeyRq754YWRhU+W28"
+ + "vf8IZixgjCmiBhaL2gt3wff6pP+NXJpTSA4aeWE5DfNK5tZlxlSxqkKOS8JRSUeNQov5T"
+ + "w== john.doe@example.com";
+ private static final String KEY2 =
+ "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDm5yP7FmEoqzQRDyskX+9+N0q9GrvZeh5"
+ + "RG52EUpE4ms/Ujm3ewV1LoGzc/lYKJAIbdcZQNJ9+06EfWZaIRA3oOwAPe1eCnX+aLr8E"
+ + "6Tw2gDMQOGc5e9HfyXpC2pDvzauoZNYqLALOG3y/1xjo7IH8GYRS2B7zO/Mf9DdCcCKSf"
+ + "w== john.doe@example.com";
+ private static final String KEY3 =
+ "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCaS7RHEcZ/zjl9hkWkqnm29RNr2OQ/TZ5"
+ + "jk2qBVMH3BgzPsTsEs+7ag9tfD8OCj+vOcwm626mQBZoR2e3niHa/9gnHBHFtOrGfzKbp"
+ + "RjTWtiOZbB9HF+rqMVD+Dawo/oicX/dDg7VAgOFSPothe6RMhbgWf84UcK5aQd5eP5y+t"
+ + "Q== john.doe@example.com";
+ private static final String KEY4 =
+ "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDIJzW9BaAeO+upFletwwEBnGS15lJmS5i"
+ + "08/NiFef0jXtNNKcLtnd13bq8jOi5VA2is0bwof1c8YbwcvUkdFa8RL5aXoyZBpfYZsWs"
+ + "/YBLZGiHy5rjooMZQMnH37A50cBPnXr0AQz0WRBxLDBDyOZho+O/DfYAKv4rzPSQ3yw4+"
+ + "w== john.doe@example.com";
+ private static final String KEY5 =
+ "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCgBRKGhiXvY6D9sM+Vbth5Kate57YF7kD"
+ + "rqIyUiYIMJK93/AXc8qR/J/p3OIFQAxvLz1qozAur3j5HaiwvxVU19IiSA0vafdhaDLRi"
+ + "zRuEL5e/QOu9yGq9xkWApCmg6edpWAHG+Bx4AldU78MiZvzoB7gMMdxc9RmZ1gYj/DjxV"
+ + "w== john.doe@example.com";
+
+ @Test
+ public void test() throws Exception {
+ List<Optional<AccountSshKey>> keys = new ArrayList<>();
+ StringBuilder expected = new StringBuilder();
+ assertSerialization(keys, expected);
+ assertParse(expected, keys);
+
+ expected.append(addKey(keys, KEY1));
+ assertSerialization(keys, expected);
+ assertParse(expected, keys);
+
+ expected.append(addKey(keys, KEY2));
+ assertSerialization(keys, expected);
+ assertParse(expected, keys);
+
+ expected.append(addInvalidKey(keys, KEY3));
+ assertSerialization(keys, expected);
+ assertParse(expected, keys);
+
+ expected.append(addKey(keys, KEY4));
+ assertSerialization(keys, expected);
+ assertParse(expected, keys);
+
+ expected.append(addDeletedKey(keys));
+ assertSerialization(keys, expected);
+ assertParse(expected, keys);
+
+ expected.append(addKey(keys, KEY5));
+ assertSerialization(keys, expected);
+ assertParse(expected, keys);
+ }
+
+ @Test
+ public void testParseWindowsLineEndings() throws Exception {
+ List<Optional<AccountSshKey>> keys = new ArrayList<>();
+ StringBuilder authorizedKeys = new StringBuilder();
+ authorizedKeys.append(toWindowsLineEndings(addKey(keys, KEY1)));
+ assertParse(authorizedKeys, keys);
+
+ authorizedKeys.append(toWindowsLineEndings(addKey(keys, KEY2)));
+ assertParse(authorizedKeys, keys);
+
+ authorizedKeys.append(toWindowsLineEndings(addInvalidKey(keys, KEY3)));
+ assertParse(authorizedKeys, keys);
+
+ authorizedKeys.append(toWindowsLineEndings(addKey(keys, KEY4)));
+ assertParse(authorizedKeys, keys);
+
+ authorizedKeys.append(toWindowsLineEndings(addDeletedKey(keys)));
+ assertParse(authorizedKeys, keys);
+
+ authorizedKeys.append(toWindowsLineEndings(addKey(keys, KEY5)));
+ assertParse(authorizedKeys, keys);
+
+ }
+
+ private static String toWindowsLineEndings(String s) {
+ return s.replaceAll("\n", "\r\n");
+ }
+
+ private static void assertSerialization(List<Optional<AccountSshKey>> keys,
+ StringBuilder expected) {
+ assertThat(AuthorizedKeys.serialize(keys)).isEqualTo(expected.toString());
+ }
+
+ private static void assertParse(StringBuilder authorizedKeys,
+ List<Optional<AccountSshKey>> expectedKeys) {
+ Account.Id accountId = new Account.Id(1);
+ List<Optional<AccountSshKey>> parsedKeys =
+ AuthorizedKeys.parse(accountId, authorizedKeys.toString());
+ assertThat(parsedKeys).containsExactlyElementsIn(expectedKeys);
+ int seq = 1;
+ for(Optional<AccountSshKey> sshKey : parsedKeys) {
+ if (sshKey.isPresent()) {
+ assertThat(sshKey.get().getAccount()).isEqualTo(accountId);
+ assertThat(sshKey.get().getKey().get()).isEqualTo(seq);
+ }
+ seq++;
+ }
+ }
+
+ /**
+ * Adds the given public key as new SSH key to the given list.
+ *
+ * @return the expected line for this key in the authorized_keys file
+ */
+ private static String addKey(List<Optional<AccountSshKey>> keys, String pub) {
+ AccountSshKey.Id keyId =
+ new AccountSshKey.Id(new Account.Id(1), keys.size() + 1);
+ AccountSshKey key = new AccountSshKey(keyId, pub);
+ keys.add(Optional.of(key));
+ return key.getSshPublicKey() + "\n";
+ }
+
+ /**
+ * Adds the given public key as invalid SSH key to the given list.
+ *
+ * @return the expected line for this key in the authorized_keys file
+ */
+ private static String addInvalidKey(List<Optional<AccountSshKey>> keys,
+ String pub) {
+ AccountSshKey.Id keyId =
+ new AccountSshKey.Id(new Account.Id(1), keys.size() + 1);
+ AccountSshKey key = new AccountSshKey(keyId, pub);
+ key.setInvalid();
+ keys.add(Optional.of(key));
+ return AuthorizedKeys.INVALID_KEY_COMMENT_PREFIX
+ + key.getSshPublicKey() + "\n";
+ }
+
+ /**
+ * Adds a deleted SSH key to the given list.
+ *
+ * @return the expected line for this key in the authorized_keys file
+ */
+ private static String addDeletedKey(List<Optional<AccountSshKey>> keys) {
+ keys.add(Optional.<AccountSshKey> absent());
+ return AuthorizedKeys.DELETED_KEY_COMMENT + "\n";
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java
index 01465a2..ad9a46a 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java
@@ -24,7 +24,6 @@
import com.google.gerrit.reviewdb.server.AccountGroupNameAccess;
import com.google.gerrit.reviewdb.server.AccountPatchReviewAccess;
import com.google.gerrit.reviewdb.server.AccountProjectWatchAccess;
-import com.google.gerrit.reviewdb.server.AccountSshKeyAccess;
import com.google.gerrit.reviewdb.server.ChangeAccess;
import com.google.gerrit.reviewdb.server.ChangeMessageAccess;
import com.google.gerrit.reviewdb.server.PatchLineCommentAccess;
@@ -97,11 +96,6 @@
}
@Override
- public AccountSshKeyAccess accountSshKeys() {
- throw new Disabled();
- }
-
- @Override
public AccountGroupAccess accountGroups() {
throw new Disabled();
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshKeyCacheImpl.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshKeyCacheImpl.java
index 4654069..bf3e6bc 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshKeyCacheImpl.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshKeyCacheImpl.java
@@ -18,13 +18,13 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
-import com.google.gerrit.common.errors.InvalidSshKeyException;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.AccountSshKey;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.ssh.SshKeyCache;
-import com.google.gwtorm.server.OrmException;
+import com.google.gerrit.server.ssh.SshKeyCreator;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Module;
@@ -32,12 +32,11 @@
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.security.NoSuchAlgorithmException;
-import java.security.NoSuchProviderException;
-import java.security.spec.InvalidKeySpecException;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -64,6 +63,7 @@
.loader(Loader.class);
bind(SshKeyCacheImpl.class);
bind(SshKeyCache.class).to(SshKeyCacheImpl.class);
+ bind(SshKeyCreator.class).to(SshKeyCreatorImpl.class);
}
};
}
@@ -97,48 +97,34 @@
}
}
- @Override
- public AccountSshKey create(AccountSshKey.Id id, String encoded)
- throws InvalidSshKeyException {
- try {
- final AccountSshKey key =
- new AccountSshKey(id, SshUtil.toOpenSshPublicKey(encoded));
- SshUtil.parse(key);
- return key;
- } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
- throw new InvalidSshKeyException();
-
- } catch (NoSuchProviderException e) {
- log.error("Cannot parse SSH key", e);
- throw new InvalidSshKeyException();
- }
- }
-
static class Loader extends CacheLoader<String, Iterable<SshKeyCacheEntry>> {
private final SchemaFactory<ReviewDb> schema;
+ private final VersionedAuthorizedKeys.Accessor authorizedKeys;
@Inject
- Loader(SchemaFactory<ReviewDb> schema) {
+ Loader(SchemaFactory<ReviewDb> schema,
+ VersionedAuthorizedKeys.Accessor authorizedKeys) {
this.schema = schema;
+ this.authorizedKeys = authorizedKeys;
}
@Override
public Iterable<SshKeyCacheEntry> load(String username) throws Exception {
try (ReviewDb db = schema.open()) {
- final AccountExternalId.Key key =
+ AccountExternalId.Key key =
new AccountExternalId.Key(SCHEME_USERNAME, username);
- final AccountExternalId user = db.accountExternalIds().get(key);
+ AccountExternalId user = db.accountExternalIds().get(key);
if (user == null) {
return NO_SUCH_USER;
}
- final List<SshKeyCacheEntry> kl = new ArrayList<>(4);
- for (AccountSshKey k : db.accountSshKeys().byAccount(
- user.getAccountId())) {
+ List<SshKeyCacheEntry> kl = new ArrayList<>(4);
+ for (AccountSshKey k : authorizedKeys.getKeys(user.getAccountId())) {
if (k.isValid()) {
- add(db, kl, k);
+ add(kl, k);
}
}
+
if (kl.isEmpty()) {
return NO_KEYS;
}
@@ -146,7 +132,7 @@
}
}
- private void add(ReviewDb db, List<SshKeyCacheEntry> kl, AccountSshKey k) {
+ private void add(List<SshKeyCacheEntry> kl, AccountSshKey k) {
try {
kl.add(new SshKeyCacheEntry(k.getKey(), SshUtil.parse(k)));
} catch (OutOfMemoryError e) {
@@ -155,16 +141,16 @@
//
throw e;
} catch (Throwable e) {
- markInvalid(db, k);
+ markInvalid(k);
}
}
- private void markInvalid(final ReviewDb db, final AccountSshKey k) {
+ private void markInvalid(AccountSshKey k) {
try {
log.info("Flagging SSH key " + k.getKey() + " invalid");
+ authorizedKeys.markKeyInvalid(k.getAccount(), k.getKey().get());
k.setInvalid();
- db.accountSshKeys().update(Collections.singleton(k));
- } catch (OrmException e) {
+ } catch (IOException | ConfigInvalidException e) {
log.error("Failed to mark SSH key" + k.getKey() + " invalid", e);
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshKeyCreatorImpl.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshKeyCreatorImpl.java
new file mode 100644
index 0000000..0fd6db4
--- /dev/null
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshKeyCreatorImpl.java
@@ -0,0 +1,48 @@
+// Copyright (C) 2016 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.sshd;
+
+import com.google.gerrit.common.errors.InvalidSshKeyException;
+import com.google.gerrit.reviewdb.client.AccountSshKey;
+import com.google.gerrit.server.ssh.SshKeyCreator;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.security.NoSuchAlgorithmException;
+import java.security.NoSuchProviderException;
+import java.security.spec.InvalidKeySpecException;
+
+public class SshKeyCreatorImpl implements SshKeyCreator {
+ private static final Logger log =
+ LoggerFactory.getLogger(SshKeyCreatorImpl.class);
+
+ @Override
+ public AccountSshKey create(AccountSshKey.Id id, String encoded)
+ throws InvalidSshKeyException {
+ try {
+ AccountSshKey key =
+ new AccountSshKey(id, SshUtil.toOpenSshPublicKey(encoded));
+ SshUtil.parse(key);
+ return key;
+ } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
+ throw new InvalidSshKeyException();
+
+ } catch (NoSuchProviderException e) {
+ log.error("Cannot parse SSH key", e);
+ throw new InvalidSshKeyException();
+ }
+ }
+}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateAccountCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateAccountCommand.java
index acbc50e..a84fe04 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateAccountCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateAccountCommand.java
@@ -29,6 +29,7 @@
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;
@@ -64,7 +65,8 @@
private CreateAccount.Factory createAccountFactory;
@Override
- protected void run() throws OrmException, IOException, UnloggedFailure {
+ protected void run() throws OrmException, IOException, ConfigInvalidException,
+ UnloggedFailure {
CreateAccount.Input input = new CreateAccount.Input();
input.username = username;
input.email = email;
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
index 6963df1..6e03ac1 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
@@ -47,6 +47,8 @@
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;
@@ -167,7 +169,8 @@
}
}
- private void setAccount() throws OrmException, IOException, UnloggedFailure {
+ private void setAccount() throws OrmException, IOException, UnloggedFailure,
+ ConfigInvalidException {
user = genericUserFactory.create(id);
rsrc = new AccountResource(user);
try {
@@ -220,7 +223,7 @@
}
private void addSshKeys(List<String> sshKeys) throws RestApiException,
- OrmException, IOException {
+ OrmException, IOException, ConfigInvalidException {
for (final String sshKey : sshKeys) {
AddSshKey.Input in = new AddSshKey.Input();
in.raw = RawInputUtil.create(sshKey.getBytes(), "plain/text");
@@ -228,8 +231,9 @@
}
}
- private void deleteSshKeys(List<String> sshKeys) throws RestApiException,
- OrmException {
+ private void deleteSshKeys(List<String> sshKeys)
+ throws RestApiException, OrmException, RepositoryNotFoundException,
+ IOException, ConfigInvalidException {
List<SshKeyInfo> infos = getSshKeys.apply(rsrc);
if (sshKeys.contains("ALL")) {
for (SshKeyInfo i : infos) {
@@ -247,7 +251,8 @@
}
}
- private void deleteSshKey(SshKeyInfo i) throws AuthException, OrmException {
+ private void deleteSshKey(SshKeyInfo i) throws AuthException, OrmException,
+ RepositoryNotFoundException, IOException, ConfigInvalidException {
AccountSshKey sshKey = new AccountSshKey(
new AccountSshKey.Id(user.getAccountId(), i.seq), i.sshPublicKey);
deleteSshKey.apply(
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Upload.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Upload.java
index 39efb26..b420a5f 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Upload.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Upload.java
@@ -68,10 +68,8 @@
}
final UploadPack up = new UploadPack(repo);
- if (!projectControl.allRefsAreVisible()) {
- up.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache, repo,
- projectControl, db, true));
- }
+ up.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache, repo,
+ projectControl, db, true));
up.setPackConfig(config.getPackConfig());
up.setTimeout(config.getTimeout());
up.setPostUploadHook(uploadMetrics);
diff --git a/lib/asciidoctor/java/AsciiDoctor.java b/lib/asciidoctor/java/AsciiDoctor.java
index 667f274..8e18feb1 100644
--- a/lib/asciidoctor/java/AsciiDoctor.java
+++ b/lib/asciidoctor/java/AsciiDoctor.java
@@ -151,6 +151,7 @@
}
File[] cssFiles = tmpdir.listFiles(new FilenameFilter() {
+ @Override
public boolean accept(File dir, String name) {
return name.endsWith(".css");
}
diff --git a/plugins/replication b/plugins/replication
index b80cd81..8db7117 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit b80cd8168ae8ba065c0186b1ddfec366a6368cb6
+Subproject commit 8db7117d509498c7d88979034afbc67220d75f6c
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
index 1d88b28..24f915a 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
@@ -68,9 +68,12 @@
<span class="cell keyboard">
<span class="positionIndicator">▶</span>
</span>
- <span class="cell star" hidden$="[[!showStar]]">
+ <span class="cell star" hidden$="[[!showStar]]" hidden>
<gr-change-star change="{{change}}"></gr-change-star>
</span>
+ <a class="cell number" href$="[[changeURL]]" hidden$="[[!showNumber]]" hidden>
+ [[change._number]]
+ </a>
<a class="cell subject" href$="[[changeURL]]">[[change.subject]]</a>
<span class="cell status">[[_computeChangeStatusString(change)]]</span>
<span class="cell owner">
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
index 8ff66cb..bab2014 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
@@ -18,6 +18,7 @@
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior.html">
<link rel="import" href="../../../styles/gr-change-list-styles.html">
+<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-change-list-item/gr-change-list-item.html">
<dom-module id="gr-change-list">
@@ -31,7 +32,8 @@
<style include="gr-change-list-styles"></style>
<div class="headerRow">
<span class="topHeader keyboard"></span> <!-- keyboard position indicator -->
- <span class="topHeader star" hidden$="[[!showStar]]"></span>
+ <span class="topHeader star" hidden$="[[!showStar]]" hidden></span>
+ <span class="topHeader number" hidden$="[[!showNumber]]" hidden>#</span>
<span class="topHeader subject">Subject</span>
<span class="topHeader status">Status</span>
<span class="topHeader owner">Owner</span>
@@ -57,10 +59,12 @@
selected$="[[_computeItemSelected(index, groupIndex, selectedIndex)]]"
needs-review="[[_computeItemNeedsReview(account, change, showReviewedState)]]"
change="[[change]]"
+ show-number="[[showNumber]]"
show-star="[[showStar]]"
label-names="[[labelNames]]"></gr-change-list-item>
</template>
</template>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
</template>
<script src="gr-change-list.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index d1b2456..4e17253 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -58,6 +58,7 @@
type: Number,
notify: true,
},
+ showNumber: Boolean, // No default value to prevent flickering.
showStar: {
type: Boolean,
value: false,
@@ -77,6 +78,31 @@
Gerrit.RESTClientBehavior,
],
+ attached: function() {
+ this._loadPreferences();
+ },
+
+ _loadPreferences: function() {
+ return this._getLoggedIn().then(function(loggedIn) {
+ if (!loggedIn) {
+ this.showNumber = false;
+ return;
+ }
+ return this._getPreferences().then(function(preferences) {
+ this.showNumber = !!(preferences &&
+ preferences.legacycid_in_change_table);
+ }.bind(this));
+ }.bind(this));
+ },
+
+ _getLoggedIn: function() {
+ return this.$.restAPI.getLoggedIn();
+ },
+
+ _getPreferences: function() {
+ return this.$.restAPI.getPreferences();
+ },
+
_computeLabelNames: function(groups) {
if (!groups) { return []; }
var labels = [];
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
index f8f34da..aa77b77 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
@@ -46,6 +46,60 @@
element = fixture('basic');
});
+ function stubRestAPI(preferences) {
+ var loggedInPromise = Promise.resolve(preferences !== null);
+ var preferencesPromise = Promise.resolve(preferences);
+ stub('gr-rest-api-interface', {
+ getLoggedIn: sinon.stub().returns(loggedInPromise),
+ getPreferences: sinon.stub().returns(preferencesPromise),
+ });
+ return Promise.all([loggedInPromise, preferencesPromise]);
+ }
+
+ suite('test show change number not logged in', function() {
+ setup(function(done) {
+ return stubRestAPI(null).then(function() {
+ element = fixture('basic');
+ element._loadPreferences().then(function() { done(); });
+ });
+ });
+
+ test('show number disabled', function() {
+ assert.isFalse(element.showNumber);
+ });
+ });
+
+ suite('test show change number preference enabled', function() {
+ setup(function(done) {
+ return stubRestAPI(
+ {legacycid_in_change_table: true, time_format: 'HHMM_12'}
+ ).then(function() {
+ element = fixture('basic');
+ element._loadPreferences().then(function() { done(); });
+ });
+ });
+
+ test('show number enabled', function() {
+ assert.isTrue(element.showNumber);
+ });
+ });
+
+ suite('test show change number preference disabled', function() {
+ setup(function(done) {
+ // legacycid_in_change_table is not set when false.
+ return stubRestAPI(
+ {time_format: 'HHMM_12'}
+ ).then(function() {
+ element = fixture('basic');
+ element._loadPreferences().then(function() { done(); });
+ });
+ });
+
+ test('show number disabled', function() {
+ assert.isFalse(element.showNumber);
+ });
+ });
+
test('computed fields', function() {
assert.equal(element._computeLabelNames(
[[{_number: 0, labels: {}}]]).length, 0);
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 d94a828..1066625 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
@@ -16,7 +16,6 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
-<link rel="import" href="../../shared/gr-request/gr-request.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<dom-module id="gr-file-list">
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index ad2b925..71fee37 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -19,7 +19,6 @@
<link rel="import" href="../../../bower_components/iron-selector/iron-selector.html">
<link rel="import" href="../../../behaviors/rest-client-behavior.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
-<link rel="import" href="../../shared/gr-request/gr-request.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<dom-module id="gr-reply-dialog">
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 1cd0ce2..47324ba 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -46,7 +46,6 @@
permittedLabels: Object,
_account: Object,
- _xhrPromise: Object, // Used for testing.
},
behaviors: [
@@ -140,26 +139,24 @@
obj.message = this.draft;
}
this.disabled = true;
- this._send(obj).then(function(req) {
- this.fire('send', null, {bubbles: false});
- this.draft = '';
+ this._saveReview(obj).then(function(response) {
this.disabled = false;
- }.bind(this)).catch(function(err) {
- alert('Oops. Something went wrong. Check the console and bug the ' +
- 'PolyGerrit team for assistance.');
- throw err;
+ if (!response.ok) {
+ alert('Oops. Something went wrong. Check the console and bug the ' +
+ 'PolyGerrit team for assistance.');
+ return response.text().then(function(text) {
+ console.error(text);
+ });
+ }
+
+ this.draft = '';
+ this.fire('send', null, {bubbles: false});
}.bind(this));
},
- _send: function(payload) {
- var xhr = document.createElement('gr-request');
- this._xhrPromise = xhr.send({
- method: 'POST',
- url: this.changeBaseURL(this.changeNum, this.patchNum) + '/review',
- body: payload,
- });
-
- return this._xhrPromise;
+ _saveReview: function(review) {
+ return this.$.restAPI.saveChangeReview(this.changeNum, this.patchNum,
+ review);
},
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index fb2de6a..bc850e3 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -34,11 +34,10 @@
<script>
suite('gr-reply-dialog tests', function() {
var element;
- var server;
setup(function() {
- stub('gr-reply-dialog', {
- _getAccount: function() { return Promise.resolve({}); },
+ stub('gr-rest-api-interface', {
+ getAccount: function() { return Promise.resolve({}); },
});
element = fixture('basic');
element.changeNum = 42;
@@ -76,31 +75,10 @@
]
};
- server = sinon.fakeServer.create();
- server.respondWith(
- 'POST',
- '/changes/42/revisions/1/review',
- [
- 200,
- {'Content-Type': 'application/json'},
- ')]}\'\n' +
- '{' +
- '"labels": {' +
- '"Code-Review": -1,' +
- '"Verified": -1' +
- '}' +
- '}'
- ]
- );
-
// Allow the elements created by dom-repeat to be stamped.
flushAsynchronousOperations();
});
- teardown(function() {
- server.restore();
- });
-
test('cancel event', function(done) {
element.addEventListener('cancel', function() { done(); });
MockInteractions.tap(element.$$('.cancel'));
@@ -122,32 +100,32 @@
'iron-selector[data-label="Verified"] > ' +
'gr-button[data-value="-1"]'));
+ var saveReviewStub = sinon.stub(element, '_saveReview',
+ function(review) {
+ assert.deepEqual(review, {
+ drafts: 'PUBLISH_ALL_REVISIONS',
+ labels: {
+ 'Code-Review': -1,
+ 'Verified': -1
+ },
+ message: 'I wholeheartedly disapprove'
+ });
+ return Promise.resolve({ok: true});
+ });
+
+ element.addEventListener('send', function() {
+ assert.isFalse(element.disabled,
+ 'Element should be enabled when done sending reply.');
+ assert.equal(element.draft.length, 0);
+ saveReviewStub.restore();
+ done();
+ });
+
// This is needed on non-Blink engines most likely due to the ways in
// which the dom-repeat elements are stamped.
flush(function() {
MockInteractions.tap(element.$$('.send'));
assert.isTrue(element.disabled);
-
- server.respond();
-
- element._xhrPromise.then(function(req) {
- assert.isFalse(element.disabled,
- 'Element should be enabled when done sending reply.');
- assert.equal(req.status, 200);
- assert.equal(req.url, '/changes/42/revisions/1/review');
- var reqObj = JSON.parse(req.xhr.requestBody);
- assert.deepEqual(reqObj, {
- drafts: 'PUBLISH_ALL_REVISIONS',
- labels: {
- 'Code-Review': -1,
- 'Verified': -1
- },
- message: 'I wholeheartedly disapprove'
- });
- assert.equal(req.response.labels['Code-Review'], -1);
- assert.equal(req.response.labels.Verified, -1);
- done();
- });
});
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-alert/gr-alert.html b/polygerrit-ui/app/elements/shared/gr-alert/gr-alert.html
new file mode 100644
index 0000000..28e45a4
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-alert/gr-alert.html
@@ -0,0 +1,58 @@
+<!--
+Copyright (C) 2016 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.
+-->
+
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../gr-button/gr-button.html">
+
+<dom-module id="gr-alert">
+ <template>
+ <style>
+ /**
+ * ALERT: DO NOT ADD TRANSITION PROPERTIES WITHOUT PROPERLY UNDERSTANDING
+ * HOW THEY ARE USED IN THE CODE.
+ */
+ :host([toast]) {
+ background-color: #333;
+ bottom: 1.25rem;
+ border-radius: 3px;
+ box-shadow: 0 1px 3px rgba(0, 0, 0, .3);
+ color: #fff;
+ left: 1.25rem;
+ padding: 1em 1.5em;
+ position: fixed;
+ transform: translateY(5rem);
+ transition: transform var(--gr-alert-transition-duration, 80ms) ease-out;
+ }
+ :host([shown]) {
+ transform: translateY(0);
+ }
+ .action {
+ color: #a1c2fa;
+ font-weight: bold;
+ margin-left: 1em;
+ text-decoration: none;
+ }
+ </style>
+ [[text]]
+ <gr-button
+ link
+ class="action"
+ hidden$="[[!actionText]]"
+ on-tap="_handleActionTap">[[actionText]]</gr-button>
+ </template>
+ <script src="gr-alert.js"></script>
+</dom-module>
+
diff --git a/polygerrit-ui/app/elements/shared/gr-alert/gr-alert.js b/polygerrit-ui/app/elements/shared/gr-alert/gr-alert.js
new file mode 100644
index 0000000..ba481ec
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-alert/gr-alert.js
@@ -0,0 +1,88 @@
+// Copyright (C) 2016 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.
+(function() {
+ 'use strict';
+
+ Polymer({
+ is: 'gr-alert',
+
+ /**
+ * Fired when the action button is pressed.
+ *
+ * @event action
+ */
+
+ properties: {
+ text: String,
+ actionText: String,
+ shown: {
+ type: Boolean,
+ value: true,
+ readOnly: true,
+ reflectToAttribute: true,
+ },
+ toast: {
+ type: Boolean,
+ value: true,
+ reflectToAttribute: true,
+ },
+
+ _boundTransitionEndHandler: {
+ type: Function,
+ value: function() { return this._handleTransitionEnd.bind(this); },
+ },
+ },
+
+ attached: function() {
+ this.addEventListener('transitionend', this._boundTransitionEndHandler);
+ },
+
+ detached: function() {
+ this.removeEventListener('transitionend',
+ this._boundTransitionEndHandler);
+ },
+
+ show: function(text, opt_actionText) {
+ this.text = text;
+ this.actionText = opt_actionText;
+ document.body.appendChild(this);
+ this._setShown(true);
+ },
+
+ hide: function() {
+ this._setShown(false);
+ if (this._hasZeroTransitionDuration()) {
+ document.body.removeChild(this);
+ }
+ },
+
+ _hasZeroTransitionDuration: function() {
+ var style = window.getComputedStyle(this);
+ // transitionDuration is always given in seconds.
+ var duration = Math.round(parseFloat(style.transitionDuration) * 100);
+ return duration === 0;
+ },
+
+ _handleTransitionEnd: function(e) {
+ if (this.shown) { return; }
+
+ document.body.removeChild(this);
+ },
+
+ _handleActionTap: function(e) {
+ e.preventDefault();
+ this.fire('action', null, {bubbles: false});
+ },
+ });
+})();
diff --git a/polygerrit-ui/app/elements/shared/gr-alert/gr-alert_test.html b/polygerrit-ui/app/elements/shared/gr-alert/gr-alert_test.html
new file mode 100644
index 0000000..067ac5b
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-alert/gr-alert_test.html
@@ -0,0 +1,60 @@
+<!DOCTYPE html>
+<!--
+Copyright (C) 2015 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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-alert</title>
+
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="../../../bower_components/web-component-tester/browser.js"></script>
+
+<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
+<link rel="import" href="gr-alert.html">
+
+<script>
+ suite('gr-alert tests', function() {
+ var element;
+
+ setup(function() {
+ element = document.createElement('gr-alert');
+ });
+
+ teardown(function() {
+ if (element.parentNode) {
+ element.parentNode.removeChild(element);
+ }
+ });
+
+ test('show/hide', function() {
+ assert.isNull(element.parentNode);
+ element.show();
+ assert.equal(element.parentNode, document.body);
+ element.customStyle['--gr-alert-transition-duration'] = '0ms';
+ element.updateStyles();
+ element.hide();
+ assert.isNull(element.parentNode);
+ });
+
+ test('action event', function(done) {
+ element.show();
+ element.addEventListener('action', function() {
+ done();
+ });
+ MockInteractions.tap(element.$$('.action'));
+ });
+
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index f23d50b..54ce15c 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -325,6 +325,12 @@
return this.send(method, url, null, opt_errFn, opt_ctx);
},
+ saveChangeReview: function(changeNum, patchNum, review, opt_errFn,
+ opt_ctx) {
+ var url = this.getChangeActionURL(changeNum, patchNum, '/review');
+ return this.send('POST', url, review, opt_errFn, opt_ctx);
+ },
+
send: function(method, url, opt_body, opt_errFn, opt_ctx) {
var headers = new Headers({
'X-Gerrit-Auth': this._getCookie('XSRF_TOKEN'),
diff --git a/polygerrit-ui/app/styles/gr-change-list-styles.html b/polygerrit-ui/app/styles/gr-change-list-styles.html
index b4cc415..6e34c2b 100644
--- a/polygerrit-ui/app/styles/gr-change-list-styles.html
+++ b/polygerrit-ui/app/styles/gr-change-list-styles.html
@@ -44,6 +44,9 @@
.star {
padding-top: .05em;
}
+ .number {
+ width: 4em;
+ }
.subject {
flex-grow: 1;
flex-shrink: 1;
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 4238f9a..dc12bf9 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -48,6 +48,7 @@
'../elements/diff/gr-diff-preferences/gr-diff-preferences_test.html',
'../elements/diff/gr-diff-view/gr-diff-view_test.html',
'../elements/diff/gr-patch-range-select/gr-patch-range-select_test.html',
+ '../elements/shared/gr-alert/gr-alert_test.html',
'../elements/shared/gr-account-label/gr-account-label_test.html',
'../elements/shared/gr-account-link/gr-account-link_test.html',
'../elements/shared/gr-avatar/gr-avatar_test.html',
