opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.12'

Browse Source 
* stable-2.12:
  EmailMerge: provide user when available
  Set version to 2.11.7
  Release notes for Gerrit 2.11.7
  Document new allowrcpt restriction on adding user email
  OutputStreamQuery: fix comments with current-patch-set option
  Add tests for ssh query command
  EqualsFilePredicate: Make it work in project watches
  commit-msg: respect commentChar from git-config
  EmailReviewComments: Provide the current user instead of exception
  Remove index defaultMaxClauseCount config setting while reusing maxTerms
  OutputStreamQuery: Take files into account when adding patch sets
  Update 2.12 release notes with fixes from stable-2.11
  Update 2.11.6 release notes
  OutgoingEmail: Fix breakage introduced in add() method
  ChangeData: Throw exception when failing to reload change
  SuggestReviewersIT: Test for case-preserved reviewer suggestion
  Return case-preserving email when suggesting reviewers
  ReceiveCommits: Improve error message when failing to insert patch set
  OutgoingEmail: Kill "Skipping delivery of email" logspam
  OutgoingEmail: Check for valid email address when adding recipients
  ldap.Helper: Kill "assuming empty group membership" logspam
  SmtpEmailSender: Refactor open method and improve logging
  CreateEmail: Don't allow to add email prohibited by sendemail.allowrcpt
  RegisterNewEmailSender: Don't unconditionally send verification mail
  Set version to 2.11.6
  Release notes for Gerrit 2.11.6
  Don't reload plugins every minute
  Update documentation of commentlink to reflect changed URL
  Fix display of active row marker in tag list
  Update replication plugin to latest revision
  Fix tcl syntax highlighting not working
  Fix index.maxTerms documented default from 500 to no limit
  Update replication plugin to latest revision
  Update replication plugin to latest revision
  XSRF token cookie should honor auth.cookieSecure setting
  Fix pushing a first patchset does not trigger replication
  Fix merge validator exception type is ignored
  Send email using email queue instead of the default queue
  Rename EmailReviewCommentsExecutor to SendEmailExecutor
  Expose IndexCollection in plugin guice injector
  Update replication plugin to latest revision
  Remove documentation of --type option from ls-groups command
  ReceiveCommits: Log error message when failing to insert patch set
  ReceiveCommits: Remove declaration of unthrown exception
  Schema_113: Fix push certificate column type
  Update replication plugin to latest version
  Return case-preserving email when suggesting reviewers
  ChangeEditUtil: Fix inconsistent change message
  Fix serving of documentation in application container
  Update replication plugin to latest revision
  Use merge strategy for mergeability testing on "Rebase if Necessary" strategy

