Merge "Fix undefined returned from JSNI method"
diff --git a/.buckversion b/.buckversion
index 62da292..187f479 100644
--- a/.buckversion
+++ b/.buckversion
@@ -1 +1 @@
-620fdf494f66c2876cfc7cae6f6feb9d36e45df4
+e42f4f1395e32a8b2e3a3ddf848e178b8558e1e9
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 2b01a22..598570da 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -27,11 +27,16 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ListChangesOption;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.reviewdb.client.AccountGroup;
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.OutputFormat;
import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.Util;
import com.google.gerrit.testutil.ConfigSuite;
import com.google.gson.Gson;
import com.google.gwtorm.server.SchemaFactory;
@@ -77,6 +82,12 @@
@Inject
protected PushOneCommit.Factory pushFactory;
+ @Inject
+ protected MetaDataUpdate.Server metaDataUpdateFactory;
+
+ @Inject
+ protected ProjectCache projectCache;
+
protected Git git;
protected GerritServer server;
protected TestAccount admin;
@@ -218,4 +229,29 @@
.id(r.getChangeId())
.current();
}
+
+ protected void allow(String permission, AccountGroup.UUID id, String ref)
+ throws Exception {
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ Util.allow(cfg, permission, id, ref);
+ saveProjectConfig(project, cfg);
+ }
+
+ protected void deny(String permission, AccountGroup.UUID id, String ref)
+ throws Exception {
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ Util.deny(cfg, permission, id, ref);
+ saveProjectConfig(project, cfg);
+ }
+
+ protected void saveProjectConfig(Project.NameKey p, ProjectConfig cfg)
+ throws Exception {
+ MetaDataUpdate md = metaDataUpdateFactory.create(p);
+ try {
+ cfg.commit(md);
+ } finally {
+ md.close();
+ }
+ projectCache.evict(cfg.getProject());
+ }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GitUtil.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GitUtil.java
index 3234ffd..3371eb2 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GitUtil.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GitUtil.java
@@ -100,6 +100,10 @@
b.append("\"");
}
s.exec(b.toString());
+ if (s.hasError()) {
+ throw new IllegalStateException(
+ "gerrit create-project returned error: " + s.getError());
+ }
}
public static Git cloneProject(String url) throws GitAPIException, IOException {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/SshSession.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/SshSession.java
index dc648fc..701b337 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/SshSession.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/SshSession.java
@@ -14,18 +14,19 @@
package com.google.gerrit.acceptance;
-import java.io.IOException;
-import java.io.InputStream;
-import java.net.InetSocketAddress;
-import java.util.Scanner;
+import static com.google.common.base.Preconditions.checkState;
import com.jcraft.jsch.ChannelExec;
import com.jcraft.jsch.JSch;
import com.jcraft.jsch.JSchException;
import com.jcraft.jsch.Session;
-public class SshSession {
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.InetSocketAddress;
+import java.util.Scanner;
+public class SshSession {
private final InetSocketAddress addr;
private final TestAccount account;
private Session session;
@@ -36,6 +37,10 @@
this.account = account;
}
+ public void open() throws JSchException {
+ getSession();
+ }
+
@SuppressWarnings("resource")
public String exec(String command) throws JSchException, IOException {
ChannelExec channel = (ChannelExec) getSession().openChannel("exec");
@@ -86,6 +91,7 @@
}
public String getUrl() {
+ checkState(session != null, "session must be opened");
StringBuilder b = new StringBuilder();
b.append("ssh://");
b.append(session.getUserName());
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/ProjectIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/ProjectIT.java
index a10e026..9aa3917 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -14,22 +14,22 @@
package com.google.gerrit.acceptance.api.project;
+import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import org.eclipse.jgit.api.errors.GitAPIException;
import org.junit.Test;
-import java.io.IOException;
import java.util.List;
@NoHttpd
@@ -68,8 +68,8 @@
}
@Test
- public void createBranch() throws GitAPIException,
- IOException, RestApiException {
+ public void createBranch() throws Exception {
+ allow(Permission.READ, ANONYMOUS_USERS, "refs/*");
gApi.projects()
.name(project.get())
.branch("foo")
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK
index 3ead2a1..bce2f65 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK
@@ -1,7 +1,11 @@
include_defs('//gerrit-acceptance-tests/tests.defs')
acceptance_tests(
- srcs = ['DraftChangeBlockedIT.java', 'SubmitOnPushIT.java'],
+ srcs = [
+ 'DraftChangeBlockedIT.java',
+ 'SubmitOnPushIT.java',
+ 'VisibleRefFilterIT.java',
+ ],
labels = ['git'],
)
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
index a73169d..917c283 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
@@ -14,8 +14,6 @@
package com.google.gerrit.acceptance.git;
-import static com.google.gerrit.acceptance.GitUtil.cloneProject;
-import static com.google.gerrit.acceptance.GitUtil.createProject;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -24,7 +22,6 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.SshSession;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
@@ -33,7 +30,6 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.account.GroupCache;
@@ -44,10 +40,8 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
-import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -57,18 +51,12 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.RefSpec;
-import org.junit.After;
-import org.junit.Before;
import org.junit.Test;
import java.io.IOException;
@NoHttpd
public class SubmitOnPushIT extends AbstractDaemonTest {
-
- @Inject
- private SchemaFactory<ReviewDb> reviewDbProvider;
-
@Inject
private GitRepositoryManager repoManager;
@@ -93,26 +81,6 @@
@Inject
private PushOneCommit.Factory pushFactory;
- private Project.NameKey project;
- private Git git;
- private ReviewDb db;
-
- @Before
- public void setUp() throws Exception {
- project = new Project.NameKey("p");
- SshSession sshSession = new SshSession(server, admin);
- createProject(sshSession, project.get());
- git = cloneProject(sshSession.getUrl() + "/" + project.get());
- sshSession.close();
-
- db = reviewDbProvider.open();
- }
-
- @After
- public void cleanup() {
- db.close();
- }
-
@Test
public void submitOnPush() throws GitAPIException, OrmException,
IOException, ConfigInvalidException {
@@ -129,6 +97,7 @@
IOException, ConfigInvalidException {
grant(Permission.SUBMIT, project, "refs/for/refs/heads/master");
grant(Permission.CREATE, project, "refs/tags/*");
+ grant(Permission.PUSH, project, "refs/tags/*");
final String tag = "v1.0";
PushOneCommit push = pushFactory.create(db, admin.getIdent());
push.setTag(tag);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java
new file mode 100644
index 0000000..4a1227c
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java
@@ -0,0 +1,131 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.git;
+
+import static com.google.gerrit.server.group.SystemGroupBackend.PROJECT_OWNERS;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+import com.google.common.base.CharMatcher;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Ordering;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.project.Util;
+import com.google.inject.Inject;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+public class VisibleRefFilterIT extends AbstractDaemonTest {
+ @Inject
+ AllProjectsName allProjects;
+
+ @Before
+ public void setUp() throws Exception {
+ setUpChanges();
+ setUpPermissions();
+ }
+
+ private void setUpPermissions() throws Exception {
+ ProjectConfig pc = projectCache.checkedGet(allProjects).getConfig();
+ for (AccessSection sec : pc.getAccessSections()) {
+ sec.removePermission(Permission.READ);
+ }
+ saveProjectConfig(allProjects, pc);
+ }
+
+ private void setUpChanges() throws Exception {
+ gApi.projects()
+ .name(project.get())
+ .branch("branch")
+ .create(new BranchInput());
+
+ pushFactory.create(db, admin.getIdent())
+ .to(git, "refs/for/master")
+ .assertOkStatus();
+ pushFactory.create(db, admin.getIdent())
+ .to(git, "refs/for/branch")
+ .assertOkStatus();
+ }
+
+ @Test
+ public void allRefsVisibleNoRefsMetaConfig() throws Exception {
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ Util.allow(cfg, Permission.READ, REGISTERED_USERS, "refs/*");
+ Util.allow(cfg, Permission.READ, PROJECT_OWNERS, "refs/meta/config");
+ Util.doNotInherit(cfg, Permission.READ, "refs/meta/config");
+ saveProjectConfig(project, cfg);
+
+ assertRefs(
+ "HEAD",
+ "refs/changes/01/1/1",
+ "refs/changes/02/2/1",
+ "refs/heads/branch",
+ "refs/heads/master");
+ }
+
+ @Test
+ public void allRefsVisibleWithRefsMetaConfig() throws Exception {
+ allow(Permission.READ, REGISTERED_USERS, "refs/*");
+ allow(Permission.READ, REGISTERED_USERS, "refs/meta/config");
+
+ assertRefs(
+ "HEAD",
+ "refs/changes/01/1/1",
+ "refs/changes/02/2/1",
+ "refs/heads/branch",
+ "refs/heads/master",
+ "refs/meta/config");
+ }
+
+ @Test
+ public void subsetOfBranchesVisibleIncludingHead() throws Exception {
+ allow(Permission.READ, REGISTERED_USERS, "refs/heads/master");
+ deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
+
+ assertRefs(
+ "HEAD",
+ "refs/changes/01/1/1",
+ "refs/heads/master");
+ }
+
+ @Test
+ public void subsetOfBranchesVisibleNotIncludingHead() throws Exception {
+ deny(Permission.READ, REGISTERED_USERS, "refs/heads/master");
+ allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
+
+ assertRefs(
+ "refs/changes/02/2/1",
+ "refs/heads/branch");
+ }
+
+ private void assertRefs(String... expected) throws Exception {
+ String out = sshSession.exec(String.format(
+ "gerrit ls-user-refs -p %s -u %s",
+ project.get(), user.getId().get()));
+ assertFalse(sshSession.getError(), sshSession.hasError());
+ Splitter s = Splitter.on(CharMatcher.WHITESPACE).omitEmptyStrings();
+ assertEquals(Arrays.asList(expected),
+ Ordering.natural().sortedCopy(s.split(out)));
+ }
+}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeOwnerIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeOwnerIT.java
index 83efd30..42043cc 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeOwnerIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeOwnerIT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.acceptance.rest.change;
import static com.google.gerrit.acceptance.GitUtil.cloneProject;
-import static com.google.gerrit.acceptance.GitUtil.createProject;
import static com.google.gerrit.acceptance.GitUtil.initSsh;
import static com.google.gerrit.common.data.Permission.LABEL;
import static org.junit.Assert.assertEquals;
@@ -63,8 +62,7 @@
sessionOwner = new RestSession(server, user);
SshSession sshSession = new SshSession(server, user);
initSsh(user);
- // need to initialize intern session
- createProject(sshSession, "foo");
+ sshSession.open();
git = cloneProject(sshSession.getUrl() + "/" + project.get());
sshSession.close();
user2 = accounts.user2();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/CacheOperationsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/CacheOperationsIT.java
index d2174bc..deb8066 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/CacheOperationsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/CacheOperationsIT.java
@@ -17,7 +17,6 @@
import static com.google.gerrit.server.config.PostCaches.Operation.FLUSH;
import static com.google.gerrit.server.config.PostCaches.Operation.FLUSH_ALL;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static com.google.gerrit.server.project.Util.allow;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -26,13 +25,14 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.server.config.ListCaches.CacheInfo;
import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.ListCaches.CacheInfo;
import com.google.gerrit.server.config.PostCaches;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.Util;
import com.google.inject.Inject;
import org.apache.http.HttpStatus;
@@ -140,8 +140,8 @@
ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
AccountGroup.UUID registeredUsers =
SystemGroupBackend.getGroup(REGISTERED_USERS).getUUID();
- allow(cfg, GlobalCapability.VIEW_CACHES, registeredUsers);
- allow(cfg, GlobalCapability.FLUSH_CACHES, registeredUsers);
+ Util.allow(cfg, GlobalCapability.VIEW_CACHES, registeredUsers);
+ Util.allow(cfg, GlobalCapability.FLUSH_CACHES, registeredUsers);
saveProjectConfig(cfg);
RestResponse r = userSession.post("/config/server/caches/",
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/FlushCacheIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/FlushCacheIT.java
index aa8d7ba..31ffd7b 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/FlushCacheIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/FlushCacheIT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.acceptance.rest.config;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static com.google.gerrit.server.project.Util.allow;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -30,6 +29,7 @@
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.Util;
import com.google.inject.Inject;
import org.apache.http.HttpStatus;
@@ -92,8 +92,8 @@
ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
AccountGroup.UUID registeredUsers =
SystemGroupBackend.getGroup(REGISTERED_USERS).getUUID();
- allow(cfg, GlobalCapability.VIEW_CACHES, registeredUsers);
- allow(cfg, GlobalCapability.FLUSH_CACHES, registeredUsers);
+ Util.allow(cfg, GlobalCapability.VIEW_CACHES, registeredUsers);
+ Util.allow(cfg, GlobalCapability.FLUSH_CACHES, registeredUsers);
saveProjectConfig(cfg);
RestResponse r = userSession.post("/config/server/caches/accounts/flush");
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/DefaultGroupsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/DefaultGroupsIT.java
index 0d229ca..f60b5dd 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/DefaultGroupsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/DefaultGroupsIT.java
@@ -14,6 +14,7 @@
package com.google.gerrit.acceptance.rest.group;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import com.google.common.collect.Sets;
@@ -47,6 +48,7 @@
public void defaultGroupsCreated_ssh() throws JSchException, IOException {
SshSession session = new SshSession(server, admin);
String result = session.exec("gerrit ls-groups");
+ assertFalse(session.getError(), session.hasError());
assertTrue(result.contains("Administrators"));
assertTrue(result.contains("Non-Interactive Users"));
session.close();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
index 23cd278..a9bba75 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
@@ -16,7 +16,6 @@
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static com.google.gerrit.server.project.Util.allow;
import static com.google.gerrit.server.project.Util.block;
import static org.junit.Assert.assertEquals;
@@ -24,28 +23,16 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.reviewdb.client.Branch;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.AllProjectsName;
-import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
-import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import org.apache.http.HttpStatus;
-import org.eclipse.jgit.errors.ConfigInvalidException;
import org.junit.Before;
import org.junit.Test;
-import java.io.IOException;
-
public class CreateBranchIT extends AbstractDaemonTest {
@Inject
- private MetaDataUpdate.Server metaDataUpdateFactory;
-
- @Inject
- private ProjectCache projectCache;
-
- @Inject
private AllProjectsName allProjects;
private Branch.NameKey branch;
@@ -56,7 +43,7 @@
}
@Test
- public void createBranch_Forbidden() throws IOException {
+ public void createBranch_Forbidden() throws Exception {
RestResponse r =
userSession.put("/projects/" + project.get()
+ "/branches/" + branch.getShortName());
@@ -64,7 +51,7 @@
}
@Test
- public void createBranchByAdmin() throws IOException {
+ public void createBranchByAdmin() throws Exception {
RestResponse r =
adminSession.put("/projects/" + project.get()
+ "/branches/" + branch.getShortName());
@@ -77,7 +64,7 @@
}
@Test
- public void branchAlreadyExists_Conflict() throws IOException {
+ public void branchAlreadyExists_Conflict() throws Exception {
RestResponse r =
adminSession.put("/projects/" + project.get()
+ "/branches/" + branch.getShortName());
@@ -90,8 +77,7 @@
}
@Test
- public void createBranchByProjectOwner() throws IOException,
- ConfigInvalidException {
+ public void createBranchByProjectOwner() throws Exception {
grantOwner();
RestResponse r =
@@ -106,8 +92,7 @@
}
@Test
- public void createBranchByAdminCreateReferenceBlocked() throws IOException,
- ConfigInvalidException {
+ public void createBranchByAdminCreateReferenceBlocked() throws Exception {
blockCreateReference();
RestResponse r =
adminSession.put("/projects/" + project.get()
@@ -122,7 +107,7 @@
@Test
public void createBranchByProjectOwnerCreateReferenceBlocked_Forbidden()
- throws IOException, ConfigInvalidException {
+ throws Exception {
grantOwner();
blockCreateReference();
RestResponse r =
@@ -131,27 +116,13 @@
assertEquals(HttpStatus.SC_FORBIDDEN, r.getStatusCode());
}
- private void blockCreateReference() throws IOException, ConfigInvalidException {
+ private void blockCreateReference() throws Exception {
ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
block(cfg, Permission.CREATE, ANONYMOUS_USERS, "refs/*");
saveProjectConfig(allProjects, cfg);
- projectCache.evict(cfg.getProject());
}
- private void grantOwner() throws IOException, ConfigInvalidException {
- ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
- allow(cfg, Permission.OWNER, REGISTERED_USERS, "refs/*");
- saveProjectConfig(project, cfg);
- projectCache.evict(cfg.getProject());
- }
-
- private void saveProjectConfig(Project.NameKey p, ProjectConfig cfg)
- throws IOException {
- MetaDataUpdate md = metaDataUpdateFactory.create(p);
- try {
- cfg.commit(md);
- } finally {
- md.close();
- }
+ private void grantOwner() throws Exception {
+ allow(Permission.OWNER, REGISTERED_USERS, "refs/*");
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
index 0d6ec5b..9a5aaa3 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
@@ -16,7 +16,6 @@
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static com.google.gerrit.server.project.Util.allow;
import static com.google.gerrit.server.project.Util.block;
import static org.junit.Assert.assertEquals;
@@ -24,28 +23,17 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.reviewdb.client.Branch;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.AllProjectsName;
-import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
-import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import org.apache.http.HttpStatus;
-import org.eclipse.jgit.errors.ConfigInvalidException;
import org.junit.Before;
import org.junit.Test;
import java.io.IOException;
public class DeleteBranchIT extends AbstractDaemonTest {
-
- @Inject
- private MetaDataUpdate.Server metaDataUpdateFactory;
-
- @Inject
- private ProjectCache projectCache;
-
@Inject
private AllProjectsName allProjects;
@@ -82,8 +70,7 @@
}
@Test
- public void deleteBranchByProjectOwner() throws IOException,
- ConfigInvalidException {
+ public void deleteBranchByProjectOwner() throws Exception {
grantOwner();
RestResponse r =
@@ -99,8 +86,7 @@
}
@Test
- public void deleteBranchByAdminForcePushBlocked() throws IOException,
- ConfigInvalidException {
+ public void deleteBranchByAdminForcePushBlocked() throws Exception {
blockForcePush();
RestResponse r =
adminSession.delete("/projects/" + project.get()
@@ -116,7 +102,7 @@
@Test
public void deleteBranchByProjectOwnerForcePushBlocked_Forbidden()
- throws IOException, ConfigInvalidException {
+ throws Exception {
grantOwner();
blockForcePush();
RestResponse r =
@@ -126,26 +112,13 @@
r.consume();
}
- private void blockForcePush() throws IOException, ConfigInvalidException {
+ private void blockForcePush() throws Exception {
ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
block(cfg, Permission.PUSH, ANONYMOUS_USERS, "refs/heads/*").setForce(true);
saveProjectConfig(allProjects, cfg);
- projectCache.evict(cfg.getProject());
}
- private void grantOwner() throws IOException, ConfigInvalidException {
- ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
- allow(cfg, Permission.OWNER, REGISTERED_USERS, "refs/*");
- saveProjectConfig(project, cfg);
- projectCache.evict(cfg.getProject());
- }
-
- private void saveProjectConfig(Project.NameKey p, ProjectConfig cfg) throws IOException {
- MetaDataUpdate md = metaDataUpdateFactory.create(p);
- try {
- cfg.commit(md);
- } finally {
- md.close();
- }
+ private void grantOwner() throws Exception {
+ allow(Permission.OWNER, REGISTERED_USERS, "refs/*");
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetCommitIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
index 2aacf20..d4a5b3d 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
@@ -14,98 +14,147 @@
package com.google.gerrit.acceptance.rest.project;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNull;
+import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.common.CommitInfo;
-import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.config.AllProjectsName;
-import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ProjectConfig;
-import com.google.gerrit.server.project.ListBranches.BranchInfo;
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import org.apache.http.HttpStatus;
+import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
-import java.io.IOException;
-
public class GetCommitIT extends AbstractDaemonTest {
-
- @Inject
- private ProjectCache projectCache;
-
@Inject
private AllProjectsName allProjects;
@Inject
- private MetaDataUpdate.Server metaDataUpdateFactory;
+ private GitRepositoryManager repoManager;
- @Test
- public void getCommit() throws IOException {
- RestResponse r =
- adminSession.get("/projects/" + project.get() + "/branches/"
- + IdString.fromDecoded(RefNames.REFS_CONFIG).encoded());
- assertEquals(HttpStatus.SC_OK, r.getStatusCode());
- BranchInfo branchInfo =
- newGson().fromJson(r.getReader(), BranchInfo.class);
- r.consume();
+ @Inject
+ private ProjectCache projectCache;
- r = adminSession.get("/projects/" + project.get() + "/commits/"
- + branchInfo.revision);
- assertEquals(HttpStatus.SC_OK, r.getStatusCode());
- CommitInfo commitInfo =
- newGson().fromJson(r.getReader(), CommitInfo.class);
- assertEquals(branchInfo.revision, commitInfo.commit);
- assertEquals("Created project", commitInfo.subject);
- assertEquals("Created project\n", commitInfo.message);
- assertNotNull(commitInfo.author);
- assertEquals("Administrator", commitInfo.author.name);
- assertNotNull(commitInfo.committer);
- assertEquals("Gerrit Code Review", commitInfo.committer.name);
- assertTrue(commitInfo.parents.isEmpty());
- }
+ private TestRepository<Repository> repo;
- @Test
- public void getNonExistingCommit_NotFound() throws IOException {
- RestResponse r = adminSession.get("/projects/" + project.get() + "/commits/"
- + ObjectId.zeroId().name());
- assertEquals(HttpStatus.SC_NOT_FOUND, r.getStatusCode());
- }
+ @Before
+ public void setUp() throws Exception {
+ repo = new TestRepository<>(repoManager.openRepository(project));
- @Test
- public void getNonVisibleCommit_NotFound() throws IOException {
- RestResponse r =
- adminSession.get("/projects/" + project.get() + "/branches/"
- + IdString.fromDecoded(RefNames.REFS_CONFIG).encoded());
- assertEquals(HttpStatus.SC_OK, r.getStatusCode());
- BranchInfo branchInfo =
- newGson().fromJson(r.getReader(), BranchInfo.class);
- r.consume();
-
- ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
- cfg.getAccessSection("refs/*", false).removePermission(Permission.READ);
- saveProjectConfig(cfg);
- projectCache.evict(cfg.getProject());
-
- r = adminSession.get("/projects/" + project.get() + "/commits/"
- + branchInfo.revision);
- assertEquals(HttpStatus.SC_NOT_FOUND, r.getStatusCode());
- }
-
- private void saveProjectConfig(ProjectConfig cfg) throws IOException {
- MetaDataUpdate md = metaDataUpdateFactory.create(allProjects);
- try {
- cfg.commit(md);
- } finally {
- md.close();
+ ProjectConfig pc = projectCache.checkedGet(allProjects).getConfig();
+ for (AccessSection sec : pc.getAccessSections()) {
+ sec.removePermission(Permission.READ);
}
+ saveProjectConfig(allProjects, pc);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ if (repo != null) {
+ repo.getRepository().close();
+ }
+ }
+
+ @Test
+ public void getNonExistingCommit_NotFound() throws Exception {
+ assertNotFound(
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
+ }
+
+ @Test
+ public void getMergedCommit_Found() throws Exception {
+ allow(Permission.READ, REGISTERED_USERS, "refs/heads/*");
+ RevCommit commit = repo.parseBody(repo.branch("master")
+ .commit()
+ .message("Create\n\nNew commit\n")
+ .create());
+
+ CommitInfo info = getCommit(commit);
+ assertEquals(commit.name(), info.commit);
+ assertEquals("Create", info.subject);
+ assertEquals("Create\n\nNew commit\n", info.message);
+ assertEquals("J. Author", info.author.name);
+ assertEquals("jauthor@example.com", info.author.email);
+ assertEquals("J. Committer", info.committer.name);
+ assertEquals("jcommitter@example.com", info.committer.email);
+
+ CommitInfo parent = Iterables.getOnlyElement(info.parents);
+ assertEquals(commit.getParent(0).name(), parent.commit);
+ assertEquals("Initial empty repository", parent.subject);
+ assertNull(parent.message);
+ assertNull(parent.author);
+ assertNull(parent.committer);
+ }
+
+ @Test
+ public void getMergedCommit_NotFound() throws Exception {
+ RevCommit commit = repo.parseBody(repo.branch("master")
+ .commit()
+ .message("Create\n\nNew commit\n")
+ .create());
+ assertNotFound(commit);
+ }
+
+ @Test
+ public void getOpenChange_Found() throws Exception {
+ allow(Permission.READ, REGISTERED_USERS, "refs/heads/*");
+ PushOneCommit.Result r = pushFactory.create(db, admin.getIdent())
+ .to(git, "refs/for/master");
+ r.assertOkStatus();
+
+ CommitInfo info = getCommit(r.getCommitId());
+ assertEquals(r.getCommitId().name(), info.commit);
+ assertEquals("test commit", info.subject);
+ assertEquals("test commit\n\nChange-Id: " + r.getChangeId() + "\n",
+ info.message);
+ assertEquals("admin", info.author.name);
+ assertEquals("admin@example.com", info.author.email);
+ assertEquals("admin", info.committer.name);
+ assertEquals("admin@example.com", info.committer.email);
+
+ CommitInfo parent = Iterables.getOnlyElement(info.parents);
+ assertEquals(r.getCommit().getParent(0).name(), parent.commit);
+ assertEquals("Initial empty repository", parent.subject);
+ assertNull(parent.message);
+ assertNull(parent.author);
+ assertNull(parent.committer);
+ }
+
+ @Test
+ public void getOpenChange_NotFound() throws Exception {
+ PushOneCommit.Result r = pushFactory.create(db, admin.getIdent())
+ .to(git, "refs/for/master");
+ r.assertOkStatus();
+ assertNotFound(r.getCommitId());
+ }
+
+ private void assertNotFound(ObjectId id) throws Exception {
+ RestResponse r = userSession.get(
+ "/projects/" + project.get() + "/commits/" + id.name());
+ assertEquals(HttpStatus.SC_NOT_FOUND, r.getStatusCode());
+ }
+
+ private CommitInfo getCommit(ObjectId id) throws Exception {
+ RestResponse r = userSession.get(
+ "/projects/" + project.get() + "/commits/" + id.name());
+ assertEquals(HttpStatus.SC_OK, r.getStatusCode());
+ CommitInfo result = newGson().fromJson(r.getReader(), CommitInfo.class);
+ r.consume();
+ return result;
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java
index e6fde73..b51a4e2 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java
@@ -26,20 +26,14 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.project.ListBranches.BranchInfo;
-import com.google.gerrit.server.project.ProjectCache;
import com.google.gson.reflect.TypeToken;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.Inject;
import com.jcraft.jsch.JSchException;
import org.apache.http.HttpStatus;
import org.eclipse.jgit.api.errors.GitAPIException;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.junit.Test;
import java.io.IOException;
@@ -47,13 +41,6 @@
import java.util.List;
public class ListBranchesIT extends AbstractDaemonTest {
-
- @Inject
- private MetaDataUpdate.Server metaDataUpdateFactory;
-
- @Inject
- private ProjectCache projectCache;
-
@Test
public void listBranchesOfNonExistingProject_NotFound() throws IOException {
assertEquals(HttpStatus.SC_NOT_FOUND,
@@ -61,8 +48,7 @@
}
@Test
- public void listBranchesOfNonVisibleProject_NotFound() throws IOException,
- OrmException, JSchException, ConfigInvalidException {
+ public void listBranchesOfNonVisibleProject_NotFound() throws Exception {
blockRead(project, "refs/*");
assertEquals(HttpStatus.SC_NOT_FOUND,
userSession.get("/projects/" + project.get() + "/branches").getStatusCode());
@@ -106,8 +92,7 @@
}
@Test
- public void listBranchesSomeHidden() throws IOException, GitAPIException,
- ConfigInvalidException, OrmException, JSchException {
+ public void listBranchesSomeHidden() throws Exception {
blockRead(project, "refs/heads/dev");
pushTo("refs/heads/master");
String masterCommit = git.getRepository().getRef("master").getTarget().getObjectId().getName();
@@ -123,8 +108,7 @@
}
@Test
- public void listBranchesHeadHidden() throws IOException, GitAPIException,
- ConfigInvalidException, OrmException, JSchException {
+ public void listBranchesHeadHidden() throws Exception {
blockRead(project, "refs/heads/master");
pushTo("refs/heads/master");
pushTo("refs/heads/dev");
@@ -139,12 +123,10 @@
return adminSession.get(endpoint);
}
- private void blockRead(Project.NameKey project, String ref)
- throws RepositoryNotFoundException, IOException, ConfigInvalidException {
+ private void blockRead(Project.NameKey project, String ref) throws Exception {
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
block(cfg, Permission.READ, REGISTERED_USERS, ref);
saveProjectConfig(project, cfg);
- projectCache.evict(cfg.getProject());
}
private static List<BranchInfo> toBranchInfoList(RestResponse r)
@@ -160,13 +142,4 @@
PushOneCommit push = pushFactory.create(db, admin.getIdent());
return push.to(git, ref);
}
-
- private void saveProjectConfig(Project.NameKey p, ProjectConfig cfg) throws IOException {
- MetaDataUpdate md = metaDataUpdateFactory.create(p);
- try {
- cfg.commit(md);
- } finally {
- md.close();
- }
- }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
index 923d752..bb3d2e7 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
@@ -121,6 +121,11 @@
Project.NameKey projectAwesome = new Project.NameKey("project-awesome");
createProject(sshSession, projectAwesome.get());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?p=some&r=.*").getStatusCode());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?p=some&m=some").getStatusCode());
+
RestResponse r = GET("/projects/?p=some");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
Map<String, ProjectInfo> result = toProjectInfoMap(r);
@@ -138,13 +143,22 @@
Project.NameKey projectAwesome = new Project.NameKey("project-awesome");
createProject(sshSession, projectAwesome.get());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?r=[.*some").getStatusCode());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?r=.*&p=s").getStatusCode());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?r=.*&m=s").getStatusCode());
+
RestResponse r = GET("/projects/?r=.*some");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
Map<String, ProjectInfo> result = toProjectInfoMap(r);
assertProjects(Arrays.asList(projectAwesome), result.values());
- r = GET("/projects/?r=[.*some");
- assertEquals(HttpStatus.SC_BAD_REQUEST, r.getStatusCode());
+ r = GET("/projects/?r=some-project$");
+ assertEquals(HttpStatus.SC_OK, r.getStatusCode());
+ result = toProjectInfoMap(r);
+ assertProjects(Arrays.asList(someProject), result.values());
r = GET("/projects/?r=.*");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
@@ -181,6 +195,11 @@
Project.NameKey projectAwesome = new Project.NameKey("project-awesome");
createProject(sshSession, projectAwesome.get());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?m=some&r=.*").getStatusCode());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?m=some&p=some").getStatusCode());
+
RestResponse r = GET("/projects/?m=some");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
Map<String, ProjectInfo> result = toProjectInfoMap(r);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java
index fb830e4..3b790b1 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java
@@ -16,36 +16,26 @@
import static com.google.gerrit.acceptance.GitUtil.checkout;
import static com.google.gerrit.acceptance.GitUtil.cloneProject;
-import static com.google.gerrit.acceptance.GitUtil.createProject;
import static com.google.gerrit.acceptance.GitUtil.fetch;
import static org.junit.Assert.assertEquals;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.SshSession;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.AllProjectsNameProvider;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Config;
-import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import java.io.IOException;
public class ProjectLevelConfigIT extends AbstractDaemonTest {
-
- @Inject
- private SchemaFactory<ReviewDb> reviewDbProvider;
-
@Inject
private ProjectCache projectCache;
@@ -55,27 +45,10 @@
@Inject
private PushOneCommit.Factory pushFactory;
- private ReviewDb db;
- private SshSession sshSession;
- private String project;
- private Git git;
-
@Before
public void setUp() throws Exception {
- sshSession = new SshSession(server, admin);
-
- project = "p";
- createProject(sshSession, project, null, true);
- git = cloneProject(sshSession.getUrl() + "/" + project);
fetch(git, RefNames.REFS_CONFIG + ":refs/heads/config");
checkout(git, "refs/heads/config");
-
- db = reviewDbProvider.open();
- }
-
- @After
- public void cleanup() {
- db.close();
}
@Test
@@ -89,13 +62,13 @@
configName, cfg.toText());
push.to(git, RefNames.REFS_CONFIG);
- ProjectState state = projectCache.get(new Project.NameKey(project));
+ ProjectState state = projectCache.get(project);
assertEquals(cfg.toText(), state.getConfig(configName).get().toText());
}
@Test
public void nonExistingConfig() {
- ProjectState state = projectCache.get(new Project.NameKey(project));
+ ProjectState state = projectCache.get(project);
assertEquals("", state.getConfig("test.config").get().toText());
}
@@ -125,7 +98,7 @@
configName, cfg.toText());
push.to(git, RefNames.REFS_CONFIG);
- ProjectState state = projectCache.get(new Project.NameKey(project));
+ ProjectState state = projectCache.get(project);
Config expectedCfg = new Config();
expectedCfg.setString("s1", null, "k1", "childValue1");
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java
new file mode 100644
index 0000000..9de5220
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -0,0 +1,198 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.server.change;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.common.Comment;
+import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.testutil.ConfigSuite;
+import com.google.gson.reflect.TypeToken;
+
+import org.apache.http.HttpStatus;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.lang.reflect.Type;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class CommentsIT extends AbstractDaemonTest {
+ @ConfigSuite.Config
+ public static Config noteDbEnabled() {
+ Config cfg = new Config();
+ cfg.setBoolean("notedb", null, "write", true);
+ cfg.setBoolean("notedb", "comments", "read", true);
+ return cfg;
+ }
+
+ @Test
+ public void createDraft() throws GitAPIException, IOException {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String revId = r.getCommit().getName();
+ ReviewInput.CommentInput comment = newCommentInfo(
+ "file1", Comment.Side.REVISION, 1, "comment 1");
+ addDraft(changeId, revId, comment);
+ Map<String, List<CommentInfo>> result = getDraftComments(changeId, revId);
+ assertEquals(1, result.size());
+ CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
+ assertCommentInfo(comment, actual);
+ }
+
+ @Test
+ public void postComment() throws RestApiException, Exception {
+ String file = "file";
+ String contents = "contents";
+ PushOneCommit push = pushFactory.create(db, admin.getIdent(),
+ "first subject", file, contents);
+ PushOneCommit.Result r = push.to(git, "refs/for/master");
+ String changeId = r.getChangeId();
+ String revId = r.getCommit().getName();
+ ReviewInput input = new ReviewInput();
+ ReviewInput.CommentInput comment = newCommentInfo(
+ file, Comment.Side.REVISION, 1, "comment 1");
+ input.comments = new HashMap<String, List<ReviewInput.CommentInput>>();
+ input.comments.put(comment.path, Lists.newArrayList(comment));
+ revision(r).review(input);
+ Map<String, List<CommentInfo>> result = getPublishedComments(changeId, revId);
+ assertTrue(!result.isEmpty());
+ CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
+ assertCommentInfo(comment, actual);
+ }
+
+ @Test
+ public void putDraft() throws GitAPIException, IOException {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String revId = r.getCommit().getName();
+ ReviewInput.CommentInput comment = newCommentInfo(
+ "file1", Comment.Side.REVISION, 1, "comment 1");
+ addDraft(changeId, revId, comment);
+ Map<String, List<CommentInfo>> result = getDraftComments(changeId, revId);
+ CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
+ assertCommentInfo(comment, actual);
+ String uuid = actual.id;
+ comment.message = "updated comment 1";
+ updateDraft(changeId, revId, comment, uuid);
+ result = getDraftComments(changeId, revId);
+ actual = Iterables.getOnlyElement(result.get(comment.path));
+ assertCommentInfo(comment, actual);
+ }
+
+ @Test
+ public void getDraft() throws GitAPIException, IOException {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String revId = r.getCommit().getName();
+ ReviewInput.CommentInput comment = newCommentInfo(
+ "file1", Comment.Side.REVISION, 1, "comment 1");
+ CommentInfo returned = addDraft(changeId, revId, comment);
+ CommentInfo actual = getDraftComment(changeId, revId, returned.id);
+ assertCommentInfo(comment, actual);
+ }
+
+ @Test
+ public void deleteDraft() throws IOException, GitAPIException {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String revId = r.getCommit().getName();
+ ReviewInput.CommentInput comment = newCommentInfo(
+ "file1", Comment.Side.REVISION, 1, "comment 1");
+ CommentInfo returned = addDraft(changeId, revId, comment);
+ deleteDraft(changeId, revId, returned.id);
+ Map<String, List<CommentInfo>> drafts = getDraftComments(changeId, revId);
+ assertTrue(drafts.isEmpty());
+ }
+
+ private CommentInfo addDraft(String changeId, String revId,
+ ReviewInput.CommentInput c) throws IOException {
+ RestResponse r = userSession.put(
+ "/changes/" + changeId + "/revisions/" + revId + "/drafts", c);
+ assertEquals(HttpStatus.SC_CREATED, r.getStatusCode());
+ return newGson().fromJson(r.getReader(), CommentInfo.class);
+ }
+
+ private void updateDraft(String changeId, String revId,
+ ReviewInput.CommentInput c, String uuid) throws IOException {
+ RestResponse r = userSession.put(
+ "/changes/" + changeId + "/revisions/" + revId + "/drafts/" + uuid, c);
+ assertEquals(HttpStatus.SC_OK, r.getStatusCode());
+ }
+
+ private void deleteDraft(String changeId, String revId, String uuid)
+ throws IOException {
+ RestResponse r = userSession.delete(
+ "/changes/" + changeId + "/revisions/" + revId + "/drafts/" + uuid);
+ assertEquals(HttpStatus.SC_NO_CONTENT, r.getStatusCode());
+ }
+
+ private Map<String, List<CommentInfo>> getPublishedComments(String changeId,
+ String revId) throws IOException {
+ RestResponse r = userSession.get(
+ "/changes/" + changeId + "/revisions/" + revId + "/comments/");
+ assertEquals(HttpStatus.SC_OK, r.getStatusCode());
+ Type mapType = new TypeToken<Map<String, List<CommentInfo>>>() {}.getType();
+ return newGson().fromJson(r.getReader(), mapType);
+ }
+
+ private Map<String, List<CommentInfo>> getDraftComments(String changeId,
+ String revId) throws IOException {
+ RestResponse r = userSession.get(
+ "/changes/" + changeId + "/revisions/" + revId + "/drafts/");
+ assertEquals(HttpStatus.SC_OK, r.getStatusCode());
+ Type mapType = new TypeToken<Map<String, List<CommentInfo>>>() {}.getType();
+ return newGson().fromJson(r.getReader(), mapType);
+ }
+
+ private CommentInfo getDraftComment(String changeId, String revId,
+ String uuid) throws IOException {
+ RestResponse r = userSession.get(
+ "/changes/" + changeId + "/revisions/" + revId + "/drafts/" + uuid);
+ assertEquals(HttpStatus.SC_OK, r.getStatusCode());
+ return newGson().fromJson(r.getReader(), CommentInfo.class);
+ }
+
+ private static void assertCommentInfo(ReviewInput.CommentInput expected,
+ CommentInfo actual) {
+ assertEquals(expected.line, actual.line);
+ assertEquals(expected.message, actual.message);
+ assertEquals(expected.inReplyTo, actual.inReplyTo);
+ if (actual.side == null) {
+ assertEquals(expected.side, Comment.Side.REVISION);
+ }
+ }
+
+ private ReviewInput.CommentInput newCommentInfo(String path,
+ Comment.Side side, int line, String message) {
+ ReviewInput.CommentInput input = new ReviewInput.CommentInput();
+ input.path = path;
+ input.side = side;
+ input.line = line;
+ input.message = message;
+ return input;
+ }
+}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
index 2a20a2c..612a32f 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.acceptance.server.project;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
-import static com.google.gerrit.server.project.Util.allow;
import static com.google.gerrit.server.project.Util.category;
import static com.google.gerrit.server.project.Util.value;
import static org.junit.Assert.assertEquals;
@@ -37,6 +36,7 @@
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.Util;
import com.google.inject.Inject;
import org.junit.Before;
@@ -64,7 +64,7 @@
ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
AccountGroup.UUID anonymousUsers =
SystemGroupBackend.getGroup(ANONYMOUS_USERS).getUUID();
- allow(cfg, Permission.forLabel(Q.getName()), -1, 1, anonymousUsers,
+ Util.allow(cfg, Permission.forLabel(Q.getName()), -1, 1, anonymousUsers,
"refs/heads/*");
saveProjectConfig(cfg);
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java
index bfce523..1bd1b71 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java
@@ -42,7 +42,7 @@
String response =
sshSession.exec("gerrit ban-commit " + project.get() + " "
+ c.getCommit().getName());
- assertFalse(sshSession.hasError());
+ assertFalse(sshSession.getError(), sshSession.hasError());
assertFalse(response, response.toLowerCase(Locale.US).contains("error"));
PushResult pushResult = pushHead(git, "refs/heads/master", false);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/GarbageCollectionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/GarbageCollectionIT.java
index 9f859dc..d32a58f 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/GarbageCollectionIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/GarbageCollectionIT.java
@@ -76,7 +76,7 @@
String response =
sshSession.exec("gerrit gc \"" + project1.get() + "\" \""
+ project2.get() + "\"");
- assertFalse(sshSession.hasError());
+ assertFalse(sshSession.getError(), sshSession.hasError());
assertNoError(response);
gcAssert.assertHasPackFile(project1, project2);
gcAssert.assertHasNoPackFile(allProjects, project3);
@@ -86,7 +86,7 @@
@UseLocalDisk
public void testGcAll() throws JSchException, IOException {
String response = sshSession.exec("gerrit gc --all");
- assertFalse(sshSession.hasError());
+ assertFalse(sshSession.getError(), sshSession.hasError());
assertNoError(response);
gcAssert.assertHasPackFile(allProjects, project1, project2, project3);
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/GerritGwtUI.gwt.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/GerritGwtUI.gwt.xml
index b8102ec..fd717ee 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/GerritGwtUI.gwt.xml
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/GerritGwtUI.gwt.xml
@@ -39,7 +39,6 @@
<add-linker name='xsiframe'/>
<set-property name='gwt.logging.logLevel' value='SEVERE'/>
- <set-property name='gwt.logging.popupHandler' value='DISABLED'/>
<entry-point class='com.google.gerrit.client.Gerrit'/>
</module>
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProxyProperties.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProxyProperties.java
new file mode 100644
index 0000000..67b97c4
--- /dev/null
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProxyProperties.java
@@ -0,0 +1,23 @@
+// Copyright (C) 2014 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.httpd;
+
+import java.net.URL;
+
+public interface ProxyProperties {
+ URL getProxyUrl();
+ String getUsername();
+ String getPassword();
+}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProxyPropertiesProvider.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProxyPropertiesProvider.java
new file mode 100644
index 0000000..0e51cc2
--- /dev/null
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProxyPropertiesProvider.java
@@ -0,0 +1,73 @@
+// Copyright (C) 2014 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.httpd;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+import org.eclipse.jgit.lib.Config;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+
+@Singleton
+class ProxyPropertiesProvider implements Provider<ProxyProperties> {
+
+ private URL proxyUrl;
+ private String proxyUser;
+ private String proxyPassword;
+
+ @Inject
+ ProxyPropertiesProvider(@GerritServerConfig Config config)
+ throws MalformedURLException {
+ String proxyUrlStr = config.getString("http", null, "proxy");
+ if (!Strings.isNullOrEmpty(proxyUrlStr)) {
+ proxyUrl = new URL(proxyUrlStr);
+ proxyUser = config.getString("http", null, "proxyUsername");
+ proxyPassword = config.getString("http", null, "proxyPassword");
+ String userInfo = proxyUrl.getUserInfo();
+ if (userInfo != null) {
+ int c = userInfo.indexOf(':');
+ if (0 < c) {
+ proxyUser = userInfo.substring(0, c);
+ proxyPassword = userInfo.substring(c + 1);
+ } else {
+ proxyUser = userInfo;
+ }
+ }
+ }
+ }
+
+ @Override
+ public ProxyProperties get() {
+ return new ProxyProperties() {
+ @Override
+ public URL getProxyUrl() {
+ return proxyUrl;
+ }
+ @Override
+ public String getUsername() {
+ return proxyUser;
+ }
+ @Override
+ public String getPassword() {
+ return proxyPassword;
+ }
+ };
+ }
+}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebModule.java
index 3443968..e2adcd5 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebModule.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebModule.java
@@ -131,6 +131,8 @@
bind(SocketAddress.class).annotatedWith(RemotePeer.class).toProvider(
HttpRemotePeerProvider.class).in(RequestScoped.class);
+ bind(ProxyProperties.class).toProvider(ProxyPropertiesProvider.class);
+
listener().toInstance(registerInParentInjectors());
}
}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java
index 73cc83a..4a673cb 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java
@@ -170,7 +170,8 @@
// quickly locate where they have pending drafts, and review them.
//
final Account.Id me = ((IdentifiedUser) user).getAccountId();
- for (final PatchLineComment c : db.patchComments().draftByPatchSetAuthor(psIdNew, me)) {
+ for (PatchLineComment c
+ : plcUtil.draftByPatchSetAuthor(db, psIdNew, me, notes)) {
final Patch p = byKey.get(c.getKey().getParentKey());
if (p != null) {
p.setDraftCount(p.getDraftCount() + 1);
diff --git a/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java b/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
index 1681bc2..dc5e102 100644
--- a/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
+++ b/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.httpd.CanonicalWebUrl;
+import com.google.gerrit.httpd.ProxyProperties;
import com.google.gerrit.httpd.WebSession;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.IdentifiedUser;
@@ -55,7 +56,6 @@
import org.openid4java.message.sreg.SRegRequest;
import org.openid4java.message.sreg.SRegResponse;
import org.openid4java.util.HttpClientFactory;
-import org.openid4java.util.ProxyProperties;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -108,29 +108,18 @@
final Provider<IdentifiedUser> iu,
CanonicalWebUrl up,
@GerritServerConfig final Config config, final AuthConfig ac,
- final AccountManager am) throws ConsumerException, MalformedURLException {
+ final AccountManager am,
+ ProxyProperties proxyProperties)
+ throws ConsumerException, MalformedURLException {
- if (config.getString("http", null, "proxy") != null) {
- final URL proxyUrl = new URL(config.getString("http", null, "proxy"));
- String username = config.getString("http", null, "proxyUsername");
- String password = config.getString("http", null, "proxyPassword");
-
- final String userInfo = proxyUrl.getUserInfo();
- if (userInfo != null) {
- int c = userInfo.indexOf(':');
- if (0 < c) {
- username = userInfo.substring(0, c);
- password = userInfo.substring(c + 1);
- } else {
- username = userInfo;
- }
- }
-
- final ProxyProperties proxy = new ProxyProperties();
- proxy.setProxyHostName(proxyUrl.getHost());
- proxy.setProxyPort(proxyUrl.getPort());
- proxy.setUserName(username);
- proxy.setPassword(password);
+ if (proxyProperties.getProxyUrl() != null) {
+ final org.openid4java.util.ProxyProperties proxy =
+ new org.openid4java.util.ProxyProperties();
+ URL url = proxyProperties.getProxyUrl();
+ proxy.setProxyHostName(url.getHost());
+ proxy.setProxyPort(url.getPort());
+ proxy.setUserName(proxyProperties.getUsername());
+ proxy.setPassword(proxyProperties.getPassword());
HttpClientFactory.setProxyProperties(proxy);
}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 4a17e68..ec98465 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -15,6 +15,7 @@
package com.google.gerrit.pgm.util;
import static com.google.inject.Scopes.SINGLETON;
+
import com.google.common.cache.Cache;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -32,6 +33,8 @@
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.FactoryModule;
+import com.google.gerrit.server.git.ChangeCache;
+import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.group.GroupModule;
import com.google.gerrit.server.patch.PatchListCacheImpl;
import com.google.gerrit.server.project.AccessControlModule;
@@ -92,6 +95,8 @@
install(ProjectCacheImpl.module());
install(SectionSortCache.module());
install(ChangeKindCacheImpl.module());
+ install(ChangeCache.module());
+ install(TagCache.module());
factory(CapabilityControl.Factory.class);
factory(ChangeData.Factory.class);
factory(ProjectState.Factory.class);
diff --git a/gerrit-reviewdb/BUCK b/gerrit-reviewdb/BUCK
index faf80a8..9b1991b 100644
--- a/gerrit-reviewdb/BUCK
+++ b/gerrit-reviewdb/BUCK
@@ -1,4 +1,5 @@
SRC = 'src/main/java/com/google/gerrit/reviewdb/'
+TESTS = 'src/test/java/com/google/gerrit/reviewdb/'
gwt_module(
name = 'client',
@@ -22,3 +23,15 @@
],
visibility = ['PUBLIC'],
)
+
+java_test(
+ name = 'client_tests',
+ srcs = glob([TESTS + 'client/**/*.java']),
+ deps = [
+ ':client',
+ '//lib:gwtorm',
+ '//lib:junit',
+ ],
+ source_under_test = [':client'],
+ visibility = ['//tools/eclipse:classpath'],
+)
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java
index e131f7a..3d156f9 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java
@@ -104,6 +104,55 @@
r.fromString(str);
return r;
}
+
+ /**
+ * Parse an Account.Id out of a part of a ref-name.
+ *
+ * @param name a ref name with the following syntax: {@code "34/1234..."}.
+ * We assume that the caller has trimmed any prefix.
+ */
+ public static Id fromRefPart(String name) {
+ if (name == null) {
+ return null;
+ }
+
+ String[] parts = name.split("/");
+ int n = parts.length;
+ if (n < 2) {
+ return null;
+ }
+
+ // Last 2 digits.
+ int le;
+ for (le = 0; le < parts[0].length(); le++) {
+ if (!Character.isDigit(parts[0].charAt(le))) {
+ return null;
+ }
+ }
+ if (le != 2) {
+ return null;
+ }
+
+ // Full ID.
+ int ie;
+ for (ie = 0; ie < parts[1].length(); ie++) {
+ if (!Character.isDigit(parts[1].charAt(ie))) {
+ if (ie == 0) {
+ return null;
+ } else {
+ break;
+ }
+ }
+ }
+
+ int shard = Integer.parseInt(parts[0]);
+ int id = Integer.parseInt(parts[1].substring(0, ie));
+
+ if (id % 100 != shard) {
+ return null;
+ }
+ return new Account.Id(id);
+ }
}
@Column(id = 1)
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java
index 613978a..ed4d999 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java
@@ -24,28 +24,8 @@
/** A single revision of a {@link Change}. */
public final class PatchSet {
/** Is the reference name a change reference? */
- public static boolean isRef(final String name) {
- if (name == null || !name.startsWith(REFS_CHANGES)) {
- return false;
- }
- boolean accepted = false;
- int numsFound = 0;
- for (int i = name.length() - 1; i >= REFS_CHANGES.length() - 1; i--) {
- char c = name.charAt(i);
- if (c >= '0' && c <= '9') {
- accepted = (c != '0');
- } else if (c == '/') {
- if (accepted) {
- if (++numsFound == 2) {
- return true;
- }
- accepted = false;
- }
- } else {
- return false;
- }
- }
- return false;
+ public static boolean isRef(String name) {
+ return Id.fromRef(name) != null;
}
public static class Id extends IntKey<Change.Id> {
@@ -106,16 +86,63 @@
/** Parse a PatchSet.Id from a {@link PatchSet#getRefName()} result. */
public static Id fromRef(String name) {
- if (!name.startsWith(REFS_CHANGES)) {
- throw new IllegalArgumentException("Not a PatchSet.Id: " + name);
+ if (name == null || !name.startsWith(REFS_CHANGES)) {
+ return null;
}
- final String[] parts = name.substring(REFS_CHANGES.length()).split("/");
- final int n = parts.length;
- if (n < 2) {
- throw new IllegalArgumentException("Not a PatchSet.Id: " + name);
+
+ // Last 2 digits.
+ int ls = REFS_CHANGES.length();
+ int le;
+ for (le = ls; le < name.length() && name.charAt(le) != '/'; le++) {
+ if (name.charAt(le) < '0' || name.charAt(le) > '9') {
+ return null;
+ }
}
- final int changeId = Integer.parseInt(parts[n - 2]);
- final int patchSetId = Integer.parseInt(parts[n - 1]);
+ if (le - ls != 2) {
+ return null;
+ }
+
+ // Change ID.
+ int cs = le + 1;
+ if (cs >= name.length() || name.charAt(cs) == '0') {
+ return null;
+ }
+ int ce;
+ for (ce = cs; ce < name.length() && name.charAt(ce) != '/'; ce++) {
+ if (name.charAt(ce) < '0' || name.charAt(ce) > '9') {
+ return null;
+ }
+ }
+ switch (ce - cs) {
+ case 0:
+ return null;
+ case 1:
+ if (name.charAt(ls) != '0'
+ || name.charAt(ls + 1) != name.charAt(cs)) {
+ return null;
+ }
+ break;
+ default:
+ if (name.charAt(ls) != name.charAt(ce - 2)
+ || name.charAt(ls + 1) != name.charAt(ce - 1)) {
+ return null;
+ }
+ break;
+ }
+
+ // Patch set ID.
+ int ps = ce + 1;
+ if (ps >= name.length() || name.charAt(ps) == '0') {
+ return null;
+ }
+ for (int i = ps; i < name.length(); i++) {
+ if (name.charAt(i) < '0' || name.charAt(i) > '9') {
+ return null;
+ }
+ }
+
+ int changeId = Integer.parseInt(name.substring(cs, ce));
+ int patchSetId = Integer.parseInt(name.substring(ps));
return new PatchSet.Id(new Change.Id(changeId), patchSetId);
}
}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
index 06e1803..390295f 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
@@ -31,6 +31,8 @@
/** Configurations of project-specific dashboards (canned search queries). */
public static final String REFS_DASHBOARDS = "refs/meta/dashboards/";
+ public static final String REFS_DRAFT_COMMENTS = "refs/draft-comments/";
+
/**
* Prefix applied to merge commit base nodes.
* <p>
@@ -43,8 +45,6 @@
*/
public static final String REFS_CACHE_AUTOMERGE = "refs/cache-automerge/";
- public static final String REFS_DRAFT_PREFIX = "comments-";
-
public static String refsUsers(Account.Id accountId) {
StringBuilder r = new StringBuilder();
r.append(REFS_USER);
@@ -62,9 +62,15 @@
public static String refsDraftComments(Account.Id accountId,
Change.Id changeId) {
StringBuilder r = new StringBuilder();
- r.append(refsUsers(accountId));
+ r.append(REFS_DRAFT_COMMENTS);
+ int n = accountId.get() % 100;
+ if (n < 10) {
+ r.append('0');
+ }
+ r.append(n);
r.append('/');
- r.append(REFS_DRAFT_PREFIX);
+ r.append(accountId.get());
+ r.append('-');
r.append(changeId.get());
return r.toString();
}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/PatchSetAncestorAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/PatchSetAncestorAccess.java
index ac2c849..47d8971 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/PatchSetAncestorAccess.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/PatchSetAncestorAccess.java
@@ -14,7 +14,7 @@
package com.google.gerrit.reviewdb.server;
-import com.google.gerrit.reviewdb.client.Change.Id;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetAncestor;
import com.google.gerrit.reviewdb.client.RevId;
@@ -33,7 +33,7 @@
ResultSet<PatchSetAncestor> ancestorsOf(PatchSet.Id id) throws OrmException;
@Query("WHERE key.patchSetId.changeId = ?")
- ResultSet<PatchSetAncestor> byChange(Id id) throws OrmException;
+ ResultSet<PatchSetAncestor> byChange(Change.Id id) throws OrmException;
@Query("WHERE key.patchSetId = ?")
ResultSet<PatchSetAncestor> byPatchSet(PatchSet.Id id) throws OrmException;
diff --git a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/AccountTest.java b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/AccountTest.java
new file mode 100644
index 0000000..3ffdb92
--- /dev/null
+++ b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/AccountTest.java
@@ -0,0 +1,55 @@
+// Copyright (C) 2014 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.client;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import com.google.gerrit.reviewdb.client.Account;
+
+import org.junit.Test;
+
+public class AccountTest {
+ @Test
+ public void parseRefNameParts() {
+ assertRefPart(1, "01/1");
+ assertRefPart(1, "01/1-drafts");
+ assertRefPart(1, "01/1-drafts/2");
+
+ assertNotRefPart(null);
+ assertNotRefPart("");
+
+ // This method assumes that the common prefix "refs/users/" will be removed.
+ assertNotRefPart("refs/users/01/1");
+
+ // Invalid characters.
+ assertNotRefPart("01a/1");
+ assertNotRefPart("01/a1");
+
+ // Mismatched shard.
+ assertNotRefPart("01/23");
+
+ // Shard too short.
+ assertNotRefPart("1/1");
+ }
+
+ private static void assertRefPart(int accountId, String refName) {
+ assertEquals(new Account.Id(accountId), Account.Id.fromRefPart(refName));
+ }
+
+ private static void assertNotRefPart(String refName) {
+ assertNull(Account.Id.fromRefPart(refName));
+ }
+}
diff --git a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/PatchSetTest.java b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/PatchSetTest.java
new file mode 100644
index 0000000..525fb18
--- /dev/null
+++ b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/PatchSetTest.java
@@ -0,0 +1,69 @@
+// Copyright (C) 2014 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.client;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+
+public class PatchSetTest {
+ @Test
+ public void parseRefNames() {
+ assertRef(1, 1, "refs/changes/01/1/1");
+ assertRef(1234, 56, "refs/changes/34/1234/56");
+
+ // Not even close.
+ assertNotRef(null);
+ assertNotRef("");
+ assertNotRef("01/1/1");
+ assertNotRef("HEAD");
+ assertNotRef("refs/tags/v1");
+
+ // Invalid characters.
+ assertNotRef("refs/changes/0x/1/1");
+ assertNotRef("refs/changes/01/x/1");
+ assertNotRef("refs/changes/01/1/x");
+
+ // Truncations.
+ assertNotRef("refs/changes/");
+ assertNotRef("refs/changes/1");
+ assertNotRef("refs/changes/01");
+ assertNotRef("refs/changes/01/");
+ assertNotRef("refs/changes/01/1/");
+ assertNotRef("refs/changes/01/1/1/");
+ assertNotRef("refs/changes/01//1/1");
+
+ // Leading zeroes.
+ assertNotRef("refs/changes/01/01/1");
+ assertNotRef("refs/changes/01/1/01");
+
+ // Mismatched last 2 digits.
+ assertNotRef("refs/changes/35/1234/56");
+ }
+
+ private static void assertRef(int changeId, int psId, String refName) {
+ assertTrue(PatchSet.isRef(refName));
+ assertEquals(new PatchSet.Id(new Change.Id(changeId), psId),
+ PatchSet.Id.fromRef(refName));
+ }
+
+ private static void assertNotRef(String refName) {
+ assertFalse(PatchSet.isRef(refName));
+ assertNull(PatchSet.Id.fromRef(refName));
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/audit/AuditService.java b/gerrit-server/src/main/java/com/google/gerrit/audit/AuditService.java
index baafb89..4844045 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/audit/AuditService.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/audit/AuditService.java
@@ -24,7 +24,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.IOException;
import java.util.Collection;
@Singleton
diff --git a/gerrit-server/src/main/java/com/google/gerrit/audit/GroupMemberAuditListener.java b/gerrit-server/src/main/java/com/google/gerrit/audit/GroupMemberAuditListener.java
index edf2392..1269f4a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/audit/GroupMemberAuditListener.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/audit/GroupMemberAuditListener.java
@@ -16,7 +16,6 @@
import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Account.Id;
import com.google.gerrit.reviewdb.client.AccountGroupById;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
@@ -31,7 +30,8 @@
void onDeleteAccountsFromGroup(Account.Id actor,
Collection<AccountGroupMember> removed);
- void onAddGroupsToGroup(Id actor, Collection<AccountGroupById> added);
+ void onAddGroupsToGroup(Account.Id actor, Collection<AccountGroupById> added);
- void onDeleteGroupsFromGroup(Id actor, Collection<AccountGroupById> deleted);
+ void onDeleteGroupsFromGroup(Account.Id actor,
+ Collection<AccountGroupById> deleted);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
index 80213f6..2ed37cb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
@@ -508,8 +508,8 @@
ReviewDb db = this.db.get();
db.accountPatchReviews().delete(db.accountPatchReviews().byPatchSet(patchSetId));
db.changeMessages().delete(db.changeMessages().byPatchSet(patchSetId));
- db.patchComments().delete(db.patchComments().byPatchSet(patchSetId));
// No need to delete from notedb; draft patch sets will be filtered out.
+ db.patchComments().delete(db.patchComments().byPatchSet(patchSetId));
db.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(patchSetId));
db.patchSetAncestors().delete(db.patchSetAncestors().byPatchSet(patchSetId));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/CommonConverters.java b/gerrit-server/src/main/java/com/google/gerrit/server/CommonConverters.java
new file mode 100644
index 0000000..09c2ea6
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/CommonConverters.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2014 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;
+
+import com.google.gerrit.extensions.common.GitPerson;
+
+import org.eclipse.jgit.lib.PersonIdent;
+
+import java.sql.Timestamp;
+
+/**
+ * Converters to classes in {@link com.google.gerrit.extensions.common}.
+ * <p>
+ * The server frequently needs to convert internal types to types exposed in the
+ * extension API, but the converters themselves are not part of this API. This
+ * class contains such converters as static utility methods.
+ */
+public class CommonConverters {
+ public static GitPerson toGitPerson(PersonIdent ident) {
+ GitPerson result = new GitPerson();
+ result.name = ident.getName();
+ result.email = ident.getEmailAddress();
+ result.date = new Timestamp(ident.getWhen().getTime());
+ result.tz = ident.getTimeZoneOffset();
+ return result;
+ }
+
+ private CommonConverters() {
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/PatchLineCommentsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/PatchLineCommentsUtil.java
index f917613..d12d3cc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/PatchLineCommentsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/PatchLineCommentsUtil.java
@@ -12,25 +12,46 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-
package com.google.gerrit.server;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
+import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.AllUsersNameProvider;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.DraftCommentNotes;
import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.patch.PatchList;
+import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.RefDatabase;
+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;
+import java.util.Set;
/**
* Utility functions to manipulate PatchLineComments.
@@ -40,45 +61,208 @@
*/
@Singleton
public class PatchLineCommentsUtil {
+ private final GitRepositoryManager repoManager;
+ private final AllUsersName allUsers;
+ private final DraftCommentNotes.Factory draftFactory;
private final NotesMigration migration;
@VisibleForTesting
@Inject
- public PatchLineCommentsUtil(NotesMigration migration) {
+ public PatchLineCommentsUtil(GitRepositoryManager repoManager,
+ AllUsersNameProvider allUsersProvider,
+ DraftCommentNotes.Factory draftFactory,
+ NotesMigration migration) {
+ this.repoManager = repoManager;
+ this.allUsers = allUsersProvider.get();
+ this.draftFactory = draftFactory;
this.migration = migration;
}
+ public Optional<PatchLineComment> get(ReviewDb db, ChangeNotes notes,
+ PatchLineComment.Key key) throws OrmException {
+ if (!migration.readComments()) {
+ return Optional.fromNullable(db.patchComments().get(key));
+ }
+ for (PatchLineComment c : publishedByChange(db, notes)) {
+ if (key.equals(c.getKey())) {
+ return Optional.of(c);
+ }
+ }
+ for (PatchLineComment c : draftByChange(db, notes)) {
+ if (key.equals(c.getKey())) {
+ return Optional.of(c);
+ }
+ }
+ return Optional.absent();
+ }
+
+ public List<PatchLineComment> publishedByChange(ReviewDb db,
+ ChangeNotes notes) throws OrmException {
+ if (!migration.readComments()) {
+ return byCommentStatus(db.patchComments().byChange(notes.getChangeId()),
+ Status.PUBLISHED);
+ }
+
+ notes.load();
+ List<PatchLineComment> comments = Lists.newArrayList();
+ comments.addAll(notes.getBaseComments().values());
+ comments.addAll(notes.getPatchSetComments().values());
+ return comments;
+ }
+
+ public List<PatchLineComment> draftByChange(ReviewDb db,
+ ChangeNotes notes) throws OrmException {
+ if (!migration.readComments()) {
+ return byCommentStatus(db.patchComments().byChange(notes.getChangeId()),
+ Status.DRAFT);
+ }
+
+ List<PatchLineComment> comments = Lists.newArrayList();
+ Iterable<String> filtered = getDraftRefs(notes.getChangeId());
+ for (String refName : filtered) {
+ Account.Id account = Account.Id.fromRefPart(refName);
+ if (account != null) {
+ comments.addAll(draftByChangeAuthor(db, notes, account));
+ }
+ }
+ return comments;
+ }
+
+ private static List<PatchLineComment> byCommentStatus(
+ ResultSet<PatchLineComment> comments,
+ final PatchLineComment.Status status) {
+ return Lists.newArrayList(
+ Iterables.filter(comments, new Predicate<PatchLineComment>() {
+ @Override
+ public boolean apply(PatchLineComment input) {
+ return (input.getStatus() == status);
+ }
+ })
+ );
+ }
+
+ public List<PatchLineComment> byPatchSet(ReviewDb db,
+ ChangeNotes notes, PatchSet.Id psId) throws OrmException {
+ if (!migration.readComments()) {
+ return db.patchComments().byPatchSet(psId).toList();
+ }
+ List<PatchLineComment> comments = Lists.newArrayList();
+ comments.addAll(publishedByPatchSet(db, notes, psId));
+
+ Iterable<String> filtered = getDraftRefs(notes.getChangeId());
+ for (String refName : filtered) {
+ Account.Id account = Account.Id.fromRefPart(refName);
+ if (account != null) {
+ comments.addAll(draftByPatchSetAuthor(db, psId, account, notes));
+ }
+ }
+ return comments;
+ }
+
public List<PatchLineComment> publishedByChangeFile(ReviewDb db,
ChangeNotes notes, Change.Id changeId, String file) throws OrmException {
- if (!migration.readPublishedComments()) {
+ if (!migration.readComments()) {
return db.patchComments().publishedByChangeFile(changeId, file).toList();
}
notes.load();
- List<PatchLineComment> commentsOnFile = new ArrayList<PatchLineComment>();
+ List<PatchLineComment> comments = Lists.newArrayList();
- // We must iterate through all comments to find the ones on this file.
- addCommentsInFile(commentsOnFile, notes.getBaseComments().values(), file);
- addCommentsInFile(commentsOnFile, notes.getPatchSetComments().values(),
+ addCommentsOnFile(comments, notes.getBaseComments().values(), file);
+ addCommentsOnFile(comments, notes.getPatchSetComments().values(),
file);
-
- Collections.sort(commentsOnFile, ChangeNotes.PatchLineCommentComparator);
- return commentsOnFile;
+ Collections.sort(comments, ChangeNotes.PatchLineCommentComparator);
+ return comments;
}
public List<PatchLineComment> publishedByPatchSet(ReviewDb db,
ChangeNotes notes, PatchSet.Id psId) throws OrmException {
- if (!migration.readPublishedComments()) {
+ if (!migration.readComments()) {
return db.patchComments().publishedByPatchSet(psId).toList();
}
notes.load();
- List<PatchLineComment> commentsOnPs = new ArrayList<PatchLineComment>();
- commentsOnPs.addAll(notes.getPatchSetComments().get(psId));
- commentsOnPs.addAll(notes.getBaseComments().get(psId));
- return commentsOnPs;
+ List<PatchLineComment> comments = new ArrayList<PatchLineComment>();
+ comments.addAll(notes.getPatchSetComments().get(psId));
+ comments.addAll(notes.getBaseComments().get(psId));
+ return comments;
}
- // TODO(yyonas): Delete drafts if they already existed.
- public void addPublishedComments(ReviewDb db, ChangeUpdate update,
+ public List<PatchLineComment> draftByPatchSetAuthor(ReviewDb db,
+ PatchSet.Id psId, Account.Id author, ChangeNotes notes)
+ throws OrmException {
+ if (!migration.readComments()) {
+ return db.patchComments().draftByPatchSetAuthor(psId, author).toList();
+ }
+
+ List<PatchLineComment> comments = Lists.newArrayList();
+
+ comments.addAll(notes.getDraftBaseComments(author).row(psId).values());
+ comments.addAll(notes.getDraftPsComments(author).row(psId).values());
+ Collections.sort(comments, ChangeNotes.PatchLineCommentComparator);
+ return comments;
+ }
+
+ public List<PatchLineComment> draftByChangeFileAuthor(ReviewDb db,
+ ChangeNotes notes, String file, Account.Id author)
+ throws OrmException {
+ if (!migration.readComments()) {
+ return db.patchComments()
+ .draftByChangeFileAuthor(notes.getChangeId(), file, author)
+ .toList();
+ }
+ List<PatchLineComment> comments = Lists.newArrayList();
+ addCommentsOnFile(comments, notes.getDraftBaseComments(author).values(),
+ file);
+ addCommentsOnFile(comments, notes.getDraftPsComments(author).values(),
+ file);
+ Collections.sort(comments, ChangeNotes.PatchLineCommentComparator);
+ return comments;
+ }
+
+ public List<PatchLineComment> draftByChangeAuthor(ReviewDb db,
+ ChangeNotes notes, Account.Id author)
+ throws OrmException {
+ if (!migration.readComments()) {
+ return db.patchComments().byChange(notes.getChangeId()).toList();
+ }
+ List<PatchLineComment> comments = Lists.newArrayList();
+ comments.addAll(notes.getDraftBaseComments(author).values());
+ comments.addAll(notes.getDraftPsComments(author).values());
+ return comments;
+ }
+
+ public List<PatchLineComment> draftByAuthor(ReviewDb db,
+ Account.Id author) throws OrmException {
+ if (!migration.readComments()) {
+ return db.patchComments().draftByAuthor(author).toList();
+ }
+
+ Set<String> refNames =
+ getRefNamesAllUsers(RefNames.REFS_DRAFT_COMMENTS);
+
+ List<PatchLineComment> comments = Lists.newArrayList();
+ for (String refName : refNames) {
+ Account.Id id = Account.Id.fromRefPart(refName);
+ if (!author.equals(id)) {
+ continue;
+ }
+ Change.Id changeId = Change.Id.parse(refName);
+ DraftCommentNotes draftNotes =
+ draftFactory.create(changeId, author).load();
+ comments.addAll(draftNotes.getDraftBaseComments().values());
+ comments.addAll(draftNotes.getDraftPsComments().values());
+ }
+ return comments;
+ }
+
+ public void insertComments(ReviewDb db, ChangeUpdate update,
+ Iterable<PatchLineComment> comments) throws OrmException {
+ for (PatchLineComment c : comments) {
+ update.insertComment(c);
+ }
+ db.patchComments().insert(comments);
+ }
+
+ public void upsertComments(ReviewDb db, ChangeUpdate update,
Iterable<PatchLineComment> comments) throws OrmException {
for (PatchLineComment c : comments) {
update.upsertComment(c);
@@ -86,7 +270,23 @@
db.patchComments().upsert(comments);
}
- private static Collection<PatchLineComment> addCommentsInFile(
+ public void updateComments(ReviewDb db, ChangeUpdate update,
+ Iterable<PatchLineComment> comments) throws OrmException {
+ for (PatchLineComment c : comments) {
+ update.updateComment(c);
+ }
+ db.patchComments().update(comments);
+ }
+
+ public void deleteComments(ReviewDb db, ChangeUpdate update,
+ Iterable<PatchLineComment> comments) throws OrmException {
+ for (PatchLineComment c : comments) {
+ update.deleteComment(c);
+ }
+ db.patchComments().delete(comments);
+ }
+
+ private static Collection<PatchLineComment> addCommentsOnFile(
Collection<PatchLineComment> commentsOnFile,
Collection<PatchLineComment> allComments,
String file) {
@@ -98,4 +298,49 @@
}
return commentsOnFile;
}
+
+ public static void setCommentRevId(PatchLineComment c,
+ PatchListCache cache, Change change, PatchSet ps) throws OrmException {
+ if (c.getRevId() != null) {
+ return;
+ }
+ PatchList patchList;
+ try {
+ patchList = cache.get(change, ps);
+ } catch (PatchListNotAvailableException e) {
+ throw new OrmException(e);
+ }
+ c.setRevId((c.getSide() == (short) 0)
+ ? new RevId(ObjectId.toString(patchList.getOldId()))
+ : new RevId(ObjectId.toString(patchList.getNewId())));
+ }
+
+ private Set<String> getRefNamesAllUsers(String prefix) throws OrmException {
+ Repository repo;
+ try {
+ repo = repoManager.openRepository(allUsers);
+ } catch (IOException e) {
+ throw new OrmException(e);
+ }
+ try {
+ RefDatabase refDb = repo.getRefDatabase();
+ return refDb.getRefs(prefix).keySet();
+ } catch (IOException e) {
+ throw new OrmException(e);
+ } finally {
+ repo.close();
+ }
+ }
+
+ private Iterable<String> getDraftRefs(final Change.Id changeId)
+ throws OrmException {
+ Set<String> refNames = getRefNamesAllUsers(RefNames.REFS_DRAFT_COMMENTS);
+ final String suffix = "-" + changeId.get();
+ return Iterables.filter(refNames, new Predicate<String>() {
+ @Override
+ public boolean apply(String input) {
+ return input.endsWith(suffix);
+ }
+ });
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
index 67dda03..e31e2e6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.account;
import com.google.gerrit.audit.AuditService;
-import com.google.gerrit.common.auth.openid.OpenIdUrls;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.Permission;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 586c294..fb80984 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -77,6 +77,7 @@
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.account.AccountInfo;
import com.google.gerrit.server.extensions.webui.UiActions;
@@ -127,6 +128,7 @@
private final Provider<WebLinks> webLinks;
private final EnumSet<ListChangesOption> options;
private final ChangeMessagesUtil cmUtil;
+ private final PatchLineCommentsUtil plcUtil;
private AccountInfo.Loader accountLoader;
@@ -147,7 +149,8 @@
DynamicMap<RestView<ChangeResource>> changeViews,
Revisions revisions,
Provider<WebLinks> webLinks,
- ChangeMessagesUtil cmUtil) {
+ ChangeMessagesUtil cmUtil,
+ PatchLineCommentsUtil plcUtil) {
this.db = db;
this.labelNormalizer = ln;
this.userProvider = user;
@@ -163,6 +166,7 @@
this.revisions = revisions;
this.webLinks = webLinks;
this.cmUtil = cmUtil;
+ this.plcUtil = plcUtil;
options = EnumSet.noneOf(ListChangesOption.class);
}
@@ -821,9 +825,8 @@
&& userProvider.get().isIdentifiedUser()) {
IdentifiedUser user = (IdentifiedUser)userProvider.get();
out.hasDraftComments =
- db.get().patchComments()
- .draftByPatchSetAuthor(in.getId(), user.getAccountId())
- .iterator().hasNext()
+ plcUtil.draftByPatchSetAuthor(db.get(), in.getId(),
+ user.getAccountId(), ctl.getNotes()).iterator().hasNext()
? true
: null;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java
index 1d2fa406..b025e65 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.change;
+import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
+
import com.google.common.base.Strings;
import com.google.gerrit.common.changes.Side;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -24,27 +26,41 @@
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.PutDraft.Input;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import java.io.IOException;
+import java.sql.Timestamp;
import java.util.Collections;
@Singleton
class CreateDraft implements RestModifyView<RevisionResource, Input> {
private final Provider<ReviewDb> db;
+ private final ChangeUpdate.Factory updateFactory;
+ private final PatchLineCommentsUtil plcUtil;
+ private final PatchListCache patchListCache;
@Inject
- CreateDraft(Provider<ReviewDb> db) {
+ CreateDraft(Provider<ReviewDb> db,
+ ChangeUpdate.Factory updateFactory,
+ PatchLineCommentsUtil plcUtil,
+ PatchListCache patchListCache) {
this.db = db;
+ this.updateFactory = updateFactory;
+ this.plcUtil = plcUtil;
+ this.patchListCache = patchListCache;
}
@Override
public Response<CommentInfo> apply(RevisionResource rsrc, Input in)
- throws BadRequestException, OrmException {
+ throws BadRequestException, OrmException, IOException {
if (Strings.isNullOrEmpty(in.path)) {
throw new BadRequestException("path must be non-empty");
} else if (in.message == null || in.message.trim().isEmpty()) {
@@ -59,15 +75,20 @@
? in.line
: in.range != null ? in.range.getEndLine() : 0;
+ Timestamp now = TimeUtil.nowTs();
+ ChangeUpdate update = updateFactory.create(rsrc.getControl(), now);
+
PatchLineComment c = new PatchLineComment(
new PatchLineComment.Key(
new Patch.Key(rsrc.getPatchSet().getId(), in.path),
ChangeUtil.messageUUID(db.get())),
- line, rsrc.getAccountId(), Url.decode(in.inReplyTo), TimeUtil.nowTs());
+ line, rsrc.getAccountId(), Url.decode(in.inReplyTo), now);
c.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1);
c.setMessage(in.message.trim());
c.setRange(in.range);
- db.get().patchComments().insert(Collections.singleton(c));
+ setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
+ plcUtil.insertComments(db.get(), update, Collections.singleton(c));
+ update.commit();
return Response.created(new CommentInfo(c, null));
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraft.java
index 46ae834..f7fb300 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraft.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraft.java
@@ -14,15 +14,22 @@
package com.google.gerrit.server.change;
+import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
+
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.DeleteDraft.Input;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.patch.PatchListCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import java.io.IOException;
import java.util.Collections;
@Singleton
@@ -31,16 +38,30 @@
}
private final Provider<ReviewDb> db;
+ private final PatchLineCommentsUtil plcUtil;
+ private final ChangeUpdate.Factory updateFactory;
+ private final PatchListCache patchListCache;
@Inject
- DeleteDraft(Provider<ReviewDb> db) {
+ DeleteDraft(Provider<ReviewDb> db,
+ PatchLineCommentsUtil plcUtil,
+ ChangeUpdate.Factory updateFactory,
+ PatchListCache patchListCache) {
this.db = db;
+ this.plcUtil = plcUtil;
+ this.updateFactory = updateFactory;
+ this.patchListCache = patchListCache;
}
@Override
public Response<CommentInfo> apply(DraftResource rsrc, Input input)
- throws OrmException {
- db.get().patchComments().delete(Collections.singleton(rsrc.getComment()));
+ throws OrmException, IOException {
+ ChangeUpdate update = updateFactory.create(rsrc.getControl());
+
+ PatchLineComment c = rsrc.getComment();
+ setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
+ plcUtil.deleteComments(db.get(), update, Collections.singleton(c));
+ update.commit();
return Response.none();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Drafts.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Drafts.java
index 322faea..b0ed7d0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Drafts.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Drafts.java
@@ -23,6 +23,7 @@
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -34,16 +35,19 @@
private final Provider<CurrentUser> user;
private final ListDrafts list;
private final Provider<ReviewDb> dbProvider;
+ private final PatchLineCommentsUtil plcUtil;
@Inject
Drafts(DynamicMap<RestView<DraftResource>> views,
Provider<CurrentUser> user,
ListDrafts list,
- Provider<ReviewDb> dbProvider) {
+ Provider<ReviewDb> dbProvider,
+ PatchLineCommentsUtil plcUtil) {
this.views = views;
this.user = user;
this.list = list;
this.dbProvider = dbProvider;
+ this.plcUtil = plcUtil;
}
@Override
@@ -62,10 +66,8 @@
throws ResourceNotFoundException, OrmException, AuthException {
checkIdentifiedUser();
String uuid = id.get();
- for (PatchLineComment c : dbProvider.get().patchComments()
- .draftByPatchSetAuthor(
- rev.getPatchSet().getId(),
- rev.getAccountId())) {
+ for (PatchLineComment c : plcUtil.draftByPatchSetAuthor(dbProvider.get(),
+ rev.getPatchSet().getId(), rev.getAccountId(), rev.getNotes())) {
if (uuid.equals(c.getKey().get())) {
return new DraftResource(rev, c);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
index 0ccb15c..fd727c4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
@@ -21,12 +21,12 @@
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.common.CommitInfo;
-import com.google.gerrit.extensions.common.GitPerson;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetAncestor;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectControl;
@@ -39,7 +39,6 @@
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -50,7 +49,6 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
-import java.sql.Timestamp;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedList;
@@ -272,15 +270,6 @@
return r;
}
- private static GitPerson toGitPerson(PersonIdent id) {
- GitPerson p = new GitPerson();
- p.name = id.getName();
- p.email = id.getEmailAddress();
- p.date = new Timestamp(id.getWhen().getTime());
- p.tz = id.getTimeZoneOffset();
- return p;
- }
-
public static class RelatedInfo {
public List<ChangeAndCommit> changes;
}
@@ -309,7 +298,7 @@
p.commit = c.getParent(i).name();
commit.parents.add(p);
}
- commit.author = toGitPerson(c.getAuthorIdent());
+ commit.author = CommonConverters.toGitPerson(c.getAuthorIdent());
commit.subject = c.getShortMessage();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java
index f4d7b49..146ded8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java
@@ -26,13 +26,10 @@
@Singleton
class ListComments extends ListDrafts {
- private final PatchLineCommentsUtil plcUtil;
-
@Inject
ListComments(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf,
PatchLineCommentsUtil plcUtil) {
- super(db, alf);
- this.plcUtil = plcUtil;
+ super(db, alf, plcUtil);
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java
index bd3aa04..1a26898 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java
@@ -22,6 +22,7 @@
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.account.AccountInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -36,20 +37,21 @@
@Singleton
class ListDrafts implements RestReadView<RevisionResource> {
protected final Provider<ReviewDb> db;
+ protected final PatchLineCommentsUtil plcUtil;
private final AccountInfo.Loader.Factory accountLoaderFactory;
@Inject
- ListDrafts(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf) {
+ ListDrafts(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf,
+ PatchLineCommentsUtil plcUtil) {
this.db = db;
this.accountLoaderFactory = alf;
+ this.plcUtil = plcUtil;
}
protected Iterable<PatchLineComment> listComments(RevisionResource rsrc)
throws OrmException {
- return db.get().patchComments()
- .draftByPatchSetAuthor(
- rsrc.getPatchSet().getId(),
- rsrc.getAccountId());
+ return plcUtil.draftByPatchSetAuthor(db.get(), rsrc.getPatchSet().getId(),
+ rsrc.getAccountId(), rsrc.getNotes());
}
protected boolean includeAuthorInfo() {
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 044e899..21f5a69 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
@@ -34,6 +34,7 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.BanCommit;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.mail.ReplacePatchSetSender;
@@ -54,6 +55,7 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
@@ -358,7 +360,7 @@
}
}
- private void validate() throws InvalidChangeOperationException {
+ private void validate() throws InvalidChangeOperationException, IOException {
CommitValidators cv =
commitValidatorsFactory.create(ctl.getRefControl(), sshInfo, git);
@@ -374,7 +376,8 @@
try {
switch (validatePolicy) {
case RECEIVE_COMMITS:
- cv.validateForReceiveCommits(event);
+ NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(git, revWalk);
+ cv.validateForReceiveCommits(event, rejectCommits);
break;
case GERRIT:
cv.validateForGerritCommits(event);
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 9a2a81e..bf8474f 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
@@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
import com.google.common.base.Objects;
import com.google.common.base.Strings;
@@ -44,7 +45,6 @@
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
@@ -54,9 +54,7 @@
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
-import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.util.LabelVote;
@@ -65,7 +63,6 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
-import org.eclipse.jgit.lib.ObjectId;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -346,15 +343,6 @@
List<PatchLineComment> del = Lists.newArrayList();
List<PatchLineComment> ups = Lists.newArrayList();
- PatchList patchList = null;
- try {
- patchList = patchListCache.get(rsrc.getChange(), rsrc.getPatchSet());
- } catch (PatchListNotAvailableException e) {
- throw new OrmException("could not load PatchList for this patchset", e);
- }
- RevId patchSetCommit = new RevId(ObjectId.toString(patchList.getNewId()));
- RevId baseCommit = new RevId(ObjectId.toString(patchList.getOldId()));
-
for (Map.Entry<String, List<CommentInput>> ent : in.entrySet()) {
String path = ent.getKey();
for (CommentInput c : ent.getValue()) {
@@ -374,7 +362,8 @@
e.setStatus(PatchLineComment.Status.PUBLISHED);
e.setWrittenOn(timestamp);
e.setSide(c.side == Side.PARENT ? (short) 0 : (short) 1);
- e.setRevId(c.side == Side.PARENT ? baseCommit : patchSetCommit);
+ setCommentRevId(e, patchListCache, rsrc.getChange(),
+ rsrc.getPatchSet());
e.setMessage(c.message);
if (c.range != null) {
e.setRange(new CommentRange(
@@ -398,13 +387,14 @@
for (PatchLineComment e : drafts.values()) {
e.setStatus(PatchLineComment.Status.PUBLISHED);
e.setWrittenOn(timestamp);
- e.setRevId(e.getSide() == (short) 0 ? baseCommit : patchSetCommit);
+ setCommentRevId(e, patchListCache, rsrc.getChange(),
+ rsrc.getPatchSet());
ups.add(e);
}
break;
}
- db.get().patchComments().delete(del);
- plcUtil.addPublishedComments(db.get(), update, ups);
+ plcUtil.deleteComments(db.get(), update, del);
+ plcUtil.upsertComments(db.get(), update, ups);
comments.addAll(ups);
return !del.isEmpty() || !ups.isEmpty();
}
@@ -412,9 +402,8 @@
private Map<String, PatchLineComment> scanDraftComments(
RevisionResource rsrc) throws OrmException {
Map<String, PatchLineComment> drafts = Maps.newHashMap();
- for (PatchLineComment c : db.get().patchComments().draftByPatchSetAuthor(
- rsrc.getPatchSet().getId(),
- rsrc.getAccountId())) {
+ for (PatchLineComment c : plcUtil.draftByPatchSetAuthor(db.get(),
+ rsrc.getPatchSet().getId(), rsrc.getAccountId(), rsrc.getNotes())) {
drafts.put(c.getKey().get(), c);
}
return drafts;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java
index c1fb304..a3b0614 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.change;
+import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
+
import com.google.gerrit.common.changes.Side;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.DefaultInput;
@@ -24,13 +26,17 @@
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.PutDraft.Input;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import java.io.IOException;
import java.sql.Timestamp;
import java.util.Collections;
@@ -51,17 +57,28 @@
private final Provider<ReviewDb> db;
private final DeleteDraft delete;
+ private final PatchLineCommentsUtil plcUtil;
+ private final ChangeUpdate.Factory updateFactory;
+ private final PatchListCache patchListCache;
@Inject
- PutDraft(Provider<ReviewDb> db, DeleteDraft delete) {
+ PutDraft(Provider<ReviewDb> db,
+ DeleteDraft delete,
+ PatchLineCommentsUtil plcUtil,
+ ChangeUpdate.Factory updateFactory,
+ PatchListCache patchListCache) {
this.db = db;
this.delete = delete;
+ this.plcUtil = plcUtil;
+ this.updateFactory = updateFactory;
+ this.patchListCache = patchListCache;
}
@Override
public Response<CommentInfo> apply(DraftResource rsrc, Input in) throws
- BadRequestException, OrmException {
+ BadRequestException, OrmException, IOException {
PatchLineComment c = rsrc.getComment();
+ ChangeUpdate update = updateFactory.create(rsrc.getControl());
if (in == null || in.message == null || in.message.trim().isEmpty()) {
return delete.apply(rsrc, null);
} else if (in.id != null && !rsrc.getId().equals(in.id)) {
@@ -76,7 +93,8 @@
&& !in.path.equals(c.getKey().getParentKey().getFileName())) {
// Updating the path alters the primary key, which isn't possible.
// Delete then recreate the comment instead of an update.
- db.get().patchComments().delete(Collections.singleton(c));
+
+ plcUtil.deleteComments(db.get(), update, Collections.singleton(c));
c = new PatchLineComment(
new PatchLineComment.Key(
new Patch.Key(rsrc.getPatchSet().getId(), in.path),
@@ -84,10 +102,18 @@
c.getLine(),
rsrc.getAuthorId(),
c.getParentUuid(), TimeUtil.nowTs());
- db.get().patchComments().insert(Collections.singleton(update(c, in)));
+ setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
+ plcUtil.insertComments(db.get(), update,
+ Collections.singleton(update(c, in)));
} else {
- db.get().patchComments().update(Collections.singleton(update(c, in)));
+ if (c.getRevId() == null) {
+ setCommentRevId(c, patchListCache, rsrc.getChange(),
+ rsrc.getPatchSet());
+ }
+ plcUtil.updateComments(db.get(), update,
+ Collections.singleton(update(c, in)));
}
+ update.commit();
return Response.ok(new CommentInfo(c, null));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java
index 7ff91ef..b97ddb5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java
@@ -48,15 +48,15 @@
@Singleton
public class BanCommit {
-
/**
* Loads a list of commits to reject from {@code refs/meta/reject-commits}.
*
* @param repo repository from which the rejected commits should be loaded
+ * @param walk open revwalk on repo.
* @return NoteMap of commits to be rejected, null if there are none.
* @throws IOException the map cannot be loaded.
*/
- public static NoteMap loadRejectCommitsMap(Repository repo)
+ public static NoteMap loadRejectCommitsMap(Repository repo, RevWalk walk)
throws IOException {
try {
Ref ref = repo.getRef(RefNames.REFS_REJECT_COMMITS);
@@ -64,13 +64,8 @@
return NoteMap.newEmptyMap();
}
- RevWalk rw = new RevWalk(repo);
- try {
- RevCommit map = rw.parseCommit(ref.getObjectId());
- return NoteMap.read(rw.getObjectReader(), map);
- } finally {
- rw.release();
- }
+ RevCommit map = walk.parseCommit(ref.getObjectId());
+ return NoteMap.read(walk.getObjectReader(), map);
} catch (IOException badMap) {
throw new IOException("Cannot load " + RefNames.REFS_REJECT_COMMITS,
badMap);
@@ -79,7 +74,7 @@
private final Provider<IdentifiedUser> currentUser;
private final GitRepositoryManager repoManager;
- private final PersonIdent gerritIdent;
+ private final TimeZone tz;
private NotesBranchUtil.Factory notesBranchUtilFactory;
@Inject
@@ -89,8 +84,8 @@
final NotesBranchUtil.Factory notesBranchUtilFactory) {
this.currentUser = currentUser;
this.repoManager = repoManager;
- this.gerritIdent = gerritIdent;
this.notesBranchUtilFactory = notesBranchUtilFactory;
+ this.tz = gerritIdent.getTimeZone();
}
public BanCommitResult ban(final ProjectControl projectControl,
@@ -99,30 +94,33 @@
MergeException, ConcurrentRefUpdateException {
if (!projectControl.isOwner()) {
throw new PermissionDeniedException(
- "No project owner: not permitted to ban commits");
+ "Not project owner: not permitted to ban commits");
}
final BanCommitResult result = new BanCommitResult();
NoteMap banCommitNotes = NoteMap.newEmptyMap();
- // add a note for each banned commit to notes
+ // Add a note for each banned commit to notes.
final Project.NameKey project = projectControl.getProject().getNameKey();
final Repository repo = repoManager.openRepository(project);
try {
final RevWalk revWalk = new RevWalk(repo);
final ObjectInserter inserter = repo.newObjectInserter();
try {
+ ObjectId noteId = null;
for (final ObjectId commitToBan : commitsToBan) {
try {
revWalk.parseCommit(commitToBan);
} catch (MissingObjectException e) {
- // ignore exception, also not existing commits can be banned
+ // Ignore exception, non-existing commits can be banned.
} catch (IncorrectObjectTypeException e) {
result.notACommit(commitToBan, e.getMessage());
continue;
}
- banCommitNotes.set(commitToBan, createNoteContent(reason, inserter));
+ if (noteId == null) {
+ noteId = createNoteContent(reason, inserter);
+ }
+ banCommitNotes.set(commitToBan, noteId);
}
- inserter.flush();
NotesBranchUtil notesBranchUtil =
notesBranchUtilFactory.create(project, repo, inserter);
NoteMap newlyCreated =
@@ -157,7 +155,6 @@
private PersonIdent createPersonIdent() {
Date now = new Date();
- TimeZone tz = gerritIdent.getTimeZone();
return currentUser.get().newCommitterIdent(now, tz);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
index b48028e..b65cc85 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.git;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -21,8 +22,10 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate;
@@ -59,7 +62,24 @@
public MetaDataUpdate create(Project.NameKey name, IdentifiedUser user)
throws RepositoryNotFoundException, IOException {
- MetaDataUpdate md = factory.create(name, mgr.openRepository(name));
+ return create(name, user, null);
+ }
+
+ /**
+ * Create an update using an existing batch ref update.
+ * <p>
+ * This allows batching together updates to multiple metadata refs. For making
+ * multiple commits to a single metadata ref, see
+ * {@link VersionedMetaData#openUpdate(MetaDataUpdate)}.
+ *
+ * @param name project name.
+ * @param user user for the update.
+ * @param batch batch update to use; the caller is responsible for committing
+ * the update.
+ */
+ public MetaDataUpdate create(Project.NameKey name, IdentifiedUser user,
+ BatchRefUpdate batch) throws RepositoryNotFoundException, IOException {
+ MetaDataUpdate md = factory.create(name, mgr.openRepository(name), batch);
md.getCommitBuilder().setAuthor(createPersonIdent(user));
md.getCommitBuilder().setCommitter(serverIdent);
return md;
@@ -86,7 +106,13 @@
public MetaDataUpdate create(Project.NameKey name)
throws RepositoryNotFoundException, IOException {
- MetaDataUpdate md = factory.create(name, mgr.openRepository(name));
+ return create(name, null);
+ }
+
+ /** @see User#create(Project.NameKey, IdentifiedUser, BatchRefUpdate) */
+ public MetaDataUpdate create(Project.NameKey name, BatchRefUpdate batch)
+ throws RepositoryNotFoundException, IOException {
+ MetaDataUpdate md = factory.create(name, mgr.openRepository(name), batch);
md.getCommitBuilder().setAuthor(serverIdent);
md.getCommitBuilder().setCommitter(serverIdent);
return md;
@@ -95,24 +121,32 @@
interface InternalFactory {
MetaDataUpdate create(@Assisted Project.NameKey projectName,
- @Assisted Repository db);
+ @Assisted Repository db, @Assisted @Nullable BatchRefUpdate batch);
}
private final GitReferenceUpdated gitRefUpdated;
private final Project.NameKey projectName;
private final Repository db;
+ private final BatchRefUpdate batch;
private final CommitBuilder commit;
private boolean allowEmpty;
- @Inject
+ @AssistedInject
public MetaDataUpdate(GitReferenceUpdated gitRefUpdated,
- @Assisted Project.NameKey projectName, @Assisted Repository db) {
+ @Assisted Project.NameKey projectName, @Assisted Repository db,
+ @Assisted @Nullable BatchRefUpdate batch) {
this.gitRefUpdated = gitRefUpdated;
this.projectName = projectName;
this.db = db;
+ this.batch = batch;
this.commit = new CommitBuilder();
}
+ public MetaDataUpdate(GitReferenceUpdated gitRefUpdated,
+ Project.NameKey projectName, Repository db) {
+ this(gitRefUpdated, projectName, db, null);
+ }
+
/** Set the commit message used when committing the update. */
public void setMessage(String message) {
getCommitBuilder().setMessage(message);
@@ -128,6 +162,11 @@
this.allowEmpty = allowEmpty;
}
+ /** @return batch in which to run the update, or {@code null} for no batch. */
+ BatchRefUpdate getBatch() {
+ return batch;
+ }
+
/** Close the cached Repository handle. */
public void close() {
getRepository().close();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
index a905385..7292cc4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
@@ -220,6 +220,10 @@
this.projectName = projectName;
}
+ public Project.NameKey getName() {
+ return projectName;
+ }
+
public Project getProject() {
return project;
}
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 711c3f8..0a91f2e 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
@@ -415,7 +415,7 @@
this.project = projectControl.getProject();
this.repo = repo;
this.rp = new ReceivePack(repo);
- this.rejectCommits = BanCommit.loadRejectCommitsMap(repo);
+ this.rejectCommits = BanCommit.loadRejectCommitsMap(repo, rp.getRevWalk());
this.subOpFactory = subOpFactory;
this.submitProvider = submitProvider;
@@ -991,7 +991,8 @@
}
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
- if (ctl.canCreate(rp.getRevWalk(), obj, allRefs.values().contains(obj))) {
+ rp.getRevWalk().reset();
+ if (ctl.canCreate(db, rp.getRevWalk(), obj)) {
validateNewCommits(ctl, cmd);
batch.addCommand(cmd);
} else {
@@ -2130,8 +2131,11 @@
allRefs.size() / estRefsPerChange,
estRefsPerChange);
for (Ref ref : allRefs.values()) {
- if (ref.getObjectId() != null && PatchSet.isRef(ref.getName())) {
- refsByChange.put(Change.Id.fromRef(ref.getName()), ref);
+ if (ref.getObjectId() != null) {
+ PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName());
+ if (psId != null) {
+ refsByChange.put(psId.getParentKey(), ref);
+ }
}
}
}
@@ -2267,7 +2271,8 @@
commitValidatorsFactory.create(ctl, sshInfo, repo);
try {
- messages.addAll(commitValidators.validateForReceiveCommits(receiveEvent));
+ messages.addAll(commitValidators.validateForReceiveCommits(
+ receiveEvent, rejectCommits));
} catch (CommitValidationException e) {
messages.addAll(e.getMessages());
reject(cmd, e.getMessage());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
index 4c5ebc6..a58d9b6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
@@ -26,6 +26,7 @@
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -40,6 +41,7 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.util.RawParseUtils;
@@ -181,6 +183,21 @@
void close();
}
+ /**
+ * Open a batch of updates to the same metadata ref.
+ * <p>
+ * This allows making multiple commits to a single metadata ref, at the end of
+ * which is a single ref update. For batching together updates to multiple
+ * refs (each consisting of one or more commits against their respective
+ * refs), create the {@link MetaDataUpdate} with a {@link BatchRefUpdate}.
+ * <p>
+ * A ref update produced by this {@link BatchMetaDataUpdate} is only committed
+ * if there is no associated {@link BatchRefUpdate}. As a result, the
+ * configured ref updated event is not fired if there is an associated batch.
+ *
+ * @param update helper info about the update.
+ * @throws IOException if the update failed.
+ */
public BatchMetaDataUpdate openUpdate(final MetaDataUpdate update) throws IOException {
final Repository db = update.getRepository();
@@ -302,6 +319,15 @@
private RevCommit updateRef(AnyObjectId oldId, AnyObjectId newId,
String refName) throws IOException {
+ BatchRefUpdate bru = update.getBatch();
+ if (bru != null) {
+ bru.addCommand(new ReceiveCommand(
+ oldId.toObjectId(), newId.toObjectId(), refName));
+ inserter.flush();
+ revision = rw.parseCommit(newId);
+ return revision;
+ }
+
RefUpdate ru = db.updateRef(refName);
ru.setExpectedOldObjectId(oldId);
ru.setNewObjectId(src);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java
index 62d80fa..30db2b9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java
@@ -69,10 +69,11 @@
}
public Map<String, Ref> filter(Map<String, Ref> refs, boolean filterTagsSeperately) {
- if (projectCtl.allRefsAreVisibleExcept(
- ImmutableSet.of(RefNames.REFS_CONFIG))) {
+ if (projectCtl.allRefsAreVisible(ImmutableSet.of(RefNames.REFS_CONFIG))) {
Map<String, Ref> r = Maps.newHashMap(refs);
- r.remove(RefNames.REFS_CONFIG);
+ if (!projectCtl.controlForRef(RefNames.REFS_CONFIG).isVisible()) {
+ r.remove(RefNames.REFS_CONFIG);
+ }
return r;
}
@@ -81,12 +82,13 @@
final List<Ref> deferredTags = new ArrayList<>();
for (Ref ref : refs.values()) {
+ PatchSet.Id psId;
if (ref.getName().startsWith(RefNames.REFS_CACHE_AUTOMERGE)) {
continue;
- } else if (PatchSet.isRef(ref.getName())) {
+ } else if ((psId = PatchSet.Id.fromRef(ref.getName())) != null) {
// Reference to a patch set is visible if the change is visible.
//
- if (showChanges && visibleChanges.contains(Change.Id.fromRef(ref.getName()))) {
+ if (showChanges && visibleChanges.contains(psId.getParentKey())) {
result.put(ref.getName(), ref);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 9d0eb66..cad5174 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -24,7 +24,6 @@
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.events.CommitReceivedEvent;
-import com.google.gerrit.server.git.BanCommit;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.ReceiveCommits;
import com.google.gerrit.server.git.ValidationError;
@@ -93,7 +92,8 @@
}
public List<CommitValidationMessage> validateForReceiveCommits(
- CommitReceivedEvent receiveEvent) throws CommitValidationException {
+ CommitReceivedEvent receiveEvent, NoteMap rejectCommits)
+ throws CommitValidationException {
List<CommitValidationListener> validators = new LinkedList<>();
@@ -110,7 +110,7 @@
installCommitMsgHookCommand, sshInfo));
}
validators.add(new ConfigValidator(refControl, repo));
- validators.add(new BannedCommitsValidator(repo));
+ validators.add(new BannedCommitsValidator(rejectCommits));
validators.add(new PluginCommitValidationListener(commitValidationListeners));
List<CommitValidationMessage> messages = new LinkedList<>();
@@ -522,24 +522,25 @@
/** Reject banned commits. */
public static class BannedCommitsValidator implements
CommitValidationListener {
- private final Repository repo;
+ private final NoteMap rejectCommits;
- public BannedCommitsValidator(Repository repo) {
- this.repo = repo;
+ public BannedCommitsValidator(NoteMap rejectCommits) {
+ this.rejectCommits = rejectCommits;
}
@Override
public List<CommitValidationMessage> onCommitReceived(
CommitReceivedEvent receiveEvent) throws CommitValidationException {
try {
- NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(repo);
if (rejectCommits.contains(receiveEvent.commit)) {
throw new CommitValidationException("contains banned commit "
+ receiveEvent.commit.getName());
}
return Collections.emptyList();
} catch (IOException e) {
- throw new CommitValidationException(e.getMessage(), e);
+ String m = "error checking banned commits";
+ log.warn(m, e);
+ throw new CommitValidationException(m, e);
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
index 41dfba5..68e7857 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
@@ -408,7 +408,7 @@
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
Set<String> r = Sets.newHashSet();
- for (PatchLineComment c : input.comments()) {
+ for (PatchLineComment c : input.publishedComments()) {
r.add(c.getMessage());
}
for (ChangeMessage m : input.messages()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java
index 2beb49f..5f2ffcb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.mail;
+import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.Ordering;
import com.google.gerrit.common.errors.EmailException;
@@ -24,6 +25,7 @@
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.patch.PatchFile;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -53,13 +55,16 @@
private final NotifyHandling notify;
private List<PatchLineComment> inlineComments = Collections.emptyList();
+ private final PatchLineCommentsUtil plcUtil;
@Inject
public CommentSender(EmailArguments ea,
@Assisted NotifyHandling notify,
- @Assisted Change c) {
+ @Assisted Change c,
+ PatchLineCommentsUtil plcUtil) {
super(ea, c, "comment");
this.notify = notify;
+ this.plcUtil = plcUtil;
}
public void setPatchLineComments(final List<PatchLineComment> plc)
@@ -232,17 +237,19 @@
private void appendQuotedParent(StringBuilder out, PatchLineComment child) {
if (child.getParentUuid() != null) {
- PatchLineComment parent;
+ Optional<PatchLineComment> parent;
+ PatchLineComment.Key key = new PatchLineComment.Key(
+ child.getKey().getParentKey(),
+ child.getParentUuid());
try {
- parent = args.db.get().patchComments().get(
- new PatchLineComment.Key(
- child.getKey().getParentKey(),
- child.getParentUuid()));
+ parent = plcUtil.get(args.db.get(), changeData.notes(), key);
} catch (OrmException e) {
- parent = null;
+ log.warn("Could not find the parent of this comment: "
+ + child.toString());
+ parent = Optional.absent();
}
- if (parent != null) {
- String msg = parent.getMessage().trim();
+ if (parent.isPresent()) {
+ String msg = parent.get().getMessage().trim();
if (msg.length() > 75) {
msg = msg.substring(0, 75);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index 6d9b30d..f114af0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST;
+import com.google.gerrit.common.data.AccountInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -28,6 +29,7 @@
import com.google.gerrit.server.project.ChangeControl;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
@@ -43,6 +45,7 @@
protected final GitRepositoryManager repoManager;
protected final MetaDataUpdate.User updateFactory;
protected final ChangeControl ctl;
+ protected final String anonymousCowardName;
protected final PersonIdent serverIdent;
protected final Date when;
protected PatchSet.Id psId;
@@ -50,12 +53,15 @@
AbstractChangeUpdate(NotesMigration migration,
GitRepositoryManager repoManager,
MetaDataUpdate.User updateFactory, ChangeControl ctl,
- PersonIdent serverIdent, Date when) {
+ PersonIdent serverIdent,
+ String anonymousCowardName,
+ Date when) {
this.migration = migration;
this.repoManager = repoManager;
this.updateFactory = updateFactory;
this.ctl = ctl;
this.serverIdent = serverIdent;
+ this.anonymousCowardName = anonymousCowardName;
this.when = when;
}
@@ -100,10 +106,15 @@
}
public BatchMetaDataUpdate openUpdate() throws IOException {
+ return openUpdateInBatch(null);
+ }
+
+ public BatchMetaDataUpdate openUpdateInBatch(BatchRefUpdate bru)
+ throws IOException {
if (migration.write()) {
load();
MetaDataUpdate md =
- updateFactory.create(getProjectName(), getUser());
+ updateFactory.create(getProjectName(), getUser(), bru);
md.setAllowEmpty(true);
return super.openUpdate(md);
}
@@ -157,7 +168,7 @@
protected PersonIdent newIdent(Account author, Date when) {
return new PersonIdent(
- author.getFullName(),
+ new AccountInfo(author).getName(anonymousCowardName),
author.getId().get() + "@" + GERRIT_PLACEHOLDER_HOST,
when, serverIdent.getTimeZone());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
index 6947849..ca32c14 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.project.ChangeControl;
@@ -75,6 +76,7 @@
@AssistedInject
private ChangeDraftUpdate(
@GerritPersonIdent PersonIdent serverIdent,
+ @AnonymousCowardName String anonymousCowardName,
GitRepositoryManager repoManager,
NotesMigration migration,
MetaDataUpdate.User updateFactory,
@@ -83,7 +85,8 @@
CommentsInNotesUtil commentsUtil,
@Assisted ChangeControl ctl,
@Assisted Date when) throws OrmException {
- super(migration, repoManager, updateFactory, ctl, serverIdent, when);
+ super(migration, repoManager, updateFactory, ctl, serverIdent,
+ anonymousCowardName, when);
this.draftsProject = allUsers;
this.commentsUtil = commentsUtil;
checkState(ctl.getCurrentUser().isIdentifiedUser(),
@@ -102,9 +105,11 @@
verifyComment(c);
checkArgument(c.getStatus() == Status.DRAFT,
"Cannot insert a published comment into a ChangeDraftUpdate");
- checkArgument(!changeNotes.containsComment(c),
- "A comment already exists with the same key,"
- + " so the following comment cannot be inserted: %s", c);
+ if (migration.readComments()) {
+ checkArgument(!changeNotes.containsComment(c),
+ "A comment already exists with the same key,"
+ + " so the following comment cannot be inserted: %s", c);
+ }
upsertComments.add(c);
}
@@ -119,16 +124,32 @@
verifyComment(c);
checkArgument(c.getStatus() == Status.DRAFT,
"Cannot update a published comment into a ChangeDraftUpdate");
- checkArgument(draftNotes.containsComment(c),
- "Cannot update this comment because it didn't exist previously");
+ // Here, we check to see if this comment existed previously as a draft.
+ // However, this could cause a race condition if there is a delete and an
+ // update operation happening concurrently (or two deletes) and they both
+ // believe that the comment exists. If a delete happens first, then
+ // the update will fail. However, this is an acceptable risk since the
+ // caller wanted the comment deleted anyways, so the end result will be the
+ // same either way.
+ if (migration.readComments()) {
+ checkArgument(draftNotes.containsComment(c),
+ "Cannot update this comment because it didn't exist previously");
+ }
upsertComments.add(c);
}
public void deleteComment(PatchLineComment c) {
verifyComment(c);
- checkArgument(draftNotes.containsComment(c), "Cannot delete this comment"
- + " because it didn't previously exist as a draft");
- deleteComments.add(c);
+ // See the comment above about potential race condition.
+ if (migration.readComments()) {
+ checkArgument(draftNotes.containsComment(c), "Cannot delete this comment"
+ + " because it didn't previously exist as a draft");
+ }
+ if (migration.write()) {
+ if (draftNotes.containsComment(c)) {
+ deleteComments.add(c);
+ }
+ }
}
/**
@@ -148,7 +169,9 @@
checkArgument(getCommentPsId(comment).equals(psId),
"Comment on %s does not match configured patch set %s",
getCommentPsId(comment), psId);
- checkArgument(comment.getRevId() != null);
+ if (migration.write()) {
+ checkArgument(comment.getRevId() != null);
+ }
checkArgument(comment.getAuthor().equals(accountId),
"The author for the following comment does not match the author of"
+ " this ChangeDraftUpdate (%s): %s", accountId, comment);
@@ -171,6 +194,8 @@
Table<PatchSet.Id, String, PatchLineComment> psDrafts =
draftNotes.getDraftPsComments();
+ boolean draftsEmpty = baseDrafts.isEmpty() && psDrafts.isEmpty();
+
// There is no need to rewrite the note for one of the sides of the patch
// set if all of the modifications were made to the comments of one side,
// so we set these flags to potentially save that extra work.
@@ -216,7 +241,8 @@
updateNoteMap(revisionSideChanged, noteMap, newPsDrafts,
psRevId);
- removedAllComments.set(baseDrafts.isEmpty() && psDrafts.isEmpty());
+ removedAllComments.set(
+ baseDrafts.isEmpty() && psDrafts.isEmpty() && !draftsEmpty);
return noteMap.writeTree(inserter);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index fe67527..c4eeb2e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -46,7 +46,6 @@
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSet.Id;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
import com.google.gerrit.reviewdb.client.Project;
@@ -163,7 +162,7 @@
private final List<Account.Id> allPastReviewers;
private final List<SubmitRecord> submitRecords;
private final Multimap<PatchSet.Id, ChangeMessage> changeMessages;
- private final Multimap<Id, PatchLineComment> commentsForPs;
+ private final Multimap<PatchSet.Id, PatchLineComment> commentsForPs;
private final Multimap<PatchSet.Id, PatchLineComment> commentsForBase;
private NoteMap commentNoteMap;
private Change.Status status;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 69b98c8..770315a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -39,6 +39,7 @@
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.config.AllUsersName;
import com.google.inject.Provider;
+import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.project.ChangeControl;
@@ -101,6 +102,7 @@
@AssistedInject
private ChangeUpdate(
@GerritPersonIdent PersonIdent serverIdent,
+ @AnonymousCowardName String anonymousCowardName,
GitRepositoryManager repoManager,
NotesMigration migration,
AccountCache accountCache,
@@ -112,14 +114,15 @@
IdentifiedUser user,
@Assisted ChangeControl ctl,
CommentsInNotesUtil commentsUtil) {
- this(serverIdent, repoManager, migration, accountCache, updateFactory,
- draftNotesFactory, allUsers, draftUpdateFactory, projectCache, ctl,
- serverIdent.getWhen(), commentsUtil);
+ this(serverIdent, anonymousCowardName, repoManager, migration, accountCache,
+ updateFactory, draftNotesFactory, allUsers, draftUpdateFactory,
+ projectCache, ctl, serverIdent.getWhen(), commentsUtil);
}
@AssistedInject
private ChangeUpdate(
@GerritPersonIdent PersonIdent serverIdent,
+ @AnonymousCowardName String anonymousCowardName,
GitRepositoryManager repoManager,
NotesMigration migration,
AccountCache accountCache,
@@ -131,8 +134,9 @@
@Assisted ChangeControl ctl,
@Assisted Date when,
CommentsInNotesUtil commentsUtil) {
- this(serverIdent, repoManager, migration, accountCache, updateFactory,
- draftNotesFactory, allUsers, draftUpdateFactory, ctl, when,
+ this(serverIdent, anonymousCowardName, repoManager, migration, accountCache,
+ updateFactory, draftNotesFactory, allUsers, draftUpdateFactory, ctl,
+ when,
projectCache.get(getProjectName(ctl)).getLabelTypes().nameComparator(),
commentsUtil);
}
@@ -144,6 +148,7 @@
@AssistedInject
private ChangeUpdate(
@GerritPersonIdent PersonIdent serverIdent,
+ @AnonymousCowardName String anonymousCowardName,
GitRepositoryManager repoManager,
NotesMigration migration,
AccountCache accountCache,
@@ -155,7 +160,8 @@
@Assisted Date when,
@Assisted Comparator<String> labelNameComparator,
CommentsInNotesUtil commentsUtil) {
- super(migration, repoManager, updateFactory, ctl, serverIdent, when);
+ super(migration, repoManager, updateFactory, ctl, serverIdent,
+ anonymousCowardName, when);
this.draftUpdateFactory = draftUpdateFactory;
this.accountCache = accountCache;
this.commentsUtil = commentsUtil;
@@ -233,9 +239,11 @@
if (notes == null) {
notes = getChangeNotes().load();
}
- checkArgument(!notes.containsComment(c),
- "A comment already exists with the same key as the following comment,"
- + " so we cannot insert this comment: %s", c);
+ if (migration.readComments()) {
+ checkArgument(!notes.containsComment(c),
+ "A comment already exists with the same key as the following comment,"
+ + " so we cannot insert this comment: %s", c);
+ }
if (c.getSide() == 0) {
commentsForBase.add(c);
} else {
@@ -248,8 +256,19 @@
draftUpdate.insertComment(c);
}
- private void upsertPublishedComment(PatchLineComment c) {
+ private void upsertPublishedComment(PatchLineComment c) throws OrmException {
verifyComment(c);
+ if (notes == null) {
+ notes = getChangeNotes().load();
+ }
+ // This could allow callers to update a published comment if migration.write
+ // is on and migration.readComments is off because we will not be able to
+ // verify that the comment didn't already exist as a published comment
+ // since we don't have a ReviewDb.
+ if (migration.readComments()) {
+ checkArgument(!notes.containsCommentPublished(c),
+ "Cannot update a comment that has already been published and saved");
+ }
if (c.getSide() == 0) {
commentsForBase.add(c);
} else {
@@ -267,8 +286,12 @@
if (notes == null) {
notes = getChangeNotes().load();
}
- checkArgument(!notes.containsCommentPublished(c),
- "Cannot update a comment that has already been published and saved");
+ // See comment above in upsertPublishedComment() about potential risk with
+ // this check.
+ if (migration.readComments()) {
+ checkArgument(!notes.containsCommentPublished(c),
+ "Cannot update a comment that has already been published and saved");
+ }
if (c.getSide() == 0) {
commentsForBase.add(c);
} else {
@@ -292,7 +315,6 @@
draftUpdate.deleteCommentIfPresent(c);
}
-
private void createDraftUpdateIfNull(PatchLineComment c) throws OrmException {
if (draftUpdate == null) {
draftUpdate = draftUpdateFactory.create(ctl, when);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java
index ede979fe..c2e5dfc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java
@@ -18,11 +18,12 @@
import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST;
import static com.google.gerrit.server.notedb.ChangeNotes.parseException;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.common.primitives.Ints;
+import com.google.gerrit.common.data.AccountInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.CommentRange;
@@ -33,13 +34,15 @@
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import com.google.inject.Singleton;
import org.eclipse.jgit.errors.ConfigInvalidException;
-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.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -48,11 +51,11 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.GitDateFormatter;
+import org.eclipse.jgit.util.GitDateFormatter.Format;
import org.eclipse.jgit.util.GitDateParser;
import org.eclipse.jgit.util.MutableInteger;
import org.eclipse.jgit.util.QuotedString;
import org.eclipse.jgit.util.RawParseUtils;
-import org.eclipse.jgit.util.GitDateFormatter.Format;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
@@ -69,6 +72,7 @@
* Utility functions to parse PatchLineComments out of a note byte array and
* store a list of PatchLineComments in the form of a note (in a byte array).
**/
+@Singleton
public class CommentsInNotesUtil {
private static final String AUTHOR = "Author";
private static final String BASE_PATCH_SET = "Base-for-patch-set";
@@ -79,6 +83,7 @@
private static final String PATCH_SET = "Patch-set";
private static final String REVISION = "Revision";
private static final String UUID = "UUID";
+ private static final int MAX_NOTE_SZ = 25 << 20;
public static NoteMap parseCommentsFromNotes(Repository repo, String refName,
RevWalk walk, Change.Id changeId,
@@ -90,14 +95,16 @@
if (ref == null) {
return null;
}
+
+ ObjectReader reader = walk.getObjectReader();
RevCommit commit = walk.parseCommit(ref.getObjectId());
- NoteMap noteMap = NoteMap.read(walk.getObjectReader(), commit);
+ NoteMap noteMap = NoteMap.read(reader, commit);
for (Note note: noteMap) {
- byte[] bytes = walk.getObjectReader().open(
- note.getData(), Constants.OBJ_BLOB).getBytes();
+ byte[] bytes =
+ reader.open(note.getData(), OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
List<PatchLineComment> result = parseNote(bytes, changeId, status);
- if ((result == null) || (result.isEmpty())) {
+ if (result == null || result.isEmpty()) {
continue;
}
PatchSet.Id psId = result.get(0).getKey().getParentKey().getParentKey();
@@ -232,6 +239,7 @@
if (note[ptr.value] == '\n') {
range.setEndLine(startLine);
+ ptr.value += 1;
return range;
} else if (note[ptr.value] == ':') {
range.setStartLine(startLine);
@@ -368,7 +376,7 @@
private PersonIdent newIdent(Account author, Date when) {
return new PersonIdent(
- author.getFullName(),
+ new AccountInfo(author).getName(anonymousCowardName),
author.getId().get() + "@" + GERRIT_PLACEHOLDER_HOST,
when, serverIdent.getTimeZone());
}
@@ -410,13 +418,15 @@
private final AccountCache accountCache;
private final PersonIdent serverIdent;
+ private final String anonymousCowardName;
- @VisibleForTesting
@Inject
public CommentsInNotesUtil(AccountCache accountCache,
- @GerritPersonIdent PersonIdent serverIdent) {
+ @GerritPersonIdent PersonIdent serverIdent,
+ @AnonymousCowardName String anonymousCowardName) {
this.accountCache = accountCache;
this.serverIdent = serverIdent;
+ this.anonymousCowardName = anonymousCowardName;
}
/**
@@ -454,11 +464,11 @@
checkArgument(psId.equals(currentPsId),
"All comments being added must all have the same PatchSet.Id. The"
+ "comment below does not have the same PatchSet.Id as the others "
- + "(%d).\n%s", psId.toString(), c.toString());
+ + "(%s).\n%s", psId.toString(), c.toString());
checkArgument(side == c.getSide(),
"All comments being added must all have the same side. The"
+ "comment below does not have the same side as the others "
- + "(%d).\n%s", side, c.toString());
+ + "(%s).\n%s", side, c.toString());
String commentFilename =
QuotedString.GIT_PATH.quote(c.getKey().getParentKey().getFileName());
@@ -520,12 +530,10 @@
throws OrmException, IOException {
checkArgument(!allComments.isEmpty(),
"No comments to write; to delete, use removeNoteFromNoteMap().");
- ObjectId commitOID =
+ ObjectId commit =
ObjectId.fromString(allComments.get(0).getRevId().get());
Collections.sort(allComments, ChangeNotes.PatchLineCommentComparator);
- byte[] note = buildNote(allComments);
- ObjectId noteId = inserter.insert(Constants.OBJ_BLOB, note, 0, note.length);
- noteMap.set(commitOID, noteId);
+ noteMap.set(commit, inserter.insert(OBJ_BLOB, buildNote(allComments)));
}
public void removeNote(NoteMap noteMap, RevId commitId)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
index 36f685d..679e272 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
@@ -36,14 +36,14 @@
cfg.setBoolean("notedb", null, "write", true);
cfg.setBoolean("notedb", "patchSetApprovals", "read", true);
cfg.setBoolean("notedb", "changeMessages", "read", true);
- cfg.setBoolean("notedb", "publishedComments", "read", true);
+ cfg.setBoolean("notedb", "comments", "read", true);
return new NotesMigration(cfg);
}
private final boolean write;
private final boolean readPatchSetApprovals;
private final boolean readChangeMessages;
- private final boolean readPublishedComments;
+ private final boolean readComments;
@Inject
NotesMigration(@GerritServerConfig Config cfg) {
@@ -52,8 +52,8 @@
cfg.getBoolean("notedb", "patchSetApprovals", "read", false);
readChangeMessages =
cfg.getBoolean("notedb", "changeMessages", "read", false);
- readPublishedComments =
- cfg.getBoolean("notedb", "publishedComments", "read", false);
+ readComments =
+ cfg.getBoolean("notedb", "comments", "read", false);
}
public boolean write() {
@@ -68,7 +68,7 @@
return readChangeMessages;
}
- public boolean readPublishedComments() {
- return readPublishedComments;
+ public boolean readComments() {
+ return readComments;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index b4337cf..decae6d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -338,7 +338,8 @@
private void loadDrafts(final Map<Patch.Key, Patch> byKey,
final AccountInfoCacheFactory aic, final Account.Id me, final String file)
throws OrmException {
- for (PatchLineComment c : db.patchComments().draftByChangeFileAuthor(changeId, file, me)) {
+ for (PatchLineComment c :
+ plcUtil.draftByChangeFileAuthor(db, control.getNotes(), file, me)) {
if (comments.include(c)) {
aic.want(me);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/BanCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/BanCommit.java
index fab1ed3..930360a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/BanCommit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/BanCommit.java
@@ -51,8 +51,12 @@
}
}
+ private final com.google.gerrit.server.git.BanCommit banCommit;
+
@Inject
- private com.google.gerrit.server.git.BanCommit banCommit;
+ BanCommit(com.google.gerrit.server.git.BanCommit banCommit) {
+ this.banCommit = banCommit;
+ }
@Override
public BanResultInfo apply(ProjectResource rsrc, Input input)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitResource.java
index fc9c807..2543818 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitResource.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitResource.java
@@ -14,22 +14,29 @@
package com.google.gerrit.server.project;
+import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.inject.TypeLiteral;
import org.eclipse.jgit.revwalk.RevCommit;
-public class CommitResource extends ProjectResource {
+public class CommitResource implements RestResource {
public static final TypeLiteral<RestView<CommitResource>> COMMIT_KIND =
new TypeLiteral<RestView<CommitResource>>() {};
+ private final ProjectResource project;
private final RevCommit commit;
- public CommitResource(ProjectControl control, RevCommit commit) {
- super(control);
+ public CommitResource(ProjectResource project, RevCommit commit) {
+ this.project = project;
this.commit = commit;
}
+ public Project.NameKey getProject() {
+ return project.getNameKey();
+ }
+
public RevCommit getCommit() {
return commit;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java
index f27dc6e..6e997f7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java
@@ -19,8 +19,10 @@
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@@ -37,12 +39,15 @@
ChildCollection<ProjectResource, CommitResource> {
private final DynamicMap<RestView<CommitResource>> views;
private final GitRepositoryManager repoManager;
+ private final Provider<ReviewDb> db;
@Inject
public CommitsCollection(DynamicMap<RestView<CommitResource>> views,
- GitRepositoryManager repoManager) {
+ GitRepositoryManager repoManager,
+ Provider<ReviewDb> db) {
this.views = views;
this.repoManager = repoManager;
+ this.db = db;
}
@Override
@@ -65,13 +70,13 @@
RevWalk rw = new RevWalk(repo);
try {
RevCommit commit = rw.parseCommit(objectId);
- if (!parent.getControl().canReadCommit(rw, commit)) {
+ if (!parent.getControl().canReadCommit(db.get(), rw, commit)) {
throw new ResourceNotFoundException(id);
}
for (int i = 0; i < commit.getParentCount(); i++) {
rw.parseCommit(commit.getParent(i));
}
- return new CommitResource(parent.getControl(), commit);
+ return new CommitResource(parent, commit);
} catch (MissingObjectException | IncorrectObjectTypeException e) {
throw new ResourceNotFoundException(id);
} finally {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
index 3dcf9f4..983549d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
@@ -25,6 +25,7 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
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.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -67,6 +68,7 @@
private final Provider<IdentifiedUser> identifiedUser;
private final GitRepositoryManager repoManager;
+ private final Provider<ReviewDb> db;
private final GitReferenceUpdated referenceUpdated;
private final ChangeHooks hooks;
private String ref;
@@ -74,10 +76,12 @@
@Inject
CreateBranch(Provider<IdentifiedUser> identifiedUser,
GitRepositoryManager repoManager,
+ Provider<ReviewDb> db,
GitReferenceUpdated referenceUpdated, ChangeHooks hooks,
@Assisted String ref) {
this.identifiedUser = identifiedUser;
this.repoManager = repoManager;
+ this.db = db;
this.referenceUpdated = referenceUpdated;
this.hooks = hooks;
this.ref = ref;
@@ -129,7 +133,8 @@
}
}
- if (!refControl.canCreate(rw, object, true)) {
+ rw.reset();
+ if (!refControl.canCreate(db.get(), rw, object)) {
throw new AuthException("Cannot create \"" + ref + "\"");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/FilesInCommitCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/FilesInCommitCollection.java
index 4dc4338..f383230 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/FilesInCommitCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/FilesInCommitCollection.java
@@ -40,7 +40,7 @@
@Override
public FileResource parse(CommitResource parent, IdString id)
throws ResourceNotFoundException {
- return new FileResource(parent.getNameKey(), parent.getCommit().getName(),
+ return new FileResource(parent.getProject(), parent.getCommit().getName(),
id.get());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetCommit.java
index d43ea68..a857914 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetCommit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetCommit.java
@@ -17,6 +17,7 @@
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.GitPerson;
import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.CommonConverters;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.PersonIdent;
@@ -30,16 +31,14 @@
@Override
public CommitInfo apply(CommitResource rsrc) {
- RevCommit c = rsrc.getCommit();
- CommitInfo info = toCommitInfo(c);
- return info;
+ return toCommitInfo(rsrc.getCommit());
}
private static CommitInfo toCommitInfo(RevCommit commit) {
CommitInfo info = new CommitInfo();
info.commit = commit.getName();
- info.author = toGitPerson(commit.getAuthorIdent());
- info.committer = toGitPerson(commit.getCommitterIdent());
+ info.author = CommonConverters.toGitPerson(commit.getAuthorIdent());
+ info.committer = CommonConverters.toGitPerson(commit.getCommitterIdent());
info.subject = commit.getShortMessage();
info.message = commit.getFullMessage();
info.parents = new ArrayList<>(commit.getParentCount());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java
index f05ece4..0530a4c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java
@@ -17,8 +17,10 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@@ -34,12 +36,14 @@
@Singleton
public class GetHead implements RestReadView<ProjectResource> {
-
private GitRepositoryManager repoManager;
+ private Provider<ReviewDb> db;
@Inject
- GetHead(GitRepositoryManager repoManager) {
+ GetHead(GitRepositoryManager repoManager,
+ Provider<ReviewDb> db) {
this.repoManager = repoManager;
+ this.db = db;
}
@Override
@@ -61,7 +65,7 @@
RevWalk rw = new RevWalk(repo);
try {
RevCommit commit = rw.parseCommit(head.getObjectId());
- if (rsrc.getControl().canReadCommit(rw, commit)) {
+ if (rsrc.getControl().canReadCommit(db.get(), rw, commit)) {
return head.getObjectId().name();
}
throw new AuthException("not allowed to see HEAD");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetReflog.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetReflog.java
index b6415b5..7e52381 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetReflog.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetReflog.java
@@ -20,12 +20,12 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.args4j.TimestampHandler;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.ReflogEntry;
import org.eclipse.jgit.lib.ReflogReader;
import org.eclipse.jgit.lib.Repository;
@@ -75,7 +75,7 @@
public List<ReflogEntryInfo> apply(BranchResource rsrc) throws AuthException,
ResourceNotFoundException, RepositoryNotFoundException, IOException {
if (!rsrc.getControl().isOwner()) {
- throw new AuthException("no project owner");
+ throw new AuthException("not project owner");
}
Repository repo = repoManager.openRepository(rsrc.getNameKey());
@@ -122,14 +122,7 @@
public ReflogEntryInfo(ReflogEntry e) {
oldId = e.getOldId().getName();
newId = e.getNewId().getName();
-
- PersonIdent ident = e.getWho();
- who = new GitPerson();
- who.name = ident.getName();
- who.email = ident.getEmailAddress();
- who.date = new Timestamp(ident.getWhen().getTime());
- who.tz = ident.getTimeZoneOffset();
-
+ who = CommonConverters.toGitPerson(e.getWho());
comment = e.getComment();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
index a28ee40..8b895f2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
@@ -16,6 +16,7 @@
import com.google.common.base.Predicate;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
@@ -38,14 +39,12 @@
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.util.RegexListSearcher;
import com.google.gerrit.server.util.TreeFormatter;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import dk.brics.automaton.RegExp;
-import dk.brics.automaton.RunAutomaton;
-
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
@@ -449,8 +448,10 @@
private Iterable<Project.NameKey> scan() throws BadRequestException {
if (matchPrefix != null) {
+ checkMatchOptions(matchSubstring == null && matchRegex == null);
return projectCache.byName(matchPrefix);
} else if (matchSubstring != null) {
+ checkMatchOptions(matchPrefix == null && matchRegex == null);
return Iterables.filter(projectCache.all(),
new Predicate<Project.NameKey>() {
public boolean apply(Project.NameKey in) {
@@ -459,32 +460,31 @@
}
});
} else if (matchRegex != null) {
- if (matchRegex.startsWith("^")) {
- matchRegex = matchRegex.substring(1);
- if (matchRegex.endsWith("$") && !matchRegex.endsWith("\\$")) {
- matchRegex = matchRegex.substring(0, matchRegex.length() - 1);
- }
- }
- if (matchRegex.equals(".*")) {
- return projectCache.all();
- }
+ checkMatchOptions(matchPrefix == null && matchSubstring == null);
+ RegexListSearcher<Project.NameKey> searcher;
try {
- final RunAutomaton a =
- new RunAutomaton(new RegExp(matchRegex).toAutomaton());
- return Iterables.filter(projectCache.all(),
- new Predicate<Project.NameKey>() {
- public boolean apply(Project.NameKey in) {
- return a.run(in.get());
- }
- });
+ searcher = new RegexListSearcher<Project.NameKey>(matchRegex) {
+ @Override
+ public String apply(Project.NameKey in) {
+ return in.get();
+ }
+ };
} catch (IllegalArgumentException e) {
throw new BadRequestException(e.getMessage());
}
+ return searcher.search(ImmutableList.copyOf(projectCache.all()));
} else {
return projectCache.all();
}
}
+ private static void checkMatchOptions(boolean cond)
+ throws BadRequestException {
+ if (!cond) {
+ throw new BadRequestException("specify exactly one of p/m/r");
+ }
+ }
+
private void printProjectTree(final PrintWriter stdout,
final TreeMap<Project.NameKey, ProjectNode> treeMap) {
final SortedSet<ProjectNode> sortedNodes = new TreeSet<>();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index 53b7368..3c6fd71 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -30,6 +30,7 @@
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
@@ -37,15 +38,16 @@
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
+import com.google.gerrit.server.git.ChangeCache;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.TagCache;
+import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -148,6 +150,8 @@
private final ChangeControl.AssistedFactory changeControlFactory;
private final PermissionCollection.Factory permissionFilter;
private final Collection<ContributorAgreement> contributorAgreements;
+ private final TagCache tagCache;
+ private final ChangeCache changeCache;
private List<SectionMatcher> allSections;
private List<SectionMatcher> localSections;
@@ -162,11 +166,15 @@
PermissionCollection.Factory permissionFilter,
GitRepositoryManager repoManager,
ChangeControl.AssistedFactory changeControlFactory,
+ TagCache tagCache,
+ ChangeCache changeCache,
@CanonicalWebUrl @Nullable String canonicalWebUrl,
@Assisted CurrentUser who,
@Assisted ProjectState ps) {
this.repoManager = repoManager;
this.changeControlFactory = changeControlFactory;
+ this.tagCache = tagCache;
+ this.changeCache = changeCache;
this.uploadGroups = uploadGroups;
this.receiveGroups = receiveGroups;
this.permissionFilter = permissionFilter;
@@ -263,12 +271,12 @@
/** Can this user see all the refs in this projects? */
public boolean allRefsAreVisible() {
- return allRefsAreVisibleExcept(Collections.<String> emptySet());
+ return allRefsAreVisible(Collections.<String> emptySet());
}
- public boolean allRefsAreVisibleExcept(Set<String> except) {
+ public boolean allRefsAreVisible(Set<String> ignore) {
return user instanceof InternalUser
- || canPerformOnAllRefs(Permission.READ, except);
+ || canPerformOnAllRefs(Permission.READ, ignore);
}
/** Is this user a project owner? Ownership does not imply {@link #isVisible()} */
@@ -426,7 +434,7 @@
return false;
}
- private boolean canPerformOnAllRefs(String permission, Set<String> except) {
+ private boolean canPerformOnAllRefs(String permission, Set<String> ignore) {
boolean canPerform = false;
Set<String> patterns = allRefPatterns(permission);
if (patterns.contains(AccessSection.ALL)) {
@@ -437,7 +445,7 @@
for (final String pattern : patterns) {
if (controlForRef(pattern).canPerform(permission)) {
canPerform = true;
- } else if (except.contains(pattern)) {
+ } else if (ignore.contains(pattern)) {
continue;
} else {
return false;
@@ -513,40 +521,33 @@
return false;
}
- public boolean canReadCommit(RevWalk rw, RevCommit commit) {
- if (controlForRef("refs/*").canPerform(Permission.READ)) {
- return true;
- }
-
- Project.NameKey projName = state.getProject().getNameKey();
+ public boolean canReadCommit(ReviewDb db, RevWalk rw, RevCommit commit) {
try {
- Repository repo = repoManager.openRepository(projName);
+ Repository repo = openRepository();
try {
- RefDatabase refDb = repo.getRefDatabase();
- List<Ref> allRefs = Lists.newLinkedList();
- allRefs.addAll(refDb.getRefs(Constants.R_HEADS).values());
- allRefs.addAll(refDb.getRefs(Constants.R_TAGS).values());
- List<Ref> canReadRefs = Lists.newLinkedList();
- for (Ref r : allRefs) {
- if (controlForRef(r.getName()).canPerform(Permission.READ)) {
- canReadRefs.add(r);
- }
- }
-
- if (!canReadRefs.isEmpty() && IncludedInResolver.includedInOne(
- repo, rw, commit, canReadRefs)) {
- return true;
- }
+ return isMergedIntoVisibleRef(repo, db, rw, commit, repo.getAllRefs());
} finally {
repo.close();
}
} catch (IOException e) {
- String msg =
- String.format(
- "Cannot verify permissions to commit object %s in repository %s",
- commit.name(), projName.get());
+ String msg = String.format(
+ "Cannot verify permissions to commit object %s in repository %s",
+ commit.name(), getProject().getNameKey());
log.error(msg, e);
+ return false;
}
- return false;
+ }
+
+ boolean isMergedIntoVisibleRef(Repository repo, ReviewDb db, RevWalk rw,
+ RevCommit commit, Map<String, Ref> unfilteredRefs) throws IOException {
+ VisibleRefFilter filter =
+ new VisibleRefFilter(tagCache, changeCache, repo, this, db, true);
+ Map<String, Ref> refs = filter.filter(unfilteredRefs, true);
+ return !refs.isEmpty()
+ && IncludedInResolver.includedInOne(repo, rw, commit, refs.values());
+ }
+
+ Repository openRepository() throws IOException {
+ return repoManager.openRepository(getProject().getNameKey());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 00ecee3..f13b411 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -24,6 +24,7 @@
import com.google.gerrit.common.errors.InvalidNameException;
import com.google.gerrit.extensions.api.projects.ProjectState;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
@@ -31,12 +32,16 @@
import dk.brics.automaton.RegExp;
+import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevTag;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.ArrayList;
@@ -49,6 +54,8 @@
/** Manages access control for Git references (aka branches, tags). */
public class RefControl {
+ private static final Logger log = LoggerFactory.getLogger(RefControl.class);
+
private final ProjectControl projectControl;
private final String refName;
@@ -231,12 +238,13 @@
/**
* Determines whether the user can create a new Git ref.
*
- * @param rw revision pool {@code object} was parsed in.
+ * @param db db for checking change visibility.
+ * @param rw revision pool {@code object} was parsed in; must be reset before
+ * calling this method.
* @param object the object the user will start the reference with.
- * @param existsOnServer the object exists on server or not.
* @return {@code true} if the user specified can create a new Git ref
*/
- public boolean canCreate(RevWalk rw, RevObject object, boolean existsOnServer) {
+ public boolean canCreate(ReviewDb db, RevWalk rw, RevObject object) {
if (!canWrite()) {
return false;
}
@@ -255,10 +263,24 @@
}
if (object instanceof RevCommit) {
- return admin
- || (owner && !isBlocked(Permission.CREATE))
- || (canPerform(Permission.CREATE) && (!existsOnServer && canUpdate() || projectControl
- .canReadCommit(rw, (RevCommit) object)));
+ if (admin || (owner && !isBlocked(Permission.CREATE))) {
+ // Admin or project owner; bypass visibility check.
+ return true;
+ } else if (!canPerform(Permission.CREATE)) {
+ // No create permissions.
+ return false;
+ } else if (canUpdate()) {
+ // If the user has push permissions, they can create the ref regardless
+ // of whether they are pushing any new objects along with the create.
+ return true;
+ } else if (isMergedIntoBranchOrTag(db, rw, (RevCommit) object)) {
+ // If the user has no push permissions, check whether the object is
+ // merged into a branch or tag readable by this user. If so, they are
+ // not effectively "pushing" more objects, so they can create the ref
+ // even if they don't have push permission.
+ return true;
+ }
+ return false;
} else if (object instanceof RevTag) {
final RevTag tag = (RevTag) object;
try {
@@ -297,6 +319,28 @@
}
}
+ private boolean isMergedIntoBranchOrTag(ReviewDb db, RevWalk rw,
+ RevCommit commit) {
+ try {
+ Repository repo = projectControl.openRepository();
+ try {
+ Map<String, Ref> refs =
+ repo.getRefDatabase().getRefs(Constants.R_HEADS);
+ refs.putAll(repo.getRefDatabase().getRefs(Constants.R_TAGS));
+ return projectControl.isMergedIntoVisibleRef(
+ repo, db, rw, commit, refs);
+ } finally {
+ repo.close();
+ }
+ } catch (IOException e) {
+ String msg = String.format(
+ "Cannot verify permissions to commit object %s in repository %s",
+ commit.name(), projectControl.getProject().getNameKey());
+ log.error(msg, e);
+ }
+ return false;
+ }
+
/**
* Determines whether the user can delete the Git ref controlled by this
* object.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java
index 8ce6fa3..37d96ea 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java
@@ -30,7 +30,8 @@
new InvalidProvider<ReviewDb>(), //
new InvalidProvider<ChangeQueryRewriter>(), //
null, null, null, null, null, null, null, //
- null, null, null, null, null, null, null, null, null, null), null);
+ null, null, null, null, null, null, null, null, null, null, null),
+ null);
private static final QueryRewriter.Definition<ChangeData, BasicChangeRewrites> mydef =
new QueryRewriter.Definition<ChangeData, BasicChangeRewrites>(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index ed64015..0310824 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -35,6 +35,7 @@
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
@@ -154,7 +155,7 @@
*/
static ChangeData createForTest(Change.Id id, int currentPatchSetId) {
ChangeData cd = new ChangeData(null, null, null, null, null,
- null, null, null, null, id);
+ null, null, null, null, null, id);
cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId));
return cd;
}
@@ -166,6 +167,7 @@
private final ChangeNotes.Factory notesFactory;
private final ApprovalsUtil approvalsUtil;
private final ChangeMessagesUtil cmUtil;
+ private final PatchLineCommentsUtil plcUtil;
private final PatchListCache patchListCache;
private final NotesMigration notesMigration;
private final Change.Id legacyId;
@@ -179,7 +181,7 @@
private ListMultimap<PatchSet.Id, PatchSetApproval> allApprovals;
private List<PatchSetApproval> currentApprovals;
private Map<Integer, List<String>> files = new HashMap<>();
- private Collection<PatchLineComment> comments;
+ private Collection<PatchLineComment> publishedComments;
private CurrentUser visibleTo;
private ChangeControl changeControl;
private List<ChangeMessage> messages;
@@ -194,6 +196,7 @@
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
+ PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache,
NotesMigration notesMigration,
@Assisted ReviewDb db,
@@ -205,6 +208,7 @@
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil;
+ this.plcUtil = plcUtil;
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
legacyId = id;
@@ -218,6 +222,7 @@
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
+ PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache,
NotesMigration notesMigration,
@Assisted ReviewDb db,
@@ -229,6 +234,7 @@
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil;
+ this.plcUtil = plcUtil;
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
legacyId = c.getId();
@@ -243,6 +249,7 @@
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
+ PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache,
NotesMigration notesMigration,
@Assisted ReviewDb db,
@@ -254,6 +261,7 @@
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil;
+ this.plcUtil = plcUtil;
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
legacyId = c.getChange().getId();
@@ -519,12 +527,12 @@
return approvalsUtil.getReviewers(notes(), approvals().values());
}
- public Collection<PatchLineComment> comments()
+ public Collection<PatchLineComment> publishedComments()
throws OrmException {
- if (comments == null) {
- comments = db.patchComments().byChange(legacyId).toList();
+ if (publishedComments == null) {
+ publishedComments = plcUtil.publishedByChange(db, notes());
}
- return comments;
+ return publishedComments;
}
public List<ChangeMessage> messages()
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index ac7b9ae..fabe61c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -25,6 +25,7 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupBackend;
@@ -146,6 +147,7 @@
final CapabilityControl.Factory capabilityControlFactory;
final ChangeControl.GenericFactory changeControlGenericFactory;
final ChangeData.Factory changeDataFactory;
+ final PatchLineCommentsUtil plcUtil;
final AccountResolver accountResolver;
final GroupBackend groupBackend;
final AllProjectsName allProjectsName;
@@ -168,6 +170,7 @@
CapabilityControl.Factory capabilityControlFactory,
ChangeControl.GenericFactory changeControlGenericFactory,
ChangeData.Factory changeDataFactory,
+ PatchLineCommentsUtil plcUtil,
AccountResolver accountResolver,
GroupBackend groupBackend,
AllProjectsName allProjectsName,
@@ -187,6 +190,7 @@
this.capabilityControlFactory = capabilityControlFactory;
this.changeControlGenericFactory = changeControlGenericFactory;
this.changeDataFactory = changeDataFactory;
+ this.plcUtil = plcUtil;
this.accountResolver = accountResolver;
this.groupBackend = groupBackend;
this.allProjectsName = allProjectsName;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasDraftByPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasDraftByPredicate.java
index 53d2bbd..ebb7389 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasDraftByPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasDraftByPredicate.java
@@ -33,7 +33,8 @@
private final Arguments args;
private final Account.Id accountId;
- HasDraftByPredicate(Arguments args, Account.Id accountId) {
+ HasDraftByPredicate(Arguments args,
+ Account.Id accountId) {
super(ChangeQueryBuilder.FIELD_DRAFTBY, accountId.toString());
this.args = args;
this.accountId = accountId;
@@ -41,20 +42,16 @@
@Override
public boolean match(final ChangeData object) throws OrmException {
- for (PatchLineComment c : object.comments()) {
- if (c.getStatus() == PatchLineComment.Status.DRAFT
- && c.getAuthor().equals(accountId)) {
- return true;
- }
- }
- return false;
+ return !args.plcUtil
+ .draftByChangeAuthor(args.db.get(), object.notes(), accountId)
+ .isEmpty();
}
@Override
public ResultSet<ChangeData> read() throws OrmException {
Set<Change.Id> ids = new HashSet<>();
- for (PatchLineComment sc : args.db.get().patchComments()
- .draftByAuthor(accountId)) {
+ for (PatchLineComment sc :
+ args.plcUtil.draftByAuthor(args.db.get(), accountId)) {
ids.add(sc.getKey().getParentKey().getParentKey().getParentKey());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
index 8889f53..f6a6108 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
@@ -366,7 +366,7 @@
eventFactory.addComments(c, d.messages());
if (includePatchSets) {
for (PatchSetAttribute attribute : c.patchSets) {
- eventFactory.addPatchSetComments(attribute, d.comments());
+ eventFactory.addPatchSetComments(attribute, d.publishedComments());
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
index d073002..3d6f8b4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
@@ -16,76 +16,21 @@
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.RegexPredicate;
+import com.google.gerrit.server.util.RegexListSearcher;
import com.google.gwtorm.server.OrmException;
-import dk.brics.automaton.Automaton;
-import dk.brics.automaton.RegExp;
-import dk.brics.automaton.RunAutomaton;
-
-import java.util.Collections;
import java.util.List;
class RegexPathPredicate extends RegexPredicate<ChangeData> {
- private final RunAutomaton pattern;
-
- private final String prefixBegin;
- private final String prefixEnd;
- private final int prefixLen;
- private final boolean prefixOnly;
-
RegexPathPredicate(String fieldName, String re) {
super(ChangeField.PATH, re);
-
- if (re.startsWith("^")) {
- re = re.substring(1);
- }
-
- if (re.endsWith("$") && !re.endsWith("\\$")) {
- re = re.substring(0, re.length() - 1);
- }
-
- Automaton automaton = new RegExp(re).toAutomaton();
- prefixBegin = automaton.getCommonPrefix();
- prefixLen = prefixBegin.length();
-
- if (0 < prefixLen) {
- char max = (char) (prefixBegin.charAt(prefixLen - 1) + 1);
- prefixEnd = prefixBegin.substring(0, prefixLen - 1) + max;
- prefixOnly = re.equals(prefixBegin + ".*");
- } else {
- prefixEnd = "";
- prefixOnly = false;
- }
-
- pattern = prefixOnly ? null : new RunAutomaton(automaton);
}
@Override
public boolean match(ChangeData object) throws OrmException {
List<String> files = object.currentFilePaths();
if (files != null) {
- int begin, end;
-
- if (0 < prefixLen) {
- begin = find(files, prefixBegin);
- end = find(files, prefixEnd);
- } else {
- begin = 0;
- end = files.size();
- }
-
- if (prefixOnly) {
- return begin < end;
- }
-
- while (begin < end) {
- if (pattern.run(files.get(begin++))) {
- return true;
- }
- }
-
- return false;
-
+ return RegexListSearcher.ofStrings(getValue()).hasMatch(files);
} else {
// The ChangeData can't do expensive lookups right now. Bypass
// them and include the result anyway. We might be able to do
@@ -95,11 +40,6 @@
}
}
- private static int find(List<String> files, String p) {
- int r = Collections.binarySearch(files, p);
- return r < 0 ? -(r + 1) : r;
- }
-
@Override
public int getCost() {
return 1;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/RegexListSearcher.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/RegexListSearcher.java
new file mode 100644
index 0000000..4b0fd35
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/RegexListSearcher.java
@@ -0,0 +1,112 @@
+// Copyright (C) 2014 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.util;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.primitives.Chars;
+
+import dk.brics.automaton.Automaton;
+import dk.brics.automaton.RegExp;
+import dk.brics.automaton.RunAutomaton;
+
+import java.util.Collections;
+import java.util.List;
+
+/** Helper to search sorted lists for elements matching a regex. */
+public abstract class RegexListSearcher<T> implements Function<T, String> {
+ public static RegexListSearcher<String> ofStrings(String re) {
+ return new RegexListSearcher<String>(re) {
+ @Override
+ public String apply(String in) {
+ return in;
+ }
+ };
+ }
+
+ private final RunAutomaton pattern;
+
+ private final String prefixBegin;
+ private final String prefixEnd;
+ private final int prefixLen;
+ private final boolean prefixOnly;
+
+ public RegexListSearcher(String re) {
+ if (re.startsWith("^")) {
+ re = re.substring(1);
+ }
+
+ if (re.endsWith("$") && !re.endsWith("\\$")) {
+ re = re.substring(0, re.length() - 1);
+ }
+
+ Automaton automaton = new RegExp(re).toAutomaton();
+ prefixBegin = automaton.getCommonPrefix();
+ prefixLen = prefixBegin.length();
+
+ if (0 < prefixLen) {
+ char max = Chars.checkedCast(prefixBegin.charAt(prefixLen - 1) + 1);
+ prefixEnd = prefixBegin.substring(0, prefixLen - 1) + max;
+ prefixOnly = re.equals(prefixBegin + ".*");
+ } else {
+ prefixEnd = "";
+ prefixOnly = false;
+ }
+
+ pattern = prefixOnly ? null : new RunAutomaton(automaton);
+ }
+
+ public Iterable<T> search(List<T> list) {
+ checkNotNull(list);
+ int begin, end;
+
+ if (0 < prefixLen) {
+ // Assumes many consecutive elements may have the same prefix, so the cost
+ // of two binary searches is less than iterating to find the endpoints.
+ begin = find(list, prefixBegin);
+ end = find(list, prefixEnd);
+ } else {
+ begin = 0;
+ end = list.size();
+ }
+
+ if (prefixOnly) {
+ return begin < end ? list.subList(begin, end) : ImmutableList.<T> of();
+ }
+
+ return Iterables.filter(
+ list.subList(begin, end),
+ new Predicate<T>() {
+ @Override
+ public boolean apply(T in) {
+ return pattern.run(RegexListSearcher.this.apply(in));
+ }
+ });
+ }
+
+ public boolean hasMatch(List<T> list) {
+ return !Iterables.isEmpty(search(list));
+ }
+
+ private int find(List<T> list, String p) {
+ int r = Collections.binarySearch(Lists.transform(list, this), p);
+ return r < 0 ? -(r + 1) : r;
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java
index da687be..a8880a1 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java
@@ -21,6 +21,7 @@
import static org.easymock.EasyMock.replay;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import com.google.common.base.Objects;
@@ -43,6 +44,7 @@
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.PatchLineCommentAccess;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
@@ -60,7 +62,6 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.TimeUtil;
@@ -74,6 +75,8 @@
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Injector;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.util.Providers;
@@ -104,7 +107,7 @@
public static @GerritServerConfig Config noteDbEnabled() {
@GerritServerConfig Config cfg = new Config();
cfg.setBoolean("notedb", null, "write", true);
- cfg.setBoolean("notedb", "publishedComments", "read", true);
+ cfg.setBoolean("notedb", "comments", "read", true);
return cfg;
}
@@ -118,15 +121,23 @@
private PatchLineComment plc1;
private PatchLineComment plc2;
private PatchLineComment plc3;
+ private PatchLineComment plc4;
+ private PatchLineComment plc5;
private IdentifiedUser changeOwner;
@Before
public void setUp() throws Exception {
@SuppressWarnings("unchecked")
- final DynamicMap<RestView<CommentResource>> views =
+ final DynamicMap<RestView<CommentResource>> commentViews =
createMock(DynamicMap.class);
- final TypeLiteral<DynamicMap<RestView<CommentResource>>> viewsType =
+ final TypeLiteral<DynamicMap<RestView<CommentResource>>> commentViewsType =
new TypeLiteral<DynamicMap<RestView<CommentResource>>>() {};
+ @SuppressWarnings("unchecked")
+ final DynamicMap<RestView<DraftResource>> draftViews =
+ createMock(DynamicMap.class);
+ final TypeLiteral<DynamicMap<RestView<DraftResource>>> draftViewsType =
+ new TypeLiteral<DynamicMap<RestView<DraftResource>>>() {};
+
final AccountInfo.Loader.Factory alf =
createMock(AccountInfo.Loader.Factory.class);
final ReviewDb db = createMock(ReviewDb.class);
@@ -139,10 +150,23 @@
@SuppressWarnings("unused")
InMemoryRepository repo = repoManager.createRepository(project);
+ Account co = new Account(new Account.Id(1), TimeUtil.nowTs());
+ co.setFullName("Change Owner");
+ co.setPreferredEmail("change@owner.com");
+ accountCache.put(co);
+ final Account.Id ownerId = co.getId();
+
+ Account ou = new Account(new Account.Id(2), TimeUtil.nowTs());
+ ou.setFullName("Other Account");
+ ou.setPreferredEmail("other@account.com");
+ accountCache.put(ou);
+ Account.Id otherUserId = ou.getId();
+
AbstractModule mod = new AbstractModule() {
@Override
protected void configure() {
- bind(viewsType).toInstance(views);
+ bind(commentViewsType).toInstance(commentViews);
+ bind(draftViewsType).toInstance(draftViews);
bind(AccountInfo.Loader.Factory.class).toInstance(alf);
bind(ReviewDb.class).toProvider(Providers.<ReviewDb>of(db));
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(config);
@@ -162,28 +186,16 @@
bind(PersonIdent.class).annotatedWith(GerritPersonIdent.class)
.toInstance(serverIdent);
}
+
+ @Provides
+ @Singleton
+ CurrentUser getCurrentUser(IdentifiedUser.GenericFactory userFactory) {
+ return userFactory.create(ownerId);
+ }
};
injector = Guice.createInjector(mod);
- NotesMigration migration = injector.getInstance(NotesMigration.class);
- allUsers = injector.getInstance(AllUsersNameProvider.class);
- repoManager.createRepository(allUsers.get());
-
- plcUtil = new PatchLineCommentsUtil(migration);
-
- Account co = new Account(new Account.Id(1), TimeUtil.nowTs());
- co.setFullName("Change Owner");
- co.setPreferredEmail("change@owner.com");
- accountCache.put(co);
- Account.Id ownerId = co.getId();
-
- Account ou = new Account(new Account.Id(2), TimeUtil.nowTs());
- ou.setFullName("Other Account");
- ou.setPreferredEmail("other@account.com");
- accountCache.put(ou);
- Account.Id otherUserId = ou.getId();
-
IdentifiedUser.GenericFactory userFactory =
injector.getInstance(IdentifiedUser.GenericFactory.class);
changeOwner = userFactory.create(ownerId);
@@ -199,6 +211,10 @@
expect(alf.create(true)).andReturn(accountLoader).anyTimes();
replay(accountLoader, alf);
+ allUsers = injector.getInstance(AllUsersNameProvider.class);
+ repoManager.createRepository(allUsers.get());
+ plcUtil = injector.getInstance(PatchLineCommentsUtil.class);
+
PatchLineCommentAccess plca = createMock(PatchLineCommentAccess.class);
expect(db.patchComments()).andReturn(plca).anyTimes();
@@ -208,7 +224,7 @@
PatchSet.Id psId2 = new PatchSet.Id(change.getId(), 2);
PatchSet ps2 = new PatchSet(psId2);
- long timeBase = TimeUtil.nowMs();
+ long timeBase = TimeUtil.roundToSecond(TimeUtil.nowTs()).getTime();
plc1 = newPatchLineComment(psId1, "Comment1", null,
"FileOne.txt", Side.REVISION, 3, ownerId, timeBase,
"First Comment", new CommentRange(1, 2, 3, 4));
@@ -221,32 +237,56 @@
"FileOne.txt", Side.PARENT, 3, ownerId, timeBase + 2000,
"First Parent Comment", new CommentRange(1, 2, 3, 4));
plc3.setRevId(new RevId("CDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEF"));
+ plc4 = newPatchLineComment(psId2, "Comment4", null, "FileOne.txt",
+ Side.REVISION, 3, ownerId, timeBase + 3000, "Second Comment",
+ new CommentRange(1, 2, 3, 4), Status.DRAFT);
+ plc4.setRevId(new RevId("BCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDE"));
+ plc5 = newPatchLineComment(psId2, "Comment5", null, "FileOne.txt",
+ Side.REVISION, 5, ownerId, timeBase + 4000, "Third Comment",
+ new CommentRange(3, 4, 5, 6), Status.DRAFT);
+ plc5.setRevId(new RevId("BCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDE"));
List<PatchLineComment> commentsByOwner = Lists.newArrayList();
commentsByOwner.add(plc1);
commentsByOwner.add(plc3);
List<PatchLineComment> commentsByReviewer = Lists.newArrayList();
commentsByReviewer.add(plc2);
+ List<PatchLineComment> drafts = Lists.newArrayList();
+ drafts.add(plc4);
+ drafts.add(plc5);
plca.upsert(commentsByOwner);
expectLastCall().anyTimes();
plca.upsert(commentsByReviewer);
expectLastCall().anyTimes();
+ plca.upsert(drafts);
+ expectLastCall().anyTimes();
expect(plca.publishedByPatchSet(psId1))
.andAnswer(results(plc1, plc2, plc3)).anyTimes();
expect(plca.publishedByPatchSet(psId2))
.andAnswer(results()).anyTimes();
+ expect(plca.draftByPatchSetAuthor(psId1, ownerId))
+ .andAnswer(results()).anyTimes();
+ expect(plca.draftByPatchSetAuthor(psId2, ownerId))
+ .andAnswer(results(plc4, plc5)).anyTimes();
+ expect(plca.byChange(change.getId()))
+ .andAnswer(results(plc1, plc2, plc3, plc4, plc5)).anyTimes();
replay(db, plca);
ChangeUpdate update = newUpdate(change, changeOwner);
update.setPatchSetId(psId1);
- plcUtil.addPublishedComments(db, update, commentsByOwner);
+ plcUtil.upsertComments(db, update, commentsByOwner);
update.commit();
update = newUpdate(change, otherUser);
update.setPatchSetId(psId1);
- plcUtil.addPublishedComments(db, update, commentsByReviewer);
+ plcUtil.upsertComments(db, update, commentsByReviewer);
+ update.commit();
+
+ update = newUpdate(change, changeOwner);
+ update.setPatchSetId(psId2);
+ plcUtil.upsertComments(db, update, drafts);
update.commit();
ChangeControl ctl = stubChangeControl(change);
@@ -286,6 +326,37 @@
assertGetComment(injector, revRes1, null, "BadComment");
}
+ @Test
+ public void testListDrafts() throws Exception {
+ // test ListDrafts for patch set 1
+ assertListDrafts(injector, revRes1,
+ Collections.<String, ArrayList<PatchLineComment>> emptyMap());
+
+ // test ListDrafts for patch set 2
+ assertListDrafts(injector, revRes2, ImmutableMap.of(
+ "FileOne.txt", Lists.newArrayList(plc4, plc5)));
+ }
+
+ @Test
+ public void testPatchLineCommentsUtilByCommentStatus() throws OrmException {
+ List<PatchLineComment> publishedActual = plcUtil.publishedByChange(
+ injector.getInstance(ReviewDb.class), revRes2.getNotes());
+ List<PatchLineComment> draftActual = plcUtil.draftByChange(
+ injector.getInstance(ReviewDb.class), revRes2.getNotes());
+ List<PatchLineComment> publishedExpected =
+ Lists.newArrayList(plc1, plc2, plc3);
+ List<PatchLineComment> draftExpected =
+ Lists.newArrayList(plc4, plc5);
+ assertEquals(publishedExpected.size(), publishedActual.size());
+ assertEquals(draftExpected.size(), draftActual.size());
+ for (PatchLineComment c : draftExpected) {
+ assertTrue(draftActual.contains(c));
+ }
+ for (PatchLineComment c : publishedExpected) {
+ assertTrue(publishedActual.contains(c));
+ }
+ }
+
private static IAnswer<ResultSet<PatchLineComment>> results(
final PatchLineComment... comments) {
return new IAnswer<ResultSet<PatchLineComment>>() {
@@ -305,7 +376,7 @@
fail("Expected no comment");
}
CommentInfo actual = getComment.apply(commentRes);
- assertComment(expected, actual);
+ assertComment(expected, actual, true);
} catch (ResourceNotFoundException e) {
if (expected != null) {
fail("Expected to find comment");
@@ -330,17 +401,42 @@
assertNotNull(actualComments);
assertEquals(expectedComments.size(), actualComments.size());
for (int i = 0; i < expectedComments.size(); i++) {
- assertComment(expectedComments.get(i), actualComments.get(i));
+ assertComment(expectedComments.get(i), actualComments.get(i), true);
}
}
}
- private static void assertComment(PatchLineComment plc, CommentInfo ci) {
+ private static void assertListDrafts(Injector inj, RevisionResource res,
+ Map<String, ArrayList<PatchLineComment>> expected) throws Exception {
+ Drafts drafts = inj.getInstance(Drafts.class);
+ RestReadView<RevisionResource> listView =
+ (RestReadView<RevisionResource>) drafts.list();
+ @SuppressWarnings("unchecked")
+ Map<String, List<CommentInfo>> actual =
+ (Map<String, List<CommentInfo>>) listView.apply(res);
+ assertNotNull(actual);
+ assertEquals(expected.size(), actual.size());
+ assertEquals(expected.keySet(), actual.keySet());
+ for (Map.Entry<String, ArrayList<PatchLineComment>> entry : expected.entrySet()) {
+ List<PatchLineComment> expectedComments = entry.getValue();
+ List<CommentInfo> actualComments = actual.get(entry.getKey());
+ assertNotNull(actualComments);
+ assertEquals(expectedComments.size(), actualComments.size());
+ for (int i = 0; i < expectedComments.size(); i++) {
+ assertComment(expectedComments.get(i), actualComments.get(i), false);
+ }
+ }
+ }
+
+ private static void assertComment(PatchLineComment plc, CommentInfo ci,
+ boolean isPublished) {
assertEquals(plc.getKey().get(), ci.id);
assertEquals(plc.getParentUuid(), ci.inReplyTo);
assertEquals(plc.getMessage(), ci.message);
- assertNotNull(ci.author);
- assertEquals(plc.getAuthor(), ci.author._id);
+ if (isPublished) {
+ assertNotNull(ci.author);
+ assertEquals(plc.getAuthor(), ci.author._id);
+ }
assertEquals(plc.getLine(), (int) ci.line);
assertEquals(plc.getSide() == 0 ? Side.PARENT : Side.REVISION,
Objects.firstNonNull(ci.side, Side.REVISION));
@@ -351,7 +447,8 @@
private static PatchLineComment newPatchLineComment(PatchSet.Id psId,
String uuid, String inReplyToUuid, String filename, Side side, int line,
- Account.Id authorId, long millis, String message, CommentRange range) {
+ Account.Id authorId, long millis, String message, CommentRange range,
+ PatchLineComment.Status status) {
Patch.Key p = new Patch.Key(psId, filename);
PatchLineComment.Key id = new PatchLineComment.Key(p, uuid);
PatchLineComment plc =
@@ -359,8 +456,15 @@
plc.setMessage(message);
plc.setRange(range);
plc.setSide(side == Side.PARENT ? (short) 0 : (short) 1);
- plc.setStatus(Status.PUBLISHED);
+ plc.setStatus(status);
plc.setWrittenOn(new Timestamp(millis));
return plc;
}
+
+ private static PatchLineComment newPatchLineComment(PatchSet.Id psId,
+ String uuid, String inReplyToUuid, String filename, Side side, int line,
+ Account.Id authorId, long millis, String message, CommentRange range) {
+ return newPatchLineComment(psId, uuid, inReplyToUuid, filename, side, line,
+ authorId, millis, message, range, Status.PUBLISHED);
+ }
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java
index 50e5764..7090f0d 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java
@@ -26,8 +26,8 @@
new FakeQueryBuilder.Definition<>(
FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments(null, null, null, null, null, null,
- null, null, null, null, null, null, null, null, indexes, null, null,
- null, null),
+ null, null, null, null, null, null, null, null, null, indexes, null,
+ null, null, null),
null);
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 0ef57c1..72e3838 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -62,13 +62,12 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.VersionedMetaData.BatchMetaDataUpdate;
import com.google.gerrit.server.group.SystemGroupBackend;
-import com.google.gerrit.server.notedb.CommentsInNotesUtil;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.TimeUtil;
-import com.google.gerrit.testutil.TestChanges;
import com.google.gerrit.testutil.FakeAccountCache;
import com.google.gerrit.testutil.FakeRealm;
import com.google.gerrit.testutil.InMemoryRepositoryManager;
+import com.google.gerrit.testutil.TestChanges;
import com.google.gwtorm.client.KeyUtil;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.StandardKeyEncoder;
@@ -77,13 +76,16 @@
import com.google.inject.util.Providers;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.notes.Note;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
import org.joda.time.DateTime;
import org.joda.time.DateTimeUtils;
import org.joda.time.DateTimeUtils.MillisProvider;
@@ -108,6 +110,7 @@
private InMemoryRepositoryManager repoManager;
private InMemoryRepository repo;
private FakeAccountCache accountCache;
+ private IdentifiedUser.GenericFactory userFactory;
private AllUsersNameProvider allUsers;
private IdentifiedUser changeOwner;
private IdentifiedUser otherUser;
@@ -161,8 +164,7 @@
}
});
- IdentifiedUser.GenericFactory userFactory =
- injector.getInstance(IdentifiedUser.GenericFactory.class);
+ userFactory = injector.getInstance(IdentifiedUser.GenericFactory.class);
allUsers = injector.getInstance(AllUsersNameProvider.class);
repoManager.createRepository(allUsers.get());
changeOwner = userFactory.create(co.getId());
@@ -326,6 +328,34 @@
}
@Test
+ public void anonymousUser() throws Exception {
+ Account anon = new Account(new Account.Id(3), TimeUtil.nowTs());
+ accountCache.put(anon);
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, userFactory.create(anon.getId()));
+ update.setChangeMessage("Comment on the change.");
+ update.commit();
+
+ RevWalk walk = new RevWalk(repo);
+ try {
+ RevCommit commit = walk.parseCommit(update.getRevision());
+ walk.parseBody(commit);
+ assertEquals("Update patch set 1\n"
+ + "\n"
+ + "Comment on the change.\n"
+ + "\n"
+ + "Patch-set: 1\n",
+ commit.getFullMessage());
+
+ PersonIdent author = commit.getAuthorIdent();
+ assertEquals("Anonymous Coward (3)", author.getName());
+ assertEquals("3@gerrit", author.getEmailAddress());
+ } finally {
+ walk.release();
+ }
+ }
+
+ @Test
public void submitWithErrorMessage() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
@@ -667,6 +697,58 @@
}
@Test
+ public void multipleUpdatesAcrossRefs() throws Exception {
+ Change c1 = newChange();
+ ChangeUpdate update1 = newUpdate(c1, changeOwner);
+ update1.putApproval("Verified", (short) 1);
+
+ Change c2 = newChange();
+ ChangeUpdate update2 = newUpdate(c2, otherUser);
+ update2.putApproval("Code-Review", (short) 2);
+
+ BatchMetaDataUpdate batch1 = null;
+ BatchMetaDataUpdate batch2 = null;
+
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+ try {
+ batch1 = update1.openUpdateInBatch(bru);
+ batch1.write(update1, new CommitBuilder());
+ batch1.commit();
+ assertNull(repo.getRef(update1.getRefName()));
+
+ batch2 = update2.openUpdateInBatch(bru);
+ batch2.write(update2, new CommitBuilder());
+ batch2.commit();
+ assertNull(repo.getRef(update2.getRefName()));
+ } finally {
+ if (batch1 != null) {
+ batch1.close();
+ }
+ if (batch2 != null) {
+ batch2.close();
+ }
+ }
+
+ List<ReceiveCommand> cmds = bru.getCommands();
+ assertEquals(2, cmds.size());
+ assertEquals(update1.getRefName(), cmds.get(0).getRefName());
+ assertEquals(update2.getRefName(), cmds.get(1).getRefName());
+
+ RevWalk rw = new RevWalk(repo);
+ try {
+ bru.execute(rw, NullProgressMonitor.INSTANCE);
+ } finally {
+ rw.release();
+ }
+
+ assertEquals(ReceiveCommand.Result.OK, cmds.get(0).getResult());
+ assertEquals(ReceiveCommand.Result.OK, cmds.get(1).getResult());
+
+ assertNotNull(repo.getRef(update1.getRefName()));
+ assertNotNull(repo.getRef(update2.getRefName()));
+ }
+
+ @Test
public void changeMessageOnePatchSet() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
@@ -867,7 +949,9 @@
public void patchLineCommentNotesFormatSide1() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
- String uuid = "uuid";
+ String uuid1 = "uuid1";
+ String uuid2 = "uuid2";
+ String uuid3 = "uuid3";
String message1 = "comment 1";
String message2 = "comment 2";
String message3 = "comment 3";
@@ -878,7 +962,7 @@
PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment comment1 = newPublishedPatchLineComment(psId, "file1",
- uuid, range1, range1.getEndLine(), otherUser, null, time1, message1,
+ uuid1, range1, range1.getEndLine(), otherUser, null, time1, message1,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment1);
@@ -887,7 +971,7 @@
update = newUpdate(c, otherUser);
CommentRange range2 = new CommentRange(2, 1, 3, 1);
PatchLineComment comment2 = newPublishedPatchLineComment(psId, "file1",
- uuid, range2, range2.getEndLine(), otherUser, null, time2, message2,
+ uuid2, range2, range2.getEndLine(), otherUser, null, time2, message2,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment2);
@@ -896,7 +980,7 @@
update = newUpdate(c, otherUser);
CommentRange range3 = new CommentRange(3, 1, 4, 1);
PatchLineComment comment3 = newPublishedPatchLineComment(psId, "file2",
- uuid, range3, range3.getEndLine(), otherUser, null, time3, message3,
+ uuid3, range3, range3.getEndLine(), otherUser, null, time3, message3,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment3);
@@ -920,14 +1004,14 @@
+ "1:1-2:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n"
+ "Author: Other Account <2@gerrit>\n"
- + "UUID: uuid\n"
+ + "UUID: uuid1\n"
+ "Bytes: 9\n"
+ "comment 1\n"
+ "\n"
+ "2:1-3:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n"
+ "Author: Other Account <2@gerrit>\n"
- + "UUID: uuid\n"
+ + "UUID: uuid2\n"
+ "Bytes: 9\n"
+ "comment 2\n"
+ "\n"
@@ -936,7 +1020,7 @@
+ "3:1-4:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time3) + "\n"
+ "Author: Other Account <2@gerrit>\n"
- + "UUID: uuid\n"
+ + "UUID: uuid3\n"
+ "Bytes: 9\n"
+ "comment 3\n"
+ "\n",
@@ -947,7 +1031,8 @@
public void patchLineCommentNotesFormatSide0() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
- String uuid = "uuid";
+ String uuid1 = "uuid1";
+ String uuid2 = "uuid2";
String message1 = "comment 1";
String message2 = "comment 2";
CommentRange range1 = new CommentRange(1, 1, 2, 1);
@@ -956,7 +1041,7 @@
PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment comment1 = newPublishedPatchLineComment(psId, "file1",
- uuid, range1, range1.getEndLine(), otherUser, null, time1, message1,
+ uuid1, range1, range1.getEndLine(), otherUser, null, time1, message1,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment1);
@@ -965,7 +1050,7 @@
update = newUpdate(c, otherUser);
CommentRange range2 = new CommentRange(2, 1, 3, 1);
PatchLineComment comment2 = newPublishedPatchLineComment(psId, "file1",
- uuid, range2, range2.getEndLine(), otherUser, null, time2, message2,
+ uuid2, range2, range2.getEndLine(), otherUser, null, time2, message2,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment2);
@@ -989,14 +1074,14 @@
+ "1:1-2:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n"
+ "Author: Other Account <2@gerrit>\n"
- + "UUID: uuid\n"
+ + "UUID: uuid1\n"
+ "Bytes: 9\n"
+ "comment 1\n"
+ "\n"
+ "2:1-3:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n"
+ "Author: Other Account <2@gerrit>\n"
- + "UUID: uuid\n"
+ + "UUID: uuid2\n"
+ "Bytes: 9\n"
+ "comment 2\n"
+ "\n",
@@ -1009,7 +1094,8 @@
throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
- String uuid = "uuid";
+ String uuid1 = "uuid1";
+ String uuid2 = "uuid2";
String messageForBase = "comment for base";
String messageForPS = "comment for ps";
CommentRange range = new CommentRange(1, 1, 2, 1);
@@ -1017,7 +1103,7 @@
PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment commentForBase =
- newPublishedPatchLineComment(psId, "filename", uuid,
+ newPublishedPatchLineComment(psId, "filename", uuid1,
range, range.getEndLine(), otherUser, null, now, messageForBase,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
@@ -1026,7 +1112,7 @@
update = newUpdate(c, otherUser);
PatchLineComment commentForPS =
- newPublishedPatchLineComment(psId, "filename", uuid,
+ newPublishedPatchLineComment(psId, "filename", uuid2,
range, range.getEndLine(), otherUser, null, now, messageForPS,
(short) 1, "abcd4567abcd4567abcd4567abcd4567abcd4567");
update.setPatchSetId(psId);
@@ -1050,7 +1136,8 @@
@Test
public void patchLineCommentMultipleOnePatchsetOneFile() throws Exception {
Change c = newChange();
- String uuid = "uuid";
+ String uuid1 = "uuid1";
+ String uuid2 = "uuid2";
CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id psId = c.currentPatchSetId();
String filename = "filename";
@@ -1060,7 +1147,7 @@
Timestamp timeForComment1 = TimeUtil.nowTs();
Timestamp timeForComment2 = TimeUtil.nowTs();
PatchLineComment comment1 = newPublishedPatchLineComment(psId, filename,
- uuid, range, range.getEndLine(), otherUser, null, timeForComment1,
+ uuid1, range, range.getEndLine(), otherUser, null, timeForComment1,
"comment 1", side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment1);
@@ -1068,7 +1155,7 @@
update = newUpdate(c, otherUser);
PatchLineComment comment2 = newPublishedPatchLineComment(psId, filename,
- uuid, range, range.getEndLine(), otherUser, null, timeForComment2,
+ uuid2, range, range.getEndLine(), otherUser, null, timeForComment2,
"comment 2", side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment2);
@@ -1342,6 +1429,34 @@
assertTrue(notes.getDraftPsComments(otherUserId).values().isEmpty());
}
+ @Test
+ public void patchLineCommentNoRange() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, otherUser);
+ String uuid = "uuid";
+ String messageForBase = "comment for base";
+ Timestamp now = TimeUtil.nowTs();
+ PatchSet.Id psId = c.currentPatchSetId();
+
+ PatchLineComment commentForBase =
+ newPublishedPatchLineComment(psId, "filename", uuid,
+ null, 1, otherUser, null, now, messageForBase,
+ (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
+ update.setPatchSetId(psId);
+ update.upsertComment(commentForBase);
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ Multimap<PatchSet.Id, PatchLineComment> commentsForBase =
+ notes.getBaseComments();
+ Multimap<PatchSet.Id, PatchLineComment> commentsForPs =
+ notes.getPatchSetComments();
+
+ assertTrue(commentsForPs.isEmpty());
+ assertEquals(commentForBase,
+ Iterables.getOnlyElement(commentsForBase.get(psId)));
+ }
+
private Change newChange() {
return TestChanges.newChange(project, changeOwner);
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java
new file mode 100644
index 0000000..c3fbccd
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java
@@ -0,0 +1,176 @@
+// Copyright (C) 2014 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.project;
+
+import static com.google.gerrit.common.data.Permission.READ;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.server.project.Util.allow;
+import static com.google.gerrit.server.project.Util.deny;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.google.gerrit.lifecycle.LifecycleManager;
+import com.google.gerrit.reviewdb.client.Account;
+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.AccountManager;
+import com.google.gerrit.server.account.AuthRequest;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.schema.SchemaCreator;
+import com.google.gerrit.testutil.InMemoryDatabase;
+import com.google.gerrit.testutil.InMemoryModule;
+import com.google.gerrit.testutil.InMemoryRepositoryManager;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Unit tests for {@link ProjectControl}. */
+public class ProjectControlTest {
+ @Inject private AccountManager accountManager;
+ @Inject private IdentifiedUser.RequestFactory userFactory;
+ @Inject private InMemoryDatabase schemaFactory;
+ @Inject private InMemoryRepositoryManager repoManager;
+ @Inject private ProjectControl.GenericFactory projectControlFactory;
+ @Inject private SchemaCreator schemaCreator;
+
+ private LifecycleManager lifecycle;
+ private ReviewDb db;
+ private TestRepository<InMemoryRepository> repo;
+ private ProjectConfig project;
+ private IdentifiedUser user;
+
+ @Before
+ public void setUp() throws Exception {
+ Injector injector = Guice.createInjector(new InMemoryModule());
+ injector.injectMembers(this);
+ lifecycle = new LifecycleManager();
+ lifecycle.add(injector);
+ lifecycle.start();
+
+ db = schemaFactory.open();
+ schemaCreator.create(db);
+ Account.Id userId = accountManager.authenticate(AuthRequest.forUser("user"))
+ .getAccountId();
+ user = userFactory.create(userId);
+
+ Project.NameKey name = new Project.NameKey("project");
+ InMemoryRepository inMemoryRepo = repoManager.createRepository(name);
+ project = new ProjectConfig(name);
+ project.load(inMemoryRepo);
+ repo = new TestRepository<InMemoryRepository>(inMemoryRepo);
+ }
+
+ @After
+ public void tearDown() {
+ if (repo != null) {
+ repo.getRepository().close();
+ }
+ if (lifecycle != null) {
+ lifecycle.stop();
+ }
+ if (db != null) {
+ db.close();
+ }
+ InMemoryDatabase.drop(schemaFactory);
+ }
+
+ @Test
+ public void canReadCommitWhenAllRefsVisible() throws Exception {
+ allow(project, READ, REGISTERED_USERS, "refs/*");
+ ObjectId id = repo.branch("master").commit().create();
+ ProjectControl pc = newProjectControl();
+ RevWalk rw = repo.getRevWalk();
+ assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(id)));
+ }
+
+ @Test
+ public void canReadCommitIfRefVisible() throws Exception {
+ allow(project, READ, REGISTERED_USERS, "refs/heads/branch1");
+ deny(project, READ, REGISTERED_USERS, "refs/heads/branch2");
+
+ ObjectId id1 = repo.branch("branch1").commit().create();
+ ObjectId id2 = repo.branch("branch2").commit().create();
+
+ ProjectControl pc = newProjectControl();
+ RevWalk rw = repo.getRevWalk();
+ assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(id1)));
+ assertFalse(pc.canReadCommit(db, rw, rw.parseCommit(id2)));
+ }
+
+ @Test
+ public void canReadCommitIfReachableFromVisibleRef() throws Exception {
+ allow(project, READ, REGISTERED_USERS, "refs/heads/branch1");
+ deny(project, READ, REGISTERED_USERS, "refs/heads/branch2");
+
+ RevCommit parent1 = repo.commit().create();
+ repo.branch("branch1").commit().parent(parent1).create();
+
+ RevCommit parent2 = repo.commit().create();
+ repo.branch("branch2").commit().parent(parent2).create();
+
+ ProjectControl pc = newProjectControl();
+ RevWalk rw = repo.getRevWalk();
+ assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(parent1)));
+ assertFalse(pc.canReadCommit(db, rw, rw.parseCommit(parent2)));
+ }
+
+ @Test
+ public void cannotReadAfterRollbackWithRestrictedRead() throws Exception {
+ allow(project, READ, REGISTERED_USERS, "refs/heads/branch1");
+
+ RevCommit parent1 = repo.commit().create();
+ ObjectId id1 = repo.branch("branch1").commit().parent(parent1).create();
+
+ ProjectControl pc = newProjectControl();
+ RevWalk rw = repo.getRevWalk();
+ assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(parent1)));
+ assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(id1)));
+
+ repo.branch("branch1").update(parent1);
+ assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(parent1)));
+ assertFalse(pc.canReadCommit(db, rw, rw.parseCommit(id1)));
+ }
+
+ @Test
+ public void canReadAfterRollbackWithAllRefsVisible() throws Exception {
+ allow(project, READ, REGISTERED_USERS, "refs/*");
+
+ RevCommit parent1 = repo.commit().create();
+ ObjectId id1 = repo.branch("branch1").commit().parent(parent1).create();
+
+ ProjectControl pc = newProjectControl();
+ RevWalk rw = repo.getRevWalk();
+ assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(parent1)));
+ assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(id1)));
+
+ repo.branch("branch1").update(parent1);
+ assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(parent1)));
+ assertFalse(pc.canReadCommit(db, rw, rw.parseCommit(id1)));
+ }
+
+ private ProjectControl newProjectControl() throws Exception {
+ return projectControlFactory.controlFor(project.getName(), user);
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
index ac1e0d7..ba292d6 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
@@ -61,6 +61,7 @@
import com.google.inject.util.Providers;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
@@ -178,7 +179,7 @@
private final CapabilityControl.Factory capabilityControlFactory;
private final ChangeControl.AssistedFactory changeControlFactory;
private final PermissionCollection.Factory sectionSorter;
- private final GitRepositoryManager repoManager;
+ private final InMemoryRepositoryManager repoManager;
private final AllProjectsName allProjectsName =
new AllProjectsName("All-Projects");
@@ -283,21 +284,26 @@
injector.getInstance(ChangeControl.AssistedFactory.class);
}
- public void add(ProjectConfig pc) {
+ public InMemoryRepository add(ProjectConfig pc) {
PrologEnvironment.Factory envFactory = null;
ProjectControl.AssistedFactory projectControlFactory = null;
RulesCache rulesCache = null;
SitePaths sitePaths = null;
List<CommentLinkInfo> commentLinks = null;
+ InMemoryRepository repo;
try {
- repoManager.createRepository(pc.getProject().getNameKey());
- } catch (IOException e) {
+ repo = repoManager.createRepository(pc.getName());
+ if (pc.getProject() == null) {
+ pc.load(repo);
+ }
+ } catch (IOException | ConfigInvalidException e) {
throw new RuntimeException(e);
}
- all.put(pc.getProject().getNameKey(), new ProjectState(sitePaths,
+ all.put(pc.getName(), new ProjectState(sitePaths,
projectCache, allProjectsName, projectControlFactory, envFactory,
repoManager, rulesCache, commentLinks, pc));
+ return repo;
}
public ProjectControl user(ProjectConfig local, AccountGroup.UUID... memberOf) {
@@ -310,8 +316,8 @@
return new ProjectControl(Collections.<AccountGroup.UUID> emptySet(),
Collections.<AccountGroup.UUID> emptySet(), projectCache,
- sectionSorter, repoManager, changeControlFactory, canonicalWebUrl,
- new MockUser(name, memberOf), newProjectState(local));
+ sectionSorter, repoManager, changeControlFactory, null, null,
+ canonicalWebUrl, new MockUser(name, memberOf), newProjectState(local));
}
private ProjectState newProjectState(ProjectConfig local) {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/util/RegexListSearcherTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/util/RegexListSearcherTest.java
new file mode 100644
index 0000000..8f73005
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/util/RegexListSearcherTest.java
@@ -0,0 +1,84 @@
+// Copyright (C) 2014 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.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Ordering;
+
+import org.junit.Test;
+
+import java.util.List;
+
+public class RegexListSearcherTest {
+ private static final List<String> EMPTY = ImmutableList.of();
+
+ @Test
+ public void emptyList() {
+ assertSearchReturns(EMPTY, "pat", EMPTY);
+ }
+
+ @Test
+ public void hasMatch() {
+ List<String> list = ImmutableList.of("bar", "foo", "quux");
+ assertTrue(RegexListSearcher.ofStrings("foo").hasMatch(list));
+ assertFalse(RegexListSearcher.ofStrings("xyz").hasMatch(list));
+ }
+
+ @Test
+ public void anchors() {
+ List<String> list = ImmutableList.of("foo");
+ assertSearchReturns(list, "^f.*", list);
+ assertSearchReturns(list, "^f.*o$", list);
+ assertSearchReturns(list, "f.*o$", list);
+ assertSearchReturns(list, "f.*o$", list);
+ assertSearchReturns(EMPTY, "^.*\\$", list);
+ }
+
+ @Test
+ public void noCommonPrefix() {
+ List<String> list = ImmutableList.of("bar", "foo", "quux");
+ assertSearchReturns(ImmutableList.of("foo"), "f.*", list);
+ assertSearchReturns(ImmutableList.of("foo"), ".*o.*", list);
+ assertSearchReturns(ImmutableList.of("bar", "foo", "quux"), ".*[aou].*",
+ list);
+ }
+
+ @Test
+ public void commonPrefix() {
+ List<String> list = ImmutableList.of(
+ "bar",
+ "baz",
+ "foo1",
+ "foo2",
+ "foo3",
+ "quux");
+ assertSearchReturns(ImmutableList.of("bar", "baz"), "b.*", list);
+ assertSearchReturns(ImmutableList.of("foo1", "foo2"), "foo[12]", list);
+ assertSearchReturns(ImmutableList.of("foo1", "foo2", "foo3"), "foo.*",
+ list);
+ assertSearchReturns(ImmutableList.of("quux"), "q.*", list);
+ }
+
+ private void assertSearchReturns(List<?> expected, String re,
+ List<String> inputs) {
+ assertTrue(Ordering.natural().isOrdered(inputs));
+ assertEquals(expected,
+ ImmutableList.copyOf(RegexListSearcher.ofStrings(re).search(inputs)));
+ }
+}
diff --git a/tools/eclipse/BUCK b/tools/eclipse/BUCK
index 4e76b029..2cc66a0 100644
--- a/tools/eclipse/BUCK
+++ b/tools/eclipse/BUCK
@@ -11,6 +11,7 @@
'//gerrit-main:main_lib',
'//gerrit-patch-jgit:jgit_patch_tests',
'//gerrit-plugin-gwtui:gwtui-api-lib',
+ '//gerrit-reviewdb:client_tests',
'//gerrit-server:server',
'//gerrit-server:server_tests',
'//lib/asciidoctor:asciidoc_lib',
