diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java index 1528deb2bf..1ecc059b52 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java @@ -68,7 +68,7 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit { assertSubmitter(change2.getChangeId(), 1); assertSubmitter(change2.getChangeId(), 2); assertPersonEquals(admin.getIdent(), head.getAuthorIdent()); - assertPersonEquals(serverIdent.get(), head.getCommitterIdent()); + assertPersonEquals(admin.getIdent(), head.getCommitterIdent()); } @Test diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/FileUtil.java b/gerrit-common/src/main/java/com/google/gerrit/common/FileUtil.java index 2335b8d1b3..ed28ec09b8 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/FileUtil.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/FileUtil.java @@ -88,7 +88,9 @@ public class FileUtil { public static Path mkdirsOrDie(Path p, String errMsg) { try { - Files.createDirectories(p); + if (!Files.isDirectory(p)) { + Files.createDirectories(p); + } return p; } catch (IOException e) { throw new Die(errMsg + ": " + p, e); diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListChangesOption.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListChangesOption.java index 7e99d1a655..4c336f7047 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListChangesOption.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListChangesOption.java @@ -46,6 +46,10 @@ public enum ListChangesOption { /** Set the reviewed boolean for the caller. */ REVIEWED(11), + /** Not used anymore, kept for backward compatibility */ + @Deprecated + DRAFT_COMMENTS(12), + /** Include download commands for the caller. */ DOWNLOAD_COMMANDS(13), diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/AvatarImage.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/AvatarImage.java index 9322a1d4cc..bec89ccd7e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/AvatarImage.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/AvatarImage.java @@ -92,6 +92,7 @@ public class AvatarImage extends Image implements LoadHandler { private void loadAvatar(AccountInfo account, int size, boolean addPopup) { if (!Gerrit.info().plugin().hasAvatars()) { + setVisible(false); return; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index c67654e93f..7b32600367 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -501,7 +501,7 @@ public class MergeOp implements AutoCloseable { Multimap cbb = cs.changesByBranch(); for (Branch.NameKey branch : cbb.keySet()) { OpenRepo or = openRepo(branch.getParentKey()); - toSubmit.put(branch, validateChangeList(or, cbb.get(branch))); + toSubmit.put(branch, validateChangeList(or, cbb.get(branch), caller)); } failFast(cs); // Done checks that don't involve running submit strategies. @@ -623,7 +623,8 @@ public class MergeOp implements AutoCloseable { } private BranchBatch validateChangeList(OpenRepo or, - Collection submitted) throws IntegrationException { + Collection submitted, IdentifiedUser caller) + throws IntegrationException { logDebug("Validating {} changes", submitted.size()); List toSubmit = new ArrayList<>(submitted.size()); Multimap revisions = getRevisions(or, submitted); @@ -698,7 +699,7 @@ public class MergeOp implements AutoCloseable { MergeValidators mergeValidators = mergeValidatorsFactory.create(); try { mergeValidators.validatePreMerge( - or.repo, commit, or.project, destBranch, ps.getId()); + or.repo, commit, or.project, destBranch, ps.getId(), caller); } catch (MergeValidationException mve) { problems.put(changeId, mve.getMessage()); continue; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java index be5fa57bd4..dd84efefaa 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java @@ -37,6 +37,7 @@ import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gwtorm.server.OrmException; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.PersonIdent; import java.io.IOException; import java.util.Collection; @@ -53,6 +54,12 @@ public class RebaseIfNecessary extends SubmitStrategy { this.newCommits = new HashMap<>(); } + private PersonIdent getSubmitterIdent() { + return args.caller.newCommitterIdent( + args.serverIdent.getWhen(), + args.serverIdent.getTimeZone()); + } + @Override public MergeTip run(final CodeReviewCommit branchTip, final Collection toMerge) throws IntegrationException { @@ -156,7 +163,7 @@ public class RebaseIfNecessary extends SubmitStrategy { // Racy read of patch set is ok; see comments in RebaseChangeOp. args.db.patchSets().get(toMerge.getPatchsetId()), mergeTip.getCurrentTip().name()) - .setCommitterIdent(args.serverIdent) + .setCommitterIdent(getSubmitterIdent()) .setRunHooks(false) .setValidatePolicy(CommitValidators.Policy.NONE); try { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationListener.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationListener.java index 6e98223b5b..d89d3b3abd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationListener.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationListener.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.git.validators; import com.google.gerrit.extensions.annotations.ExtensionPoint; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.project.ProjectState; @@ -37,12 +38,14 @@ public interface MergeValidationListener { * @param destProject the destination project * @param destBranch the destination branch * @param patchSetId the patch set ID + * @param caller the user who initiated the merge request * @throws MergeValidationException if the commit fails to validate */ void onPreMerge(Repository repo, CodeReviewCommit commit, ProjectState destProject, Branch.NameKey destBranch, - PatchSet.Id patchSetId) + PatchSet.Id patchSetId, + IdentifiedUser caller) throws MergeValidationException; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java index e6585370ba..fe46888cd3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java @@ -60,7 +60,8 @@ public class MergeValidators { CodeReviewCommit commit, ProjectState destProject, Branch.NameKey destBranch, - PatchSet.Id patchSetId) + PatchSet.Id patchSetId, + IdentifiedUser caller) throws MergeValidationException { List validators = Lists.newLinkedList(); @@ -68,7 +69,8 @@ public class MergeValidators { validators.add(projectConfigValidatorFactory.create()); for (MergeValidationListener validator : validators) { - validator.onPreMerge(repo, commit, destProject, destBranch, patchSetId); + validator.onPreMerge(repo, commit, destProject, destBranch, patchSetId, + caller); } } @@ -124,7 +126,8 @@ public class MergeValidators { final CodeReviewCommit commit, final ProjectState destProject, final Branch.NameKey destBranch, - final PatchSet.Id patchSetId) + final PatchSet.Id patchSetId, + IdentifiedUser caller) throws MergeValidationException { if (RefNames.REFS_CONFIG.equals(destBranch.get())) { final Project.NameKey newParent; @@ -200,10 +203,12 @@ public class MergeValidators { CodeReviewCommit commit, ProjectState destProject, Branch.NameKey destBranch, - PatchSet.Id patchSetId) + PatchSet.Id patchSetId, + IdentifiedUser caller) throws MergeValidationException { for (MergeValidationListener validator : mergeValidationListeners) { - validator.onPreMerge(repo, commit, destProject, destBranch, patchSetId); + validator.onPreMerge(repo, commit, destProject, destBranch, patchSetId, + caller); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java index 30479b77d5..122b369e74 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java @@ -45,10 +45,13 @@ import org.eclipse.jgit.revwalk.RevWalk; import java.io.IOException; import java.sql.ResultSet; +import java.sql.ResultSetMetaData; import java.sql.SQLException; import java.sql.Statement; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; public class Schema_115 extends SchemaVersion { private final GitRepositoryManager mgr; @@ -72,54 +75,73 @@ public class Schema_115 extends SchemaVersion { Map imports = new HashMap<>(); try (Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); ResultSet rs = stmt.executeQuery( - "SELECT " - + "id, " - + "context, " - + "expand_all_comments, " - + "hide_line_numbers, " - + "hide_top_menu, " - + "ignore_whitespace, " - + "intraline_difference, " - + "line_length, " - + "manual_review, " - + "render_entire_file, " - + "retain_header, " - + "show_line_endings, " - + "show_tabs, " - + "show_whitespace_errors, " - + "skip_deleted, " - + "skip_uncommented, " - + "syntax_highlighting, " - + "tab_size, " - + "theme, " - + "hide_empty_pane, " - + "auto_hide_diff_table_header " - + "FROM account_diff_preferences")) { + "SELECT * FROM account_diff_preferences")) { + Set availableColumns = getColumns(rs); while (rs.next()) { - Account.Id accountId = new Account.Id(rs.getInt(1)); + Account.Id accountId = new Account.Id(rs.getInt("id")); DiffPreferencesInfo prefs = new DiffPreferencesInfo(); - prefs.context = (int)rs.getShort(2); - prefs.expandAllComments = toBoolean(rs.getString(3)); - prefs.hideLineNumbers = toBoolean(rs.getString(4)); - prefs.hideTopMenu = toBoolean(rs.getString(5)); - // Enum with char as value - prefs.ignoreWhitespace = toWhitespace(rs.getString(6)); - prefs.intralineDifference = toBoolean(rs.getString(7)); - prefs.lineLength = rs.getInt(8); - prefs.manualReview = toBoolean(rs.getString(9)); - prefs.renderEntireFile = toBoolean(rs.getString(10)); - prefs.retainHeader = toBoolean(rs.getString(11)); - prefs.showLineEndings = toBoolean(rs.getString(12)); - prefs.showTabs = toBoolean(rs.getString(13)); - prefs.showWhitespaceErrors = toBoolean(rs.getString(14)); - prefs.skipDeleted = toBoolean(rs.getString(15)); - prefs.skipUncommented = toBoolean(rs.getString(16)); - prefs.syntaxHighlighting = toBoolean(rs.getString(17)); - prefs.tabSize = rs.getInt(18); - // Enum with name as values; can be null - prefs.theme = toTheme(rs.getString(19)); - prefs.hideEmptyPane = toBoolean(rs.getString(20)); - prefs.autoHideDiffTableHeader = toBoolean(rs.getString(21)); + if (availableColumns.contains("context")) { + prefs.context = (int)rs.getShort("context"); + } + if (availableColumns.contains("expand_all_comments")) { + prefs.expandAllComments = toBoolean(rs.getString("expand_all_comments")); + } + if (availableColumns.contains("hide_line_numbers")) { + prefs.hideLineNumbers = toBoolean(rs.getString("hide_line_numbers")); + } + if (availableColumns.contains("hide_top_menu")) { + prefs.hideTopMenu = toBoolean(rs.getString("hide_top_menu")); + } + if (availableColumns.contains("ignore_whitespace")) { + // Enum with char as value + prefs.ignoreWhitespace = toWhitespace(rs.getString("ignore_whitespace")); + } + if (availableColumns.contains("intraline_difference")) { + prefs.intralineDifference = toBoolean(rs.getString("intraline_difference")); + } + if (availableColumns.contains("line_length")) { + prefs.lineLength = rs.getInt("line_length"); + } + if (availableColumns.contains("manual_review")) { + prefs.manualReview = toBoolean(rs.getString("manual_review")); + } + if (availableColumns.contains("render_entire_file")) { + prefs.renderEntireFile = toBoolean(rs.getString("render_entire_file")); + } + if (availableColumns.contains("retain_header")) { + prefs.retainHeader = toBoolean(rs.getString("retain_header")); + } + if (availableColumns.contains("show_line_endings")) { + prefs.showLineEndings = toBoolean(rs.getString("show_line_endings")); + } + if (availableColumns.contains("show_tabs")) { + prefs.showTabs = toBoolean(rs.getString("show_tabs")); + } + if (availableColumns.contains("show_whitespace_errors")) { + prefs.showWhitespaceErrors = toBoolean(rs.getString("show_whitespace_errors")); + } + if (availableColumns.contains("skip_deleted")) { + prefs.skipDeleted = toBoolean(rs.getString("skip_deleted")); + } + if (availableColumns.contains("skip_uncommented")) { + prefs.skipUncommented = toBoolean(rs.getString("skip_uncommented")); + } + if (availableColumns.contains("syntax_highlighting")) { + prefs.syntaxHighlighting = toBoolean(rs.getString("syntax_highlighting")); + } + if (availableColumns.contains("tab_size")) { + prefs.tabSize = rs.getInt("tab_size"); + } + if (availableColumns.contains("theme")) { + // Enum with name as values; can be null + prefs.theme = toTheme(rs.getString("theme")); + } + if (availableColumns.contains("hide_empty_pane")) { + prefs.hideEmptyPane = toBoolean(rs.getString("hide_empty_pane")); + } + if (availableColumns.contains("auto_hide_diff_table_header")) { + prefs.autoHideDiffTableHeader = toBoolean(rs.getString("auto_hide_diff_table_header")); + } imports.put(accountId, prefs); } } @@ -151,6 +173,16 @@ public class Schema_115 extends SchemaVersion { } } + private Set getColumns(ResultSet rs) throws SQLException { + ResultSetMetaData metaData = rs.getMetaData(); + int columnCount = metaData.getColumnCount(); + Set columns = new HashSet<>(columnCount); + for (int i = 1; i <= columnCount; i++) { + columns.add(metaData.getColumnLabel(i).toLowerCase()); + } + return columns; + } + private static Theme toTheme(String v) { if (v == null) { return Theme.DEFAULT; diff --git a/plugins/cookbook-plugin b/plugins/cookbook-plugin index 88b0984638..29f134c383 160000 --- a/plugins/cookbook-plugin +++ b/plugins/cookbook-plugin @@ -1 +1 @@ -Subproject commit 88b0984638857ac7139de83b18dc7cad23670b4d +Subproject commit 29f134c383e8958132887f5275e331472b981faa