The changes done in Id1de81b31 ("Fix merge validator exception type is
ignored") are not compatible with the master branch and are reverted
in this merge.

This reverts commit dd41c53bd7.

Change-Id: I821a40bbfe77832c0bd2342ab22d37433a683799
This commit is contained in:
David Pursehouse
2016-02-10 17:37:19 +09:00
committed by David Ostrovsky
parent 685b5b76cc a3f22a26ef
commit 557ec3c776
33 changed files with 726 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java
View File

@@ -103,7 +103,8 @@ public class CreateEmail implements RestModifyView<AccountResource, EmailInput>
public Response<EmailInfo> apply(IdentifiedUser user, EmailInput input)
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, OrmException, EmailException {
ResourceNotFoundException, OrmException, EmailException,
MethodNotAllowedException {
if (input.email != null && !email.equals(input.email)) {
throw new BadRequestException("email address must match URL");
}
@@ -126,7 +127,11 @@ public class CreateEmail implements RestModifyView<AccountResource, EmailInput>
}
} else {
try {
registerNewEmailFactory.create(email).send();
RegisterNewEmailSender sender = registerNewEmailFactory.create(email);
if (!sender.isAllowed()) {
throw new MethodNotAllowedException("Not allowed to add email address " + email);
}
sender.send();
info.pendingConfirmation = true;
} catch (EmailException | RuntimeException e) {
log.error("Cannot send email verification message to " + email, e);

4
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java
View File

@@ -225,8 +225,6 @@ import javax.security.auth.login.LoginException;
try {
account = findAccount(schema, ctx, username, false);
} catch (AccountException e) {
LdapRealm.log.warn("Account " + username +
" not found, assuming empty group membership");
return Collections.emptySet();
}
}
@@ -248,8 +246,6 @@ import javax.security.auth.login.LoginException;
try {
account = findAccount(schema, ctx, username, true);
} catch (AccountException e) {
LdapRealm.log.warn("Account " + username +
" not found, assuming empty group membership");
return Collections.emptySet();
}
}

17
gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java
View File

@@ -17,12 +17,12 @@ package com.google.gerrit.server.change;
import static com.google.gerrit.server.PatchLineCommentsUtil.PLC_ORDER;
import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.SendEmailExecutor;
import com.google.gerrit.server.mail.CommentSender;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -32,7 +32,6 @@ import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.OutOfScopeException;
import com.google.inject.Provider;
import com.google.inject.ProvisionException;
import com.google.inject.assistedinject.Assisted;
@@ -51,7 +50,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
NotifyHandling notify,
ChangeNotes notes,
PatchSet patchSet,
Account.Id authorId,
IdentifiedUser user,
ChangeMessage message,
List<PatchLineComment> comments);
}
@@ -65,13 +64,13 @@ public class EmailReviewComments implements Runnable, RequestContext {
private final NotifyHandling notify;
private final ChangeNotes notes;
private final PatchSet patchSet;
private final Account.Id authorId;
private final IdentifiedUser user;
private final ChangeMessage message;
private List<PatchLineComment> comments;
private ReviewDb db;
@Inject
EmailReviewComments (
EmailReviewComments(
@SendEmailExecutor ExecutorService executor,
PatchSetInfoFactory patchSetInfoFactory,
CommentSender.Factory commentSenderFactory,
@@ -80,7 +79,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
@Assisted NotifyHandling notify,
@Assisted ChangeNotes notes,
@Assisted PatchSet patchSet,
@Assisted Account.Id authorId,
@Assisted IdentifiedUser user,
@Assisted ChangeMessage message,
@Assisted List<PatchLineComment> comments) {
this.sendEmailsExecutor = executor;
@@ -91,7 +90,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
this.notify = notify;
this.notes = notes;
this.patchSet = patchSet;
this.authorId = authorId;
this.user = user;
this.message = message;
this.comments = PLC_ORDER.sortedCopy(comments);
}
@@ -107,7 +106,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
CommentSender cm = commentSenderFactory.create(notes.getProjectName(),
notes.getChangeId());
cm.setFrom(authorId);
cm.setFrom(user.getAccountId());
cm.setPatchSet(patchSet,
patchSetInfoFactory.get(notes.getProjectName(), patchSet));
cm.setChangeMessage(message);
@@ -132,7 +131,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
@Override
public CurrentUser getUser() {
throw new OutOfScopeException("No user on email thread");
return user.getRealUser();
}
@Override

2
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@@ -384,7 +384,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
in.notify,
notes,
ps,
user.getAccountId(),
user,
message,
comments).sendAsync();
}

2
gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestionCache.java
View File

