Merge branch 'stable-2.15'

* stable-2.15:
  AbstractQueryChangesTest: Add byMessageMixedCase test
  ElasticContainer: replace the deprecated override
  ElasticVersion: remove redundant private modifier
  ElasticIndexVersionManager: remove unneeded type
  Elasticsearch: make package-private usage explicit
  MyPasswordScreen: Fix default text in password field
  Move: Return the modified change in JSON response
  Elasticsearch: fix changes index type for bulk calls
  RevisionIT: Add test for posting review comment on non-existing file
  PostReview: Use correct patch set ID to reject comment on non-existing file
  Don't fail on removing star labels from change without star labels
  AbstractQueryAccountsTest: Add byDeletedAccount to ensure consistency
  AbstractQueryGroupsTest: Add byDeletedGroup to achieve higher coverage
  Elastic{Account,Group}Index: Move the static Logger right after class
  Elastic{Account,Change,Group}Index: Make names/types explicitly private
  {Account,Change,Group}Mapping: Set package-privacy
  Docs intro-gerrit-walkthrough: Rewrite for v2.15 PG UI
  Add integration test classes for "ssh index" commands
  ActionJson: Add workInProgress to ChangeInfo copy method
  Allow to toggle ready flag by project owners
  Allow to toggle WIP flag by project owners
  Upgrade JGit to 4.9.2.201712150930-r.15-g5fe8e31d4
  Bazel: Harmonize names of external repositories

