Respect the Gerrit preference for the number of context lines

GitBlit unconditionally uses a diff context of 3 lines, which may be a
tad small. Change this so that the plugin respects the value the user
has set in his or her Gerrit preferences.

Introduce a new GitBlit configuration property to limit the number of
context lines in a commit diff. Otherwise GitBlit might report too many
files as "Diff too large" in a diff over all files in a commit.

Update documentation and increase version number.

docu/index.md

@@ -5,7 +5,7 @@ with full SSO through Gerrit.
 
 * License: [Apache Public License 2.0](http://www.apache.org/licenses/LICENSE-2.0)
 * [Home page](https://github.com/tomaswolf/gerrit-gitblit-plugin)
-* Installed plugin version: <em id='gerrit-gitblit-current-version'>2.12.162.1-SNAPSHOT</em> &mdash; <a id='gerrit-gitblit-version-check' style='display:none;' href='#'>Check for updates</a>
+* Installed plugin version: <em id='gerrit-gitblit-current-version'>2.12.162.2-SNAPSHOT</em> &mdash; <a id='gerrit-gitblit-version-check' style='display:none;' href='#'>Check for updates</a>
 
 For a list of contributors, see at [GitHub](https://github.com/tomaswolf/gerrit-gitblit-plugin/graphs/contributors).
 
@@ -76,13 +76,41 @@ The GitBlit `${baseFolder}` is the plugin's data directory provided by Gerrit at
 > Up to and including v2.11.162.1 of this plugin, GitBlit's `${baseFolder}` was at `$GERRIT_SITE/etc/gitblit/`. If you upgraded from that or an earlier
 > version and do have data in `$GERRIT_SITE/etc/gitblit/`, move it over to the new place. The directory `$GERRIT_SITE/etc/gitblit/` can then be removed.
 
+### GitBlit settings specific to this plugin
+
+<dl>
+	<dt><code>web.maxCommitDiffContext</code> = [1 .. 10]</dt>
+	<dd>
+		<p>
+			<em>Since 2.12.162.2.</em> In a GitBlit <em>commit diff</em>, diffs of all changed files in a commit are shown on one page. This setting
+			defines how many context lines shall be shown shown at most in such diffs. The default is 10, but can be reduced further through this property
+			in your <code>gitblit.properties</code> file.
+		</p>
+		<p>
+			If the Gerrit preference setting for the number of context lines in diffs is lower than this GitBlit setting, the Gerrit setting is taken.
+		</p>
+		<p>
+			For single-file diffs, the plugin respects the Gerrit preference setting of the currently logged-in user for the number of context lines
+			in a diff. For non-logged-in users, the GitBlit default of 3 context lines applies. (The Gerrit default is 10.)
+		</p>
+	</dd>
+</dl>
+
+
 # Issue tracking
 
 Report bugs or make feature requests at the [GitHub issue tracker](https://github.com/tomaswolf/gerrit-gitblit-plugin/issues).
 
+## Known issues
+
+* For logged-in users, their preference page ("my profile") is accessible but non-functional. Per-user GitBlit preferences cannot be saved. There are no
+  plans to fix this; GitBlit uses a plain file-based mechanism to store such user preferences anyway, which won't scale well for large user bases.
+
+* I have never tried Lucene indexing of repositories through GitBlit. It's functionality I have never needed, and I don't plan to ever do something about
+  it in case it doesn't work.
+
 <hr style="color: #C0C0C0; background-color: #C0C0C0; border-color: #C0C0C0; height: 2px;" />
 <div style="float:right;">
-<a href="https://github.com/tomaswolf/gerrit-gitblit-plugin" target="_blank">GitBlit plugin 2.12.162.1-SNAPSHOT</a>
+<a href="https://github.com/tomaswolf/gerrit-gitblit-plugin" target="_blank">GitBlit plugin 2.12.162.2-SNAPSHOT</a>
 </div>
-
 <script type="text/javascript" src="version_check.js"></script>

pom.xml

@@ -20,7 +20,7 @@ limitations under the License.
   <artifactId>gitblit-plugin</artifactId>
   <description>GitBlit for Gerrit integrated as a plugin</description>
   <name>Gerrit - GitBlit Plugin</name>
-  <version>2.12.162.1-SNAPSHOT</version><!-- Gerrit API version followed by collapsed GitBlit version, followed by plugin version -->
+  <version>2.12.162.2-SNAPSHOT</version><!-- Gerrit API version followed by collapsed GitBlit version, followed by plugin version -->
   <licenses>
     <license>
       <name>Apache License 2.0</name>

src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java

@@ -36,9 +36,12 @@
 import org.eclipse.jgit.util.RawParseUtils;
 
 import com.gitblit.models.PathModel.PathChangeModel;
+import com.gitblit.models.UserModel;
 import com.gitblit.utils.DiffUtils.BinaryDiffHandler;
 import com.gitblit.utils.DiffUtils.DiffStat;
 import com.gitblit.wicket.GitBlitWebApp;
+import com.gitblit.wicket.GitBlitWebSession;
+import com.googlesource.gerrit.plugins.gitblit.auth.GerritGitBlitUserModel;
 
 /**
  * Generates an html snippet of a diff in Gitblit's style, tracks changed paths, and calculates diff stats.
@@ -63,8 +66,13 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 	private static final String GLOBAL_DIFF_LIMIT_KEY = "web.maxDiffLines";
 
 	/**
-	 * Diffs with more lines are not shown in commitdiffs. (Similar to what GitHub does.) Can be reduced (but not increased) through gitblit.properties key
-	 * {@link #DIFF_LIMIT_PER_FILE_KEY}.
+	 * gitblit.properties key for the number of context lines in a diff for a commitdiff if the Gerrit setting is "full file".
+	 */
+	private static final String COMMIT_DIFF_CONTEXT_MAXIMUM_KEY = "web.maxCommitDiffContext";
+
+	/**
+	 * Diffs with more lines are not shown in commitdiffs. (Similar to what GitHub does.) Can be reduced (but not increased) through
+	 * gitblit.properties key {@link #DIFF_LIMIT_PER_FILE_KEY}.
 	 */
 	private static final int DIFF_LIMIT_PER_FILE = 4000;
 
@@ -74,6 +82,12 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 	 */
 	private static final int GLOBAL_DIFF_LIMIT = 20000;
 
+	/**
+	 * Maximum number of diff context lines allowed for commitdiffs. Can be reduced (but not increased) through gitblit.properties key
+	 * {@link #COMMIT_DIFF_CONTEXT_MAXIMUM_KEY}.
+	 */
+	private static final int COMMIT_DIFF_CONTEXT_MAXIMUM = 10;
+
 	private final DiffOutputStream os;
 
 	private final DiffStat diffStat;
@@ -83,17 +97,18 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 	private int left, right;
 
 	/**
-	 * If a single file diff in a commitdiff produces more than this number of lines, we don't display the diff. First, it's too taxing on the browser: it'll
-	 * spend an awful lot of time applying the CSS rules (despite my having optimized them). And second, no human can read a diff with thousands of lines and
-	 * make sense of it.
+	 * If a single file diff in a commitdiff produces more than this number of lines, we don't display the diff. First, it's too taxing on the
+	 * browser: it'll spend an awful lot of time applying the CSS rules (despite my having optimized them). And second, no human can read a diff with
+	 * thousands of lines and make sense of it.
 	 * <p>
 	 * Set to {@link #DIFF_LIMIT_PER_FILE} for commitdiffs, and to -1 (switches off the limit) for single-file diffs.
 	 * </p>
 	 */
 	private final int maxDiffLinesPerFile;
 
 	/**
-	 * Global limit on the number of diff lines. Set to {@link #GLOBAL_DIFF_LIMIT} for commitdiffs, and to -1 (switched off the limit) for single-file diffs.
+	 * Global limit on the number of diff lines. Set to {@link #GLOBAL_DIFF_LIMIT} for commitdiffs, and to -1 (switched off the limit) for single-file
+	 * diffs.
 	 */
 	private final int globalDiffLimit;
 
@@ -122,9 +137,9 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 	private final List<DiffEntry> skipped = new ArrayList<DiffEntry>();
 
 	/**
-	 * A {@link ResettableByteArrayOutputStream} that intercept the "Binary files differ" message produced by the super implementation. Unfortunately the super
-	 * implementation has far too many things private; otherwise we'd just have re-implemented {@link GitBlitDiffFormatter#format(DiffEntry) format(DiffEntry)}
-	 * completely without ever calling the super implementation.
+	 * A {@link ResettableByteArrayOutputStream} that intercept the "Binary files differ" message produced by the super implementation. Unfortunately
+	 * the super implementation has far too many things private; otherwise we'd just have re-implemented
+	 * {@link GitBlitDiffFormatter#format(DiffEntry) format(DiffEntry)} completely without ever calling the super implementation.
 	 */
 	private static class DiffOutputStream extends ResettableByteArrayOutputStream {
 
@@ -162,6 +177,26 @@ public GitBlitDiffFormatter(String commitId, String path, BinaryDiffHandler hand
 		// will only tax the browser too much.
 		maxDiffLinesPerFile = path != null ? -1 : getLimit(DIFF_LIMIT_PER_FILE_KEY, 500, DIFF_LIMIT_PER_FILE);
 		globalDiffLimit = path != null ? -1 : getLimit(GLOBAL_DIFF_LIMIT_KEY, 1000, GLOBAL_DIFF_LIMIT);
+		GitBlitWebSession webSession = GitBlitWebSession.get();
+		if (webSession != null) {
+			UserModel user = webSession.getUser();
+			if (user instanceof GerritGitBlitUserModel) {
+				int context = ((GerritGitBlitUserModel) user).diffContext();
+				if (context < 0) {
+					// Full file. Integer.MAX_VALUE may lead to overflows...
+					context = Integer.MAX_VALUE / 2;
+				}
+				if (path == null) {
+					// For a full commitdiff, limit the number of context lines. Otherwise too many files may end up
+					// being shown as "Diff too large".
+					int maxContext = getLimit(COMMIT_DIFF_CONTEXT_MAXIMUM_KEY, 1, COMMIT_DIFF_CONTEXT_MAXIMUM);
+					if (context > maxContext) {
+						context = maxContext;
+					}
+				}
+				setContext(context);
+			}
+		}
 	}
 
 	/**
@@ -240,8 +275,8 @@ public void format(DiffEntry ent) throws IOException {
 				path = ent.getNewPath();
 				id = ent.getNewId().name();
 			}
-			StringBuilder sb = new StringBuilder(MessageFormat.format("<div class='header'><div class=\"diffHeader\" id=\"n{0}\"><i class=\"icon-file\"></i> ",
-					id));
+			StringBuilder sb = new StringBuilder(MessageFormat.format(
+					"<div class='header'><div class=\"diffHeader\" id=\"n{0}\"><i class=\"icon-file\"></i> ", id));
 			sb.append(StringUtils.escapeForHtml(path, false)).append("</div></div>");
 			sb.append("<div class=\"diff\"><table cellpadding='0'><tbody>\n");
 			os.write(sb.toString().getBytes());
@@ -275,8 +310,8 @@ private void reset() {
 	}
 
 	/**
-	 * Writes an initial table row containing information about added/removed/renamed/copied files. In case of a deletion, we also suppress generating the diff;
-	 * it's not interesting. (All lines removed.)
+	 * Writes an initial table row containing information about added/removed/renamed/copied files. In case of a deletion, we also suppress generating
+	 * the diff; it's not interesting. (All lines removed.)
 	 */
 	private void handleChange() {
 		// XXX Would be nice if we could generate blob links for the cases handled here. Alas, we lack the repo
@@ -421,7 +456,8 @@ protected void writeLine(final char prefix, final RawText text, final int cur) t
 				os.write("<td class='diff-cell remove2'>".getBytes());
 				break;
 			default:
-				os.write(("<th class='diff-line' data-lineno='" + (left++) + "'></th><th class='diff-line' data-lineno='" + (right++) + "'></th>").getBytes());
+				os.write(("<th class='diff-line' data-lineno='" + (left++) + "'></th><th class='diff-line' data-lineno='" + (right++) + "'></th>")
+						.getBytes());
 				os.write("<th class='diff-state'></th>".getBytes());
 				os.write("<td class='diff-cell context2'>".getBytes());
 				break;
@@ -505,8 +541,8 @@ public String getHtml() {
 					sb.append('\n');
 				}
 				if (ChangeType.DELETE.equals(entry.getChangeType())) {
-					sb.append("<span id=\"n" + entry.getOldId().name() + "\">" + StringUtils.escapeForHtml(entry.getOldPath(), false) + ' ' + deletedSuffix
-							+ "</span>");
+					sb.append("<span id=\"n" + entry.getOldId().name() + "\">" + StringUtils.escapeForHtml(entry.getOldPath(), false) + ' '
+							+ deletedSuffix + "</span>");
 				} else {
 					sb.append("<span id=\"n" + entry.getNewId().name() + "\">" + StringUtils.escapeForHtml(entry.getNewPath(), false) + "</span>");
 				}

src/main/java/com/googlesource/gerrit/plugins/gitblit/auth/GerritGitBlitUserManager.java

@@ -34,6 +34,7 @@
 import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.GetDiffPreferences;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -51,9 +52,11 @@ public class GerritGitBlitUserManager implements IUserManager {
 
 	private final Provider<AnonymousUser> anonymousUser;
 
+	private final GetDiffPreferences getDiffPreferences;
+
 	@Inject
 	public GerritGitBlitUserManager(final ProjectControl.GenericFactory projectControl, final GitBlitSettings settings,
-			final DynamicItem<WebSession> gerritSession, final Provider<AnonymousUser> anonymousUser) {
+			final DynamicItem<WebSession> gerritSession, final Provider<AnonymousUser> anonymousUser, final GetDiffPreferences getDiffPreferences) {
 		this.projectControl = projectControl;
 		this.userProvider = new Provider<CurrentUser>() {
 			@Override
@@ -62,6 +65,7 @@ public CurrentUser get() {
 			}
 		};
 		this.anonymousUser = anonymousUser;
+		this.getDiffPreferences = getDiffPreferences;
 		if (!settings.getBoolean(Keys.web.authenticateViewPages, false) && !fixAnonymousUser()) {
 			settings.saveSettings(ImmutableMap.of(Keys.web.authenticateViewPages, Boolean.TRUE.toString()));
 		}
@@ -84,9 +88,9 @@ public void setup(IRuntimeManager runtimeManager) {
 	@Override
 	public UserModel getUserModel(String username) {
 		if (username == null || GerritGitBlitUserModel.ANONYMOUS_USER.equals(username)) {
-			return new GerritGitBlitUserModel(projectControl, anonymousUser);
+			return new GerritGitBlitUserModel(projectControl, anonymousUser, getDiffPreferences);
 		}
-		return new GerritGitBlitUserModel(username, projectControl, userProvider);
+		return new GerritGitBlitUserModel(username, projectControl, userProvider, getDiffPreferences);
 	}
 
 	/**
@@ -99,7 +103,7 @@ public UserModel getUnnamedGerritUser() {
 		CurrentUser user = userProvider.get();
 		if (!user.isIdentifiedUser()) {
 			log.warn("\"Logged-in\" user according to session is anonymous.");
-			return new GerritGitBlitUserModel(projectControl, anonymousUser);
+			return new GerritGitBlitUserModel(projectControl, anonymousUser, getDiffPreferences);
 		}
 		IdentifiedUser loggedInUser = (IdentifiedUser) user;
 		// We know that this user has no username. Synthesize one for GitBlit.
@@ -110,7 +114,7 @@ public UserModel getUnnamedGerritUser() {
 				fakeUserName = "external" + loggedInUser.getAccountId().toString();
 			}
 		}
-		return new GerritGitBlitUserModel(fakeUserName, projectControl, userProvider);
+		return new GerritGitBlitUserModel(fakeUserName, projectControl, userProvider, getDiffPreferences);
 	}
 
 	@Override
@@ -251,7 +255,7 @@ private boolean fixAnonymousUser() {
 					modifiers.setAccessible(true);
 					int modifierFlags = anonymousField.getModifiers();
 					modifiers.set(anonymousField, modifierFlags & ~Modifier.FINAL); // Remove "final" from the "ANONYMOUS" field
-					anonymousField.set(null, new GerritGitBlitUserModel(projectControl, anonymousUser));
+					anonymousField.set(null, new GerritGitBlitUserModel(projectControl, anonymousUser, getDiffPreferences));
 					modifiers.set(anonymousField, modifierFlags); // Make the field "final" again.
 					modifiers.setAccessible(false); // Re-enable Java-language accessibility checks
 				}

src/main/java/com/googlesource/gerrit/plugins/gitblit/auth/GerritGitBlitUserModel.java

@@ -15,15 +15,21 @@
 
 import java.io.IOException;
 
+import org.eclipse.jgit.errors.ConfigInvalidException;
+
 import com.gitblit.Constants.AccessPermission;
 import com.gitblit.Constants.AccessRestrictionType;
 import com.gitblit.models.RepositoryModel;
 import com.gitblit.models.UserModel;
 import com.gitblit.utils.StringUtils;
 import com.google.common.base.Strings;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.reviewdb.client.Project.NameKey;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountResource;
+import com.google.gerrit.server.account.GetDiffPreferences;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.RefControl;
@@ -40,22 +46,26 @@ public class GerritGitBlitUserModel extends UserModel {
 
 	private transient final ProjectControl.GenericFactory projectControlFactory;
 	private transient final Provider<? extends CurrentUser> userProvider;
+	private transient final GetDiffPreferences getDiffPreferences;
 
-	public GerritGitBlitUserModel(final ProjectControl.GenericFactory projectControlFactory, final Provider<? extends CurrentUser> userProvider) {
+	public GerritGitBlitUserModel(final ProjectControl.GenericFactory projectControlFactory, final Provider<? extends CurrentUser> userProvider,
+			final GetDiffPreferences getDiffPreferences) {
 		super(ANONYMOUS_USER);
 		this.isAuthenticated = false;
 		this.projectControlFactory = projectControlFactory;
 		this.userProvider = userProvider;
 		this.displayName = this.username;
+		this.getDiffPreferences = getDiffPreferences;
 	}
 
 	public GerritGitBlitUserModel(String username, final ProjectControl.GenericFactory projectControlFactory,
-			final Provider<? extends CurrentUser> userProvider) {
+			final Provider<? extends CurrentUser> userProvider, final GetDiffPreferences getDiffPreferences) {
 		super(username);
 		this.username = username;
 		this.isAuthenticated = true;
 		this.projectControlFactory = projectControlFactory;
 		this.userProvider = userProvider;
+		this.getDiffPreferences = getDiffPreferences;
 		CurrentUser user = userProvider.get();
 		if (user != null && user.isIdentifiedUser()) {
 			this.displayName = ((IdentifiedUser) user).getAccount().getFullName();
@@ -112,4 +122,26 @@ public boolean canView(RepositoryModel repository, String ref) {
 		return false;
 	}
 
+	/**
+	 * Retrieves the Gerrit preference setting for the number of diff context lines. A value < 0 indicates a "full file" context. If the current user
+	 * is not logged in, returns the Gitblit (and JGit) default of 3, otherwise the setting as configured by the user in his Gerrit settings.
+	 * 
+	 * @return the number of context lines to display in a diff, or < 0 if the whole file shall be shown.
+	 */
+	public int diffContext() {
+		CurrentUser user = userProvider.get();
+		if (user != null && user.isIdentifiedUser()) {
+			AccountResource accountRsc = new AccountResource((IdentifiedUser) user);
+			try {
+				DiffPreferencesInfo diffPrefs = getDiffPreferences.apply(accountRsc);
+				if (diffPrefs != null) {
+					return diffPrefs.context;
+				}
+			} catch (AuthException | ConfigInvalidException | IOException e) {
+				// Ignore and return default below.
+			}
+		}
+		// This is the DiffFormatter default, and what Gitblit normally uses. The Gerrit default is 10.
+		return 3;
+	}
 }