@@ -172,9 +172,9 @@ public class ReviewerSuggestionCache {
doc.add(new TextField(NAME, a.getFullName(), Store.YES));
}
if (a.getPreferredEmail() != null) {
doc.add(new TextField(EMAIL, a.getPreferredEmail(), Store.YES));
doc.add(new StringField(EMAIL, a.getPreferredEmail().toLowerCase(),
Store.YES));
doc.add(new TextField(EMAIL, a.getPreferredEmail(), Store.YES));
}
AccountExternalIdAccess extIdAccess = db.get().accountExternalIds();
String username = AccountState.getUserName(

2
gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java
View File

@@ -247,7 +247,7 @@ public class ChangeEditUtil {
PatchSetInserter inserter =
patchSetInserterFactory.create(ctl, psId, squashed);
StringBuilder message = new StringBuilder("Patch set ")
StringBuilder message = new StringBuilder("Patch Set ")
.append(inserter.getPatchSetId().get())
.append(": ");

8
gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java
View File

@@ -20,6 +20,7 @@ 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.mail.MergedSender;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -48,6 +49,7 @@ public class EmailMerge implements Runnable, RequestContext {
private final MergedSender.Factory mergedSenderFactory;
private final SchemaFactory<ReviewDb> schemaFactory;
private final ThreadLocalRequestContext requestContext;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final Project.NameKey project;
private final Change.Id changeId;
@@ -59,6 +61,7 @@ public class EmailMerge implements Runnable, RequestContext {
MergedSender.Factory mergedSenderFactory,
SchemaFactory<ReviewDb> schemaFactory,
ThreadLocalRequestContext requestContext,
IdentifiedUser.GenericFactory identifiedUserFactory,
@Assisted Project.NameKey project,
@Assisted Change.Id changeId,
@Assisted @Nullable Account.Id submitter) {
@@ -66,6 +69,7 @@ public class EmailMerge implements Runnable, RequestContext {
this.mergedSenderFactory = mergedSenderFactory;
this.schemaFactory = schemaFactory;
this.requestContext = requestContext;
this.identifiedUserFactory = identifiedUserFactory;
this.project = project;
this.changeId = changeId;
this.submitter = submitter;
@@ -102,6 +106,10 @@ public class EmailMerge implements Runnable, RequestContext {
@Override
public CurrentUser getUser() {
if (submitter != null) {
return identifiedUserFactory.create(
getReviewDbProvider(), submitter).getRealUser();
}
throw new OutOfScopeException("No user on email thread");
}

5
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -798,7 +798,7 @@ public class ReceiveCommits {
} catch (RestApiException err) {
reject(replace.inputCommand, "internal server error");
log.error(String.format(
"Cannot add patch set to %d of %s",
"Cannot add patch set to change %d in project %s",
e.getKey().get(), project.getName()), err);
}
} else if (replace.inputCommand.getResult() == NOT_ATTEMPTED) {
@@ -2169,6 +2169,9 @@ public class ReceiveCommits {
try (RequestState state = requestState(caller)) {
return insertPatchSet(state);
}
} catch (OrmException | IOException e) {
log.error("Failed to insert patch set", e);
throw e;
} finally {
synchronizedIncrement(replaceProgress);
}

6
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
View File

@@ -216,8 +216,10 @@ public class RebaseIfNecessary extends SubmitStrategy {
static boolean dryRun(SubmitDryRun.Arguments args,
CodeReviewCommit mergeTip, CodeReviewCommit toMerge)
throws IntegrationException {
// Test for merge instead of cherry pick to avoid false negatives
// on commit chains.
return !args.mergeUtil.hasMissingDependencies(args.mergeSorter, toMerge)
&& args.mergeUtil.canCherryPick(args.mergeSorter, args.repo, mergeTip,
args.rw, toMerge);
&& args.mergeUtil.canMerge(args.mergeSorter, args.repo, mergeTip,
toMerge);
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java
View File

@@ -29,7 +29,7 @@ import org.eclipse.jgit.lib.Config;
*/
@AutoValue
public abstract class IndexConfig {
private static final int DEFAULT_MAX_TERMS = 500;
private static final int DEFAULT_MAX_TERMS = 1024;
public static IndexConfig createDefault() {
return create(0, 0, DEFAULT_MAX_TERMS);
@@ -47,7 +47,7 @@ public abstract class IndexConfig {
return new AutoValue_IndexConfig(
checkLimit(maxLimit, "maxLimit", Integer.MAX_VALUE),
checkLimit(maxPages, "maxPages", Integer.MAX_VALUE),
checkLimit(maxTerms, "maxTerms", Integer.MAX_VALUE));
checkLimit(maxTerms, "maxTerms", DEFAULT_MAX_TERMS));
}
private static int checkLimit(int limit, String name, int defaultValue) {

31
gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java
View File

@@ -29,6 +29,7 @@ import com.google.gerrit.server.validators.ValidationException;
import com.google.gwtorm.server.OrmException;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.validator.routines.EmailValidator;
import org.apache.velocity.Template;
import org.apache.velocity.VelocityContext;
import org.apache.velocity.context.InternalContextAdapterImpl;
@@ -336,7 +337,6 @@ public abstract class OutgoingEmail {
protected boolean shouldSendMessage() {
if (body.length() == 0) {
// If we have no message body, don't send.
log.warn("Skipping delivery of email with no body");
return false;
}
@@ -344,7 +344,6 @@ public abstract class OutgoingEmail {
// If we have nobody to send this message to, then all of our
// selection filters previously for this type of message were
// unable to match a destination. Don't bother sending it.
log.warn("Skipping delivery of email with no recipients");
return false;
}
@@ -394,21 +393,21 @@ public abstract class OutgoingEmail {
/** Schedule delivery of this message to the given account. */
protected void add(final RecipientType rt, final Address addr) {
if (addr != null && addr.email != null && addr.email.length() > 0) {
if (args.emailSender.canEmail(addr.email)) {
if (smtpRcptTo.add(addr)) {
switch (rt) {
case TO:
((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr);
break;
case CC:
((EmailHeader.AddressList) headers.get(HDR_CC)).add(addr);
break;
case BCC:
break;
}
}
} else {
if (!EmailValidator.getInstance().isValid(addr.email)) {
log.warn("Not emailing " + addr.email + " (invalid email address)");
} else if (!args.emailSender.canEmail(addr.email)) {
log.warn("Not emailing " + addr.email + " (prohibited by allowrcpt)");
} else if (smtpRcptTo.add(addr)) {
switch (rt) {
case TO:
((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr);
break;
case CC:
((EmailHeader.AddressList) headers.get(HDR_CC)).add(addr);
break;
case BCC:
break;
}
}
}
}

9
gerrit-server/src/main/java/com/google/gerrit/server/mail/RegisterNewEmailSender.java
View File

@@ -49,11 +49,6 @@ public class RegisterNewEmailSender extends OutgoingEmail {
add(RecipientType.TO, new Address(addr));
}
@Override
protected boolean shouldSendMessage() {
return true;
}
@Override
protected void format() throws EmailException {
appendText(velocifyFile("RegisterNewEmail.vm"));
@@ -70,4 +65,8 @@ public class RegisterNewEmailSender extends OutgoingEmail {
}
return emailToken;
}
public boolean isAllowed() {
return args.emailSender.canEmail(addr);
}
}

35
gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java
View File

@@ -249,16 +249,19 @@ public class SmtpEmailSender implements EmailSender {
client.enableSSL(sslVerify);
}
client.setConnectTimeout(connectTimeout);
try {
client.setConnectTimeout(connectTimeout);
client.connect(smtpHost, smtpPort);
if (!SMTPReply.isPositiveCompletion(client.getReplyCode())) {
throw new EmailException("SMTP server rejected connection");
int replyCode = client.getReplyCode();
String replyString = client.getReplyString();
if (!SMTPReply.isPositiveCompletion(replyCode)) {
throw new EmailException(
String.format("SMTP server rejected connection: %d: %s",
replyCode, replyString));
}
if (!client.login()) {
String e = client.getReplyString();
throw new EmailException(
"SMTP server rejected HELO/EHLO greeting: " + e);
"SMTP server rejected HELO/EHLO greeting: " + replyString);
}
if (smtpEncryption == Encryption.TLS) {
@@ -266,16 +269,15 @@ public class SmtpEmailSender implements EmailSender {
throw new EmailException("SMTP server does not support TLS");
}
if (!client.login()) {
String e = client.getReplyString();
throw new EmailException("SMTP server rejected login: " + e);
throw new EmailException("SMTP server rejected login: " + replyString);
}
}
if (smtpUser != null && !client.auth(smtpUser, smtpPass)) {
String e = client.getReplyString();
throw new EmailException("SMTP server rejected auth: " + e);
throw new EmailException("SMTP server rejected auth: " + replyString);
}
} catch (IOException e) {
return client;
} catch (IOException | EmailException e) {
if (client.isConnected()) {
try {
client.disconnect();
@@ -283,17 +285,10 @@ public class SmtpEmailSender implements EmailSender {
//Ignored
}
}
throw new EmailException(e.getMessage(), e);
} catch (EmailException e) {
if (client.isConnected()) {
try {
client.disconnect();
} catch (IOException e2) {
// Ignored
}
if (e instanceof EmailException) {
throw (EmailException) e;
}
throw e;
throw new EmailException(e.getMessage(), e);
}
return client;
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java
View File

@@ -14,8 +14,6 @@
package com.google.gerrit.server.plugins;
import static com.google.gerrit.common.FileUtil.lastModified;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
@@ -171,6 +169,6 @@ public abstract class Plugin {
protected abstract boolean canReload();
boolean isModified(Path jar) {
return snapshot.lastModified() != lastModified(jar);
return snapshot.isModified(jar.toFile());
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java
View File

@@ -564,7 +564,9 @@ public class PluginGuiceEnvironment {
return false;
}
Class<?> type = key.getTypeLiteral().getRawType();
if (LifecycleListener.class.isAssignableFrom(type)) {
if (LifecycleListener.class.isAssignableFrom(type)
// This is needed for secondary index to work from plugin listeners
&& !is("com.google.gerrit.server.index.IndexCollection", type)) {
return false;
}
if (StartPluginListener.class.isAssignableFrom(type)) {

6
gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
View File

@@ -293,6 +293,10 @@ public class OutputStreamQuery {
eventFactory.addPatchSetFileNames(c.currentPatchSet,
d.change(), d.currentPatchSet());
}
if (includeComments) {
eventFactory.addPatchSetComments(c.currentPatchSet,
d.publishedComments());
}
}
}
@@ -301,7 +305,7 @@ public class OutputStreamQuery {
if (includePatchSets) {
eventFactory.addPatchSets(db.get(), rw, c, d.patchSets(),
includeApprovals ? d.approvals().asMap() : null,
labelTypes);
includeFiles, d.notes(), labelTypes);
for (PatchSetAttribute attribute : c.patchSets) {
eventFactory.addPatchSetComments(
attribute, d.publishedComments());