The changes done in I4598a6fb9 ("Allow to toggle ready flag by project
owners") and I259a170ed ("Allow to toggle WIP flag by project owners") are
adjusted to replace usage of ProjectControl with PermissionBackend.

Change-Id: I82443ea0e4f2d5a3db2169bbbe129a10520adc0a
This commit is contained in:
David Pursehouse 2018-06-13 13:08:35 +09:00
commit 7a88969052
43 changed files with 411 additions and 77 deletions

Binary file not shown.

Before

Width:  |  Height:  |  Size: 82 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 65 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 87 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 41 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 89 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 67 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 49 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 37 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 60 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 28 KiB

View File

@ -4,7 +4,7 @@ To understand how Gerrit works, let's follow a change through its entire
life cycle. This example uses a Gerrit server configured as follows:
* *Hostname*: gerrithost
* *HTTP interface port*: 8080
* *HTTP interface port*: 80
* *SSH interface port*: 29418
In this walkthrough, we'll follow two developers, Max and Hannah, as they make
@ -52,19 +52,20 @@ follows:
----
$ <work>
$ git commit
[master 9651f22] Change to a proper, yeast based pizza dough.
1 files changed, 3 insertions(+), 2 deletions(-)
[master 3cc9e62] Change to a proper, yeast based pizza dough.
1 file changed, 10 insertions(+), 5 deletions(-)
$ git push origin HEAD:refs/for/master
Counting objects: 5, done.
Counting objects: 3, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 542 bytes, done.
Writing objects: 100% (3/3), 532 bytes | 0 bytes/s, done.
Total 3 (delta 0), reused 0 (delta 0)
remote: Processing changes: new: 1, done
remote:
remote: New Changes:
remote: http://gerrithost:8080/68
remote: http://gerrithost/#/c/RecipeBook/+/702 Change to a proper, yeast based pizza dough.
remote:
To ssh://gerrithost:29418/RecipeBook.git
To ssh://gerrithost:29418/RecipeBook
* [new branch] HEAD -> refs/for/master
----
@ -79,7 +80,7 @@ review this commit. Clicking on that link takes him to a screen similar to
the following.
.Gerrit Code Review Screen
image::images/intro-quick-new-review.jpg[Gerrit Review Screen]
image::images/intro-quick-new-review.png[Gerrit Review Screen]
This is the Gerrit code review screen, where other contributors can review
his change. Max can also perform tasks such as:
@ -109,14 +110,12 @@ offers other ways for reviewers to find changes, including:
Because Max added Hannah as a reviewer, she receives an email telling her about
his change. She opens up the Gerrit code review screen and selects Max's change.
.Gerrit Code Review Screen
image::images/intro-quick-new-review.jpg[Gerrit Review Screen]
Notice the two "Need" lines:
Notice the *Label status* section above:
----
* Need Verified
* Need Code-Review
Label Status Needs label:
* Code-Review
* Verified
----
These two lines indicate what checks must be completed before the change is
@ -147,13 +146,13 @@ link:user-review-ui.html#reply[summary] comments.
Hannah opts to view the change using Gerrit's side-by-side view:
.Side By Side Patch View
image::images/intro-quick-review-line-comment.jpg[Adding a Comment]
image::images/intro-quick-review-line-comment.png[Adding a Comment]
Hannah reviews the change and is ready to provide her feedback. She clicks the
*Review* button on the change screen. This allows her to vote on the change.
*REPLY* button on the change screen. This allows her to vote on the change.
.Reviewing the Change
image::images/intro-quick-reviewing-the-change.jpg[Reviewing the Change]
image::images/intro-quick-reviewing-the-change.png[Reviewing the Change]
For Hannah and Max's team, a code review vote is a numerical score between -2
and 2. The possible options are:
@ -175,7 +174,7 @@ link:config-project-config.html[Project Configuration File Format] topic.
Hannah notices a possible issue with Max's change, so she selects a `-1` vote.
She uses the *Cover Message* text box to provide Max with some additional
feedback. When she is satisfied with her review, Hannah clicks the
*Publish Comments* button. At this point, her vote and cover message become
*SEND* button. At this point, her vote and cover message become
visible to to all users.
== Reworking the Change
@ -193,18 +192,21 @@ workflow for updating a commit:
$ <checkout first commit>
$ <rework>
$ git commit --amend
[master 30a6f44] Change to a proper, yeast based pizza dough.
Date: Fri Jun 8 16:28:23 2018 +0200
1 file changed, 10 insertions(+), 5 deletions(-)
$ git push origin HEAD:refs/for/master
Counting objects: 5, done.
Counting objects: 3, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 546 bytes, done.
Writing objects: 100% (3/3), 528 bytes | 0 bytes/s, done.
Total 3 (delta 0), reused 0 (delta 0)
remote: Processing changes: updated: 1, done
remote:
remote: Updated Changes:
remote: http://gerrithost:8080/68
remote: http://gerrithost/#/c/RecipeBook/+/702 Change to a proper, yeast based pizza dough.
remote:
To ssh://gerrithost:29418/RecipeBook.git
To ssh://gerrithost:29418/RecipeBook
* [new branch] HEAD -> refs/for/master
----
@ -212,13 +214,10 @@ Notice that the output of this command is slightly different from Max's first
commit. This time, the output verifies that the change was updated.
Having uploaded the reworked commit, Max can go back to the Gerrit web
interface and look at his change.
.Reviewing the Rework
image::images/intro-quick-review-2-patches.jpg[Reviewing the Rework]
Notice that there are now two patch sets associated with this change: the
initial submission and the rework.
interface, look at his change and diff the first patch set with his rework in
the second one. Once he has verified that the rework follows Hannahs
recommendation he presses the *DONE* button to let Hannah know that she can
review the changes.
When Hannah next looks at Max's change, she sees that he incorporated her
feedback. The change looks good to her, so she changes her vote to a `+2`.
@ -254,7 +253,7 @@ NOTE: The Verifier can be the same person as the code reviewer or a
different person entirely.
.Verifying the Change
image::images/intro-quick-verifying.jpg[Verifying the Change]
image::images/intro-quick-verifying.png[Verifying the Change]
Unlike the code review check, the verify check is pass/fail. Hannah can provide
a score of either `+1` or `-1`. A change must have at least one `+1` and no
@ -266,7 +265,7 @@ submitted.
== Submitting the Change
Max is now ready to submit his change. He opens up the change in the Code Review
screen and clicks the *Publish and Submit* button.
screen and clicks the *SUBMIT* button.
At this point, Max's change is merged into the repository's master branch and
becomes an accepted part of the project.

View File

@ -548,6 +548,9 @@ request.
----
Alternatively, click *Ready* from the Change screen.
Only change owners, project owners and site administrators can mark changes as
`work-in-progress` and `ready`.
[[wip-polygerrit]]
In the new PolyGerrit UI, you can mark a change as WIP, by selecting *WIP* from
the *More* menu. The Change screen updates with a yellow header, indicating that

View File

@ -2145,7 +2145,8 @@ Only the change owner, a project owner, or an administrator may fix changes.
'POST /changes/link:#change-id[\{change-id\}]/wip'
--
Marks the change as not ready for review yet.
Marks the change as not ready for review yet. Changes may only be marked not
ready by the owner, project owners or site administrators.
The request body does not need to include a
link:#work-in-progress-input[WorkInProgressInput] entity if no review comment
@ -2173,7 +2174,8 @@ notifying *OWNER* instead of *ALL*.
'POST /changes/link:#change-id[\{change-id\}]/ready'
--
Marks the change as ready for review (set WIP property to false).
Marks the change as ready for review (set WIP property to false). Changes may
only be marked ready by the owner, project owners or site administrators.
Activates notifications of reviewer. The request body does not need
to include a link:#work-in-progress-input[WorkInProgressInput] entity

View File

@ -306,6 +306,9 @@ flag from a change on push, explicitly specify the `ready` option:
git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master%ready
----
Only change owners, project owners and site administrators can specify
`work-in-progress` and `ready` options on push.
[[message]]
==== Message

View File

@ -129,6 +129,8 @@ public interface AccountConstants extends Constants {
String buttonGeneratePassword();
String revokePassword();
String linkObtainPassword();
String linkEditFullName();

View File

@ -74,6 +74,7 @@ confirmSetUserNameTitle = Confirm Setting the Username
confirmSetUserName = Setting the Username is permanent. Are you sure?
buttonClearPassword = Clear Password
buttonGeneratePassword = Generate Password
revokePassword = (click 'Generate Password' to revoke an old password)
linkObtainPassword = Obtain Password
linkEditFullName = Edit
linkReloadContact = Reload

View File

@ -49,7 +49,7 @@ public class MyPasswordScreen extends SettingsScreen {
return;
}
password = new CopyableLabel("(click 'generate' to revoke an old password)");
password = new CopyableLabel(Util.C.revokePassword());
password.addStyleName(Gerrit.RESOURCES.css().accountPassword());
generatePassword = new Button(Util.C.buttonGeneratePassword());

View File

@ -118,7 +118,8 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
SitePaths sitePaths,
Schema<V> schema,
ElasticRestClientProvider client,
String indexName) {
String indexName,
String indexType) {
this.sitePaths = sitePaths;
this.schema = schema;
this.gson = new GsonBuilder().setFieldNamingPolicy(LOWER_CASE_WITH_UNDERSCORES).create();
@ -126,7 +127,16 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
this.indexName = cfg.getIndexName(indexName, schema.getVersion());
this.indexNameRaw = indexName;
this.client = client;
this.type = client.adapter().getType(indexName);
this.type = client.adapter().getType(indexType);
}
AbstractElasticIndex(
ElasticConfiguration cfg,
SitePaths sitePaths,
Schema<V> schema,
ElasticRestClientProvider client,
String indexName) {
this(cfg, sitePaths, schema, client, indexName, indexName);
}
@Override

View File

@ -46,14 +46,14 @@ import org.elasticsearch.client.Response;
public class ElasticAccountIndex extends AbstractElasticIndex<Account.Id, AccountState>
implements AccountIndex {
static class AccountMapping {
MappingProperties accounts;
final MappingProperties accounts;
AccountMapping(Schema<AccountState> schema, ElasticQueryAdapter adapter) {
this.accounts = ElasticMapping.createMapping(schema, adapter);
}
}
static final String ACCOUNTS = "accounts";
private static final String ACCOUNTS = "accounts";
private final AccountMapping mapping;
private final Provider<AccountCache> accountCache;

View File

@ -76,9 +76,9 @@ import org.elasticsearch.client.Response;
class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
implements ChangeIndex {
static class ChangeMapping {
MappingProperties changes;
MappingProperties openChanges;
MappingProperties closedChanges;
final MappingProperties changes;
final MappingProperties openChanges;
final MappingProperties closedChanges;
ChangeMapping(Schema<ChangeData> schema, ElasticQueryAdapter adapter) {
MappingProperties mapping = ElasticMapping.createMapping(schema, adapter);
@ -88,9 +88,9 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
}
}
static final String CHANGES = "changes";
static final String OPEN_CHANGES = "open_" + CHANGES;
static final String CLOSED_CHANGES = "closed_" + CHANGES;
private static final String CHANGES = "changes";
private static final String OPEN_CHANGES = "open_" + CHANGES;
private static final String CLOSED_CHANGES = "closed_" + CHANGES;
private final ChangeMapping mapping;
private final Provider<ReviewDb> db;

View File

@ -80,11 +80,11 @@ class ElasticConfiguration {
}
}
public Config getConfig() {
Config getConfig() {
return cfg;
}
public String getIndexName(String name, int schemaVersion) {
String getIndexName(String name, int schemaVersion) {
return String.format("%s%s_%04d", prefix, name, schemaVersion);
}

View File

@ -44,14 +44,14 @@ import org.elasticsearch.client.Response;
public class ElasticGroupIndex extends AbstractElasticIndex<AccountGroup.UUID, InternalGroup>
implements GroupIndex {
static class GroupMapping {
MappingProperties groups;
final MappingProperties groups;
GroupMapping(Schema<InternalGroup> schema, ElasticQueryAdapter adapter) {
this.groups = ElasticMapping.createMapping(schema, adapter);
}
}
static final String GROUPS = "groups";
private static final String GROUPS = "groups";
private final GroupMapping mapping;
private final Provider<GroupCache> groupCache;

View File

@ -63,7 +63,7 @@ public class ElasticIndexVersionManager extends VersionManager {
logger.atWarning().log("Unrecognized version in index %s: %s", def.getName(), version);
continue;
}
versions.put(v, new Version<V>(null, v, true, cfg.getReady(def.getName(), v)));
versions.put(v, new Version<>(null, v, true, cfg.getReady(def.getName(), v)));
}
} catch (IOException e) {
logger.atSevere().withCause(e).log("Error scanning index: %s", def.getName());

View File

@ -34,7 +34,7 @@ import java.time.Instant;
public class ElasticQueryBuilder {
protected <T> QueryBuilder toQueryBuilder(Predicate<T> p) throws QueryParseException {
<T> QueryBuilder toQueryBuilder(Predicate<T> p) throws QueryParseException {
if (p instanceof AndPredicate) {
return and(p);
} else if (p instanceof OrPredicate) {

View File

@ -25,7 +25,7 @@ public enum ElasticVersion {
private final String version;
private final Pattern pattern;
private ElasticVersion(String version) {
ElasticVersion(String version) {
this.version = version;
this.pattern = Pattern.compile(version);
}

View File

@ -139,6 +139,7 @@ public class ActionJson {
copy.stars = changeInfo.stars;
copy.submitted = changeInfo.submitted;
copy.submitter = changeInfo.submitter;
copy.workInProgress = changeInfo.workInProgress;
copy.id = changeInfo.id;
return copy;
}

View File

@ -23,7 +23,7 @@ import static com.google.gerrit.reviewdb.client.RefNames.isConfigRef;
import static com.google.gerrit.server.change.HashtagsUtil.cleanupHashtag;
import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN;
import static com.google.gerrit.server.git.receive.ReceiveConstants.COMMAND_REJECTION_MESSAGE_FOOTER;
import static com.google.gerrit.server.git.receive.ReceiveConstants.ONLY_OWNER_CAN_MODIFY_WIP;
import static com.google.gerrit.server.git.receive.ReceiveConstants.ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP;
import static com.google.gerrit.server.git.receive.ReceiveConstants.PUSH_OPTION_SKIP_VALIDATION;
import static com.google.gerrit.server.git.receive.ReceiveConstants.SAME_CHANGE_ID_IN_MULTIPLE_CHANGES;
import static com.google.gerrit.server.git.validators.CommitValidators.NEW_PATCHSET_PATTERN;
@ -2528,8 +2528,10 @@ class ReceiveCommits {
if (magicBranch != null
&& (magicBranch.workInProgress || magicBranch.ready)
&& magicBranch.workInProgress != change.isWorkInProgress()
&& !user.getAccountId().equals(change.getOwner())) {
reject(inputCommand, ONLY_OWNER_CAN_MODIFY_WIP);
&& (!user.getAccountId().equals(change.getOwner())
&& !permissions.test(ProjectPermission.WRITE_CONFIG)
&& !permissionBackend.user(user).test(GlobalPermission.ADMINISTRATE_SERVER))) {
reject(inputCommand, ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP);
return false;
}

View File

@ -20,8 +20,8 @@ public final class ReceiveConstants {
public static final String PUSH_OPTION_SKIP_VALIDATION = "skip-validation";
@VisibleForTesting
public static final String ONLY_OWNER_CAN_MODIFY_WIP =
"only change owner can modify Work-in-Progress";
public static final String ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP =
"only change owner or project owner can modify Work-in-Progress";
static final String COMMAND_REJECTION_MESSAGE_FOOTER =
"Please read the documentation and contact an administrator\n"

View File

@ -21,6 +21,7 @@ import static com.google.gerrit.server.query.change.ChangeData.asChanges;
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.api.changes.MoveInput;
@ -145,12 +146,13 @@ public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, Chan
}
projectCache.checkedGet(project).checkStatePermitsWrite();
Op op = new Op(input);
try (BatchUpdate u =
updateFactory.create(dbProvider.get(), project, caller, TimeUtil.nowTs())) {
u.addOp(change.getId(), new Op(input));
u.addOp(change.getId(), op);
u.execute();
}
return json.noOptions().format(project, rsrc.getId());
return json.noOptions().format(op.getChange());
}
private class Op implements BatchUpdateOp {
@ -163,6 +165,11 @@ public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, Chan
this.input = input;
}
@Nullable
public Change getChange() {
return change;
}
@Override
public boolean updateChange(ChangeContext ctx)
throws OrmException, ResourceConflictException, IOException {

View File

@ -572,7 +572,7 @@ public class PostReview
Set<String> revisionFilePaths = getAffectedFilePaths(revision);
for (Map.Entry<String, List<T>> entry : commentsPerPath.entrySet()) {
String path = entry.getKey();
PatchSet.Id patchSetId = revision.getChange().currentPatchSetId();
PatchSet.Id patchSetId = revision.getPatchSet().getId();
ensurePathRefersToAvailableOrMagicFile(path, revisionFilePaths, patchSetId);
List<T> comments = entry.getValue();

View File

@ -33,6 +33,7 @@ import com.google.gerrit.server.change.WorkInProgressOp.Input;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
@ -66,7 +67,11 @@ public class SetReadyForReview extends RetryingRestModifyView<ChangeResource, In
throws RestApiException, UpdateException, PermissionBackendException {
Change change = rsrc.getChange();
if (!rsrc.isUserOwner()
&& !permissionBackend.currentUser().test(GlobalPermission.ADMINISTRATE_SERVER)) {
&& !permissionBackend.currentUser().test(GlobalPermission.ADMINISTRATE_SERVER)
&& !permissionBackend
.currentUser()
.project(rsrc.getProject())
.test(ProjectPermission.WRITE_CONFIG)) {
throw new AuthException("not allowed to set ready for review");
}
@ -96,8 +101,13 @@ public class SetReadyForReview extends RetryingRestModifyView<ChangeResource, In
rsrc.getChange().getStatus() == Status.NEW && rsrc.getChange().isWorkInProgress(),
or(
rsrc.isUserOwner(),
permissionBackend
.currentUser()
.testCond(GlobalPermission.ADMINISTRATE_SERVER))));
or(
permissionBackend
.currentUser()
.testCond(GlobalPermission.ADMINISTRATE_SERVER),
permissionBackend
.currentUser()
.project(rsrc.getProject())
.testCond(ProjectPermission.WRITE_CONFIG)))));
}
}

View File

@ -33,6 +33,7 @@ import com.google.gerrit.server.change.WorkInProgressOp.Input;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
@ -67,7 +68,11 @@ public class SetWorkInProgress extends RetryingRestModifyView<ChangeResource, In
Change change = rsrc.getChange();
if (!rsrc.isUserOwner()
&& !permissionBackend.currentUser().test(GlobalPermission.ADMINISTRATE_SERVER)) {
&& !permissionBackend.currentUser().test(GlobalPermission.ADMINISTRATE_SERVER)
&& !permissionBackend
.currentUser()
.project(rsrc.getProject())
.test(ProjectPermission.WRITE_CONFIG)) {
throw new AuthException("not allowed to set work in progress");
}
@ -97,8 +102,13 @@ public class SetWorkInProgress extends RetryingRestModifyView<ChangeResource, In
rsrc.getChange().getStatus() == Status.NEW && !rsrc.getChange().isWorkInProgress(),
or(
rsrc.isUserOwner(),
permissionBackend
.currentUser()
.testCond(GlobalPermission.ADMINISTRATE_SERVER))));
or(
permissionBackend
.currentUser()
.testCond(GlobalPermission.ADMINISTRATE_SERVER),
permissionBackend
.currentUser()
.project(rsrc.getProject())
.testCond(ProjectPermission.WRITE_CONFIG)))));
}
}

View File

@ -432,6 +432,19 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(gApi.changes().id(changeId).get().workInProgress).isTrue();
}
@Test
public void setWorkInProgressAllowedAsProjectOwner() throws Exception {
setApiUser(user);
String changeId =
gApi.changes().create(new ChangeInput(project.get(), "master", "Test Change")).get().id;
com.google.gerrit.acceptance.TestAccount user2 = accountCreator.user2();
grant(project, "refs/*", Permission.OWNER, false, REGISTERED_USERS);
setApiUser(user2);
gApi.changes().id(changeId).setWorkInProgress();
assertThat(gApi.changes().id(changeId).get().workInProgress).isTrue();
}
@Test
public void setReadyForReviewNotAllowedWithoutPermission() throws Exception {
PushOneCommit.Result rready = createChange();
@ -456,6 +469,20 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(gApi.changes().id(changeId).get().workInProgress).isNull();
}
@Test
public void setReadyForReviewAllowedAsProjectOwner() throws Exception {
setApiUser(user);
String changeId =
gApi.changes().create(new ChangeInput(project.get(), "master", "Test Change")).get().id;
gApi.changes().id(changeId).setWorkInProgress();
com.google.gerrit.acceptance.TestAccount user2 = accountCreator.user2();
grant(project, "refs/*", Permission.OWNER, false, REGISTERED_USERS);
setApiUser(user2);
gApi.changes().id(changeId).setReadyForReview();
assertThat(gApi.changes().id(changeId).get().workInProgress).isNull();
}
@Test
public void hasReviewStarted() throws Exception {
PushOneCommit.Result r = createWorkInProgressChange();

View File

@ -1225,6 +1225,26 @@ public class RevisionIT extends AbstractDaemonTest {
.isEqualTo(in.message);
}
@Test
public void commentOnNonExistingFile() throws Exception {
PushOneCommit.Result r = createChange();
r = updateChange(r, "new content");
CommentInput in = new CommentInput();
in.line = 1;
in.message = "nit: trailing whitespace";
in.path = "non-existing.txt";
ReviewInput reviewInput = new ReviewInput();
Map<String, List<CommentInput>> comments = new HashMap<>();
comments.put("non-existing.txt", Collections.singletonList(in));
reviewInput.comments = comments;
reviewInput.message = "comment test";
exception.expect(BadRequestException.class);
exception.expectMessage(
String.format("not found in revision %d,1", r.getChange().change().getId().id));
gApi.changes().id(r.getChangeId()).revision(1).review(reviewInput);
}
@Test
public void patch() throws Exception {
PushOneCommit.Result r = createChange();

View File

@ -656,11 +656,11 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
assertThat(r.getChange().change().getOwner()).isEqualTo(user.id);
assertThat(r.getChange().change().isWorkInProgress()).isTrue();
// Other user trying to move from WIP to ready should fail.
// Admin user trying to move from WIP to ready should succeed.
GitUtil.fetch(testRepo, r.getPatchSet().getRefName() + ":ps");
testRepo.reset("ps");
r = amendChange(r.getChangeId(), "refs/for/master%ready", admin, testRepo);
r.assertErrorStatus(ReceiveConstants.ONLY_OWNER_CAN_MODIFY_WIP);
r = amendChange(r.getChangeId(), "refs/for/master%ready", user, testRepo);
r.assertOkStatus();
// Other user trying to move from WIP to WIP should succeed.
r = amendChange(r.getChangeId(), "refs/for/master%wip", admin, testRepo);
@ -672,14 +672,29 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
r.assertOkStatus();
assertThat(r.getChange().change().isWorkInProgress()).isFalse();
// Other user trying to move from ready to WIP should fail.
// Admin user trying to move from ready to WIP should succeed.
GitUtil.fetch(testRepo, r.getPatchSet().getRefName() + ":ps");
testRepo.reset("ps");
r = amendChange(r.getChangeId(), "refs/for/master%wip", admin, testRepo);
r.assertErrorStatus(ReceiveConstants.ONLY_OWNER_CAN_MODIFY_WIP);
r.assertOkStatus();
// Other user trying to move from ready to ready should succeed.
r = amendChange(r.getChangeId(), "refs/for/master%ready", admin, testRepo);
// Other user trying to move from wip to wip should succeed.
r = amendChange(r.getChangeId(), "refs/for/master%wip", admin, testRepo);
r.assertOkStatus();
// Non owner, non admin and non project owner cannot flip wip bit:
TestAccount user2 = accountCreator.user2();
grant(
project, "refs/*", Permission.FORGE_COMMITTER, false, SystemGroupBackend.REGISTERED_USERS);
TestRepository<?> user2Repo = cloneProject(project, user2);
GitUtil.fetch(user2Repo, r.getPatchSet().getRefName() + ":ps");
user2Repo.reset("ps");
r = amendChange(r.getChangeId(), "refs/for/master%ready", user2, user2Repo);
r.assertErrorStatus(ReceiveConstants.ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP);
// Project owner trying to move from WIP to ready should succeed.
allow("refs/*", Permission.OWNER, SystemGroupBackend.REGISTERED_USERS);
r = amendChange(r.getChangeId(), "refs/for/master%ready", user2, user2Repo);
r.assertOkStatus();
}

View File

@ -0,0 +1,67 @@
// Copyright (C) 2018 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.acceptance.ssh;
import static com.google.common.truth.Truth.assertThat;
import com.google.common.base.Joiner;
import com.google.common.collect.FluentIterable;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Injector;
import java.util.List;
import org.junit.Test;
@NoHttpd
@UseSsh
public abstract class AbstractIndexTests extends AbstractDaemonTest {
/** @param injector injector */
public abstract void configureIndex(Injector injector) throws Exception;
@Test
public void indexChange() throws Exception {
configureIndex(server.getTestInjector());
PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
String changeId = change.getChangeId();
String changeLegacyId = change.getChange().getId().toString();
disableChangeIndexWrites();
amendChange(changeId, "second test", "test2.txt", "test2");
assertQuery("message:second", change.getChange(), false);
enableChangeIndexWrites();
String cmd = Joiner.on(" ").join("gerrit", "index", "changes", changeLegacyId);
adminSshSession.exec(cmd);
assertQuery("message:second", change.getChange(), true);
}
protected void assertQuery(String q, ChangeData change, Boolean assertTrue) throws Exception {
List<ChangeInfo> result = query(q);
Iterable<Integer> ids = ids(result);
if (assertTrue) assertThat(ids).contains(change.getId().get());
else assertThat(ids).doesNotContain(change.getId().get());
}
protected static Iterable<Integer> ids(Iterable<ChangeInfo> changes) {
return FluentIterable.from(changes).transform(in -> in._number);
}
}

View File

@ -1,9 +1,38 @@
load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
java_library(
name = "util",
testonly = 1,
srcs = ["AbstractIndexTests.java"],
deps = ["//java/com/google/gerrit/acceptance:lib"],
)
acceptance_tests(
srcs = glob(["*IT.java"]),
srcs = glob(
["*IT.java"],
exclude = ["ElasticIndexIT.java"],
),
group = "ssh",
labels = ["ssh"],
vm_args = ["-Xmx512m"],
deps = ["//lib/commons:compress"],
deps = [
":util",
"//lib/commons:compress",
],
)
acceptance_tests(
srcs = ["ElasticIndexIT.java"],
group = "elastic",
labels = [
"elastic",
"docker",
"ssh",
],
deps = [
":util",
"//java/com/google/gerrit/elasticsearch",
"//javatests/com/google/gerrit/elasticsearch:elasticsearch_test_utils",
"//lib/commons:compress",
],
)

View File

@ -0,0 +1,62 @@
// Copyright (C) 2018 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.acceptance.ssh;
import com.google.gerrit.elasticsearch.ElasticContainer;
import com.google.gerrit.elasticsearch.ElasticTestUtils;
import com.google.gerrit.elasticsearch.ElasticTestUtils.ElasticNodeInfo;
import com.google.gerrit.elasticsearch.ElasticVersion;
import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Injector;
import java.util.UUID;
import org.eclipse.jgit.lib.Config;
public class ElasticIndexIT extends AbstractIndexTests {
private static ElasticContainer<?> container;
private static Config getConfig(ElasticVersion version) {
ElasticNodeInfo elasticNodeInfo;
try {
container = ElasticContainer.createAndStart(version);
elasticNodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
} catch (Throwable t) {
return null;
}
String indicesPrefix = UUID.randomUUID().toString();
Config cfg = new Config();
ElasticTestUtils.configure(cfg, elasticNodeInfo.port, indicesPrefix);
return cfg;
}
@ConfigSuite.Default
public static Config elasticsearchV2() {
return getConfig(ElasticVersion.V2_4);
}
@ConfigSuite.Config
public static Config elasticsearchV5() {
return getConfig(ElasticVersion.V5_6);
}
@ConfigSuite.Config
public static Config elasticsearchV6() {
return getConfig(ElasticVersion.V6_2);
}
@Override
public void configureIndex(Injector injector) throws Exception {
ElasticTestUtils.createAllIndexes(injector);
}
}

View File

@ -0,0 +1,23 @@
// Copyright (C) 2018 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.acceptance.ssh;
import com.google.inject.Injector;
public class IndexIT extends AbstractIndexTests {
@Override
public void configureIndex(Injector injector) throws Exception {}
}

View File

@ -65,7 +65,7 @@ public class ElasticContainer<SELF extends ElasticContainer<SELF>> extends Gener
}
@Override
protected Set<Integer> getLivenessCheckPorts() {
public Set<Integer> getLivenessCheckPortNumbers() {
return ImmutableSet.of(getMappedPort(ELASTICSEARCH_DEFAULT_PORT));
}

View File

@ -71,6 +71,7 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.index.account.AccountField;
import com.google.gerrit.server.index.account.AccountIndex;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.index.account.AccountIndexer;
import com.google.gerrit.server.schema.SchemaCreator;
@ -448,6 +449,18 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
assertAccounts(queryProvider.get().byWatchedProject(allProjects), user3);
}
@Test
public void byDeletedAccount() throws Exception {
AccountInfo user = newAccountWithFullName("jdoe", "John Doe");
Account.Id userId = Account.Id.tryParse(user._accountId.toString()).get();
assertQuery("John", user);
for (AccountIndex index : indexes.getWriteIndexes()) {
index.delete(userId);
}
assertQuery("John");
}
@Test
public void withLimit() throws Exception {
String domain = name("test.com");

View File

@ -886,6 +886,18 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
assertQuery("message:12346", change2);
}
@Test
public void byMessageMixedCase() throws Exception {
TestRepository<Repo> repo = createProject("repo");
RevCommit commit1 = repo.parseBody(repo.commit().message("Hello gerrit").create());
Change change1 = insert(repo, newChangeForCommit(repo, commit1));
RevCommit commit2 = repo.parseBody(repo.commit().message("Hello Gerrit").create());
Change change2 = insert(repo, newChangeForCommit(repo, commit2));
assertQuery("message:gerrit", change2, change1);
assertQuery("message:Gerrit", change2, change1);
}
@Test
public void byLabel() throws Exception {
accountManager.authenticate(AuthRequest.forUser("anotheruser"));

View File

@ -50,6 +50,7 @@ import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.group.db.GroupsUpdate;
import com.google.gerrit.server.group.db.InternalGroupUpdate;
import com.google.gerrit.server.index.group.GroupField;
import com.google.gerrit.server.index.group.GroupIndex;
import com.google.gerrit.server.index.group.GroupIndexCollection;
import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.util.ManualRequestContext;
@ -104,6 +105,8 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests {
@Inject protected GroupIndexCollection indexes;
@Inject private GroupIndexCollection groupIndexes;
protected LifecycleManager lifecycle;
protected Injector injector;
protected ReviewDb db;
@ -392,6 +395,19 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests {
assertThat(rawFields.get().getValue(GroupField.UUID)).isEqualTo(uuid.get());
}
@Test
public void byDeletedGroup() throws Exception {
GroupInfo group = createGroup(name("group"));
AccountGroup.UUID uuid = new AccountGroup.UUID(group.id);
String query = "uuid:" + uuid;
assertQuery(query, group);
for (GroupIndex index : groupIndexes.getWriteIndexes()) {
index.delete(uuid);
}
assertQuery(query);
}
private Account.Id createAccountOutsideRequestContext(
String username, String fullName, String email, boolean active) throws Exception {
try (ManualRequestContext ctx = oneOffRequestContext.open()) {