Unit testing. Documentation. Simplified settings classes.

9 files changed, 184 insertions, 231 deletions

diff --git a/src/com/gitblit/FileSettings.java b/src/com/gitblit/FileSettings.java
index 01176c03..b70daa0f 100644
--- a/src/com/gitblit/FileSettings.java
+++ b/src/com/gitblit/FileSettings.java
@@ -18,144 +18,42 @@ package com.gitblit;
 import java.io.File;

 import java.io.FileInputStream;

 import java.io.FileNotFoundException;

-import java.util.ArrayList;

-import java.util.List;

 import java.util.Properties;

-import java.util.regex.PatternSyntaxException;

-

-import org.slf4j.Logger;

-import org.slf4j.LoggerFactory;

 

 /**

  * Reads GitBlit settings file.

  * 

  */

-public class FileSettings implements IStoredSettings {

-

-	private final Logger logger = LoggerFactory.getLogger(FileSettings.class);

+public class FileSettings extends IStoredSettings {

 

 	private final File propertiesFile;

 

-	private Properties properties = new Properties();

+	private final Properties properties = new Properties();

 

-	private long lastread;

+	private volatile long lastread;

 

 	public FileSettings(String file) {

+		super(FileSettings.class);

 		this.propertiesFile = new File(file);

 	}

 

 	@Override

-	public List<String> getAllKeys(String startingWith) {

-		startingWith = startingWith.toLowerCase();

-		List<String> keys = new ArrayList<String>();

-		Properties props = read();

-		for (Object o : props.keySet()) {

-			String key = o.toString().toLowerCase();

-			if (key.startsWith(startingWith)) {

-				keys.add(key);

-			}

-		}

-		return keys;

-	}

-

-	@Override

-	public boolean getBoolean(String name, boolean defaultValue) {

-		Properties props = read();

-		if (props.containsKey(name)) {

-			try {

-				String value = props.getProperty(name);

-				if (value != null && value.trim().length() > 0) {

-					return Boolean.parseBoolean(value);

-				}

-			} catch (Exception e) {

-				logger.warn("No override setting for " + name + " using default of " + defaultValue);

-			}

-		}

-		return defaultValue;

-	}

-

-	@Override

-	public int getInteger(String name, int defaultValue) {

-		Properties props = read();

-		if (props.containsKey(name)) {

-			try {

-				String value = props.getProperty(name);

-				if (value != null && value.trim().length() > 0) {

-					return Integer.parseInt(value);

-				}

-			} catch (Exception e) {

-				logger.warn("No override setting for " + name + " using default of " + defaultValue);

-			}

-		}

-		return defaultValue;

-	}

-

-	@Override

-	public String getString(String name, String defaultValue) {

-		Properties props = read();

-		if (props.containsKey(name)) {

-			try {

-				String value = props.getProperty(name);

-				if (value != null) {

-					return value;

-				}

-			} catch (Exception e) {

-				logger.warn("No override setting for " + name + " using default of " + defaultValue);

-			}

-		}

-		return defaultValue;

-	}

-

-	@Override

-	public List<String> getStrings(String name) {

-		return getStrings(name, " ");

-	}

-

-	@Override

-	public List<String> getStringsFromValue(String value) {

-		return getStringsFromValue(value, " ");

-	}

-

-	@Override

-	public List<String> getStrings(String name, String separator) {

-		List<String> strings = new ArrayList<String>();

-		Properties props = read();

-		if (props.containsKey(name)) {

-			String value = props.getProperty(name);

-			strings = getStringsFromValue(value, separator);

-		}

-		return strings;

-	}

-

-	@Override

-	public List<String> getStringsFromValue(String value, String separator) {

-		List<String> strings = new ArrayList<String>();

-		try {

-			String[] chunks = value.split(separator);

-			for (String chunk : chunks) {

-				chunk = chunk.trim();

-				if (chunk.length() > 0) {

-					strings.add(chunk);

-				}

-			}

-		} catch (PatternSyntaxException e) {

-			logger.error("Failed to parse " + value, e);

-		}

-		return strings;

-	}

-

-	private synchronized Properties read() {

+	protected synchronized Properties read() {

 		if (propertiesFile.exists() && (propertiesFile.lastModified() > lastread)) {

 			FileInputStream is = null;

 			try {

-				properties = new Properties();

+				Properties props = new Properties();

 				is = new FileInputStream(propertiesFile);

-				properties.load(is);

+				props.load(is);

+				

+				// load properties after we have successfully read file

+				properties.clear();

+				properties.putAll(props);

 				lastread = propertiesFile.lastModified();

 			} catch (FileNotFoundException f) {

 				// IGNORE - won't happen because file.exists() check above

 			} catch (Throwable t) {

-				t.printStackTrace();

+				logger.error("Failed to read " + propertiesFile.getName(), t);

 			} finally {

 				if (is != null) {

 					try {

diff --git a/src/com/gitblit/GitBlit.java b/src/com/gitblit/GitBlit.java
index dcf5a6b2..01326230 100644
--- a/src/com/gitblit/GitBlit.java
+++ b/src/com/gitblit/GitBlit.java
@@ -312,17 +312,6 @@ public class GitBlit implements ServletContextListener {
 		return false;

 	}

 

-	public boolean renameRepository(RepositoryModel model, String newName) {

-		File folder = new File(repositoriesFolder, model.name);

-		if (folder.exists() && folder.isDirectory()) {

-			File newFolder = new File(repositoriesFolder, newName);

-			if (folder.renameTo(newFolder)) {

-				return loginService.renameRole(model.name, newName);

-			}

-		}

-		return false;

-	}

-

 	public void configureContext(IStoredSettings settings) {

 		logger.info("Reading configuration from " + settings.toString());

 		this.storedSettings = settings;

@@ -334,6 +323,7 @@ public class GitBlit implements ServletContextListener {
 	@Override

 	public void contextInitialized(ServletContextEvent contextEvent) {

 		if (storedSettings == null) {

+			// for running gitblit as a traditional webapp in a servlet container

 			WebXmlSettings webxmlSettings = new WebXmlSettings(contextEvent.getServletContext());

 			configureContext(webxmlSettings);

 		}

@@ -341,6 +331,6 @@ public class GitBlit implements ServletContextListener {
 

 	@Override

 	public void contextDestroyed(ServletContextEvent contextEvent) {

-		logger.info("GitBlit context destroyed by servlet container.");

+		logger.info("Gitblit context destroyed by servlet container.");

 	}

 }

diff --git a/src/com/gitblit/IStoredSettings.java b/src/com/gitblit/IStoredSettings.java
index 07d21e25..7108c068 100644
--- a/src/com/gitblit/IStoredSettings.java
+++ b/src/com/gitblit/IStoredSettings.java
@@ -15,24 +15,87 @@
  */

 package com.gitblit;

 

+import java.util.ArrayList;

 import java.util.List;

+import java.util.Properties;

 

-public interface IStoredSettings {

+import org.slf4j.Logger;

+import org.slf4j.LoggerFactory;

 

-	List<String> getAllKeys(String startingWith);

+import com.gitblit.utils.StringUtils;

 

-	boolean getBoolean(String name, boolean defaultValue);

+public abstract class IStoredSettings {

 

-	int getInteger(String name, int defaultValue);

+	protected final Logger logger;

+	

+	public IStoredSettings(Class<? extends IStoredSettings> clazz) {

+		logger = LoggerFactory.getLogger(clazz);

+	}

+		

+	protected abstract Properties read();

 

-	String getString(String name, String defaultValue);

+	public List<String> getAllKeys(String startingWith) {

+		startingWith = startingWith.toLowerCase();

+		List<String> keys = new ArrayList<String>();

+		Properties props = read();

+		for (Object o : props.keySet()) {

+			String key = o.toString();

+			if (key.toLowerCase().startsWith(startingWith)) {

+				keys.add(key);

+			}

+		}

+		return keys;

+	}

 

-	List<String> getStrings(String name);

+	public boolean getBoolean(String name, boolean defaultValue) {

+		Properties props = read();

+		if (props.containsKey(name)) {

+			String value = props.getProperty(name);

+			if (!StringUtils.isEmpty(value)) {

+				return Boolean.parseBoolean(value);

+			}

+		}

+		return defaultValue;

+	}

 

-	List<String> getStringsFromValue(String value);

+	public int getInteger(String name, int defaultValue) {

+		Properties props = read();

+		if (props.containsKey(name)) {

+			try {

+				String value = props.getProperty(name);

+				if (!StringUtils.isEmpty(value)) {

+					return Integer.parseInt(value);

+				}

+			} catch (NumberFormatException e) {

+				logger.warn("Failed to parse integer for " + name + " using default of "

+						+ defaultValue);

+			}

+		}

+		return defaultValue;

+	}

 

-	List<String> getStrings(String name, String separator);

+	public String getString(String name, String defaultValue) {

+		Properties props = read();

+		if (props.containsKey(name)) {

+			String value = props.getProperty(name);

+			if (value != null) {

+				return value;

+			}

+		}

+		return defaultValue;

+	}

 

-	List<String> getStringsFromValue(String value, String separator);

+	public List<String> getStrings(String name) {

+		return getStrings(name, " ");

+	}

 

+	public List<String> getStrings(String name, String separator) {

+		List<String> strings = new ArrayList<String>();

+		Properties props = read();

+		if (props.containsKey(name)) {

+			String value = props.getProperty(name);

+			strings = StringUtils.getStringsFromValue(value, separator);

+		}

+		return strings;

+	}

 }
\ No newline at end of file
diff --git a/src/com/gitblit/WebXmlSettings.java b/src/com/gitblit/WebXmlSettings.java
index 6ca38f78..0ff2a3e7 100644
--- a/src/com/gitblit/WebXmlSettings.java
+++ b/src/com/gitblit/WebXmlSettings.java
@@ -15,62 +15,28 @@
  */

 package com.gitblit;

 

-import java.util.List;

+import java.util.Enumeration;

+import java.util.Properties;

 

 import javax.servlet.ServletContext;

 

-public class WebXmlSettings implements IStoredSettings {

+public class WebXmlSettings extends IStoredSettings {

 

+	private final Properties properties = new Properties();

+	

 	public WebXmlSettings(ServletContext context) {

-

-	}

-

-	@Override

-	public List<String> getAllKeys(String startingWith) {

-		// TODO Auto-generated method stub

-		return null;

-	}

-

-	@Override

-	public boolean getBoolean(String name, boolean defaultValue) {

-		// TODO Auto-generated method stub

-		return false;

-	}

-

-	@Override

-	public int getInteger(String name, int defaultValue) {

-		// TODO Auto-generated method stub

-		return 0;

-	}

-

-	@Override

-	public String getString(String name, String defaultValue) {

-		// TODO Auto-generated method stub

-		return null;

-	}

-

-	@Override

-	public List<String> getStrings(String name) {

-		// TODO Auto-generated method stub

-		return null;

-	}

-

-	@Override

-	public List<String> getStringsFromValue(String value) {

-		// TODO Auto-generated method stub

-		return null;

-	}

-

-	@Override

-	public List<String> getStrings(String name, String separator) {

-		// TODO Auto-generated method stub

-		return null;

-	}

-

-	@Override

-	public List<String> getStringsFromValue(String value, String separator) {

-		// TODO Auto-generated method stub

-		return null;

+		super(WebXmlSettings.class);

+		Enumeration<?> keys = context.getInitParameterNames();

+		while (keys.hasMoreElements()) {

+			String key = keys.nextElement().toString();

+			String value = context.getInitParameter(key);

+			properties.put(key, value);

+		}

+	}

+	

+	@Override

+	protected Properties read() {

+		return properties;

 	}

 

 	@Override

diff --git a/src/com/gitblit/utils/DiffUtils.java b/src/com/gitblit/utils/DiffUtils.java
index 0f569074..c1401f96 100644
--- a/src/com/gitblit/utils/DiffUtils.java
+++ b/src/com/gitblit/utils/DiffUtils.java
@@ -25,6 +25,7 @@ import org.eclipse.jgit.diff.DiffEntry;
 import org.eclipse.jgit.diff.DiffFormatter;

 import org.eclipse.jgit.diff.RawText;

 import org.eclipse.jgit.diff.RawTextComparator;

+import org.eclipse.jgit.lib.Constants;

 import org.eclipse.jgit.lib.Repository;

 import org.eclipse.jgit.revwalk.RevCommit;

 import org.eclipse.jgit.revwalk.RevTree;

@@ -69,6 +70,7 @@ public class DiffUtils {
 

 	public static String getDiff(Repository r, RevCommit baseCommit, RevCommit commit, String path,

 			DiffOutputType outputType) {

+		String diff = null;

 		try {

 			RevTree baseTree;

 			if (baseCommit == null) {

@@ -107,18 +109,17 @@ public class DiffUtils {
 			df.setRepository(r);

 			df.setDiffComparator(cmp);

 			df.setDetectRenames(true);

-			List<DiffEntry> diffs = df.scan(baseTree, commitTree);

+			List<DiffEntry> diffEntries = df.scan(baseTree, commitTree);

 			if (path != null && path.length() > 0) {

-				for (DiffEntry diff : diffs) {

-					if (diff.getNewPath().equalsIgnoreCase(path)) {

-						df.format(diff);

+				for (DiffEntry diffEntry : diffEntries) {

+					if (diffEntry.getNewPath().equalsIgnoreCase(path)) {

+						df.format(diffEntry);

 						break;

 					}

 				}

 			} else {

-				df.format(diffs);

+				df.format(diffEntries);

 			}

-			String diff;

 			if (df instanceof GitWebDiffFormatter) {

 				// workaround for complex private methods in DiffFormatter

 				diff = ((GitWebDiffFormatter) df).getHtml();

@@ -126,15 +127,15 @@ public class DiffUtils {
 				diff = os.toString();

 			}

 			df.flush();

-			return diff;

 		} catch (Throwable t) {

 			LOGGER.error("failed to generate commit diff!", t);

 		}

-		return null;

+		return diff;

 	}

 

 	public static String getCommitPatch(Repository r, RevCommit baseCommit, RevCommit commit,

 			String path) {

+		String diff = null;

 		try {

 			RevTree baseTree;

 			if (baseCommit == null) {

@@ -159,29 +160,31 @@ public class DiffUtils {
 			df.setRepository(r);

 			df.setDiffComparator(cmp);

 			df.setDetectRenames(true);

-			List<DiffEntry> diffs = df.scan(baseTree, commitTree);

+			List<DiffEntry> diffEntries = df.scan(baseTree, commitTree);

 			if (path != null && path.length() > 0) {

-				for (DiffEntry diff : diffs) {

-					if (diff.getNewPath().equalsIgnoreCase(path)) {

-						df.format(diff);

+				for (DiffEntry diffEntry : diffEntries) {

+					if (diffEntry.getNewPath().equalsIgnoreCase(path)) {

+						df.format(diffEntry);

 						break;

 					}

 				}

 			} else {

-				df.format(diffs);

+				df.format(diffEntries);

 			}

-			String diff = df.getPatch(commit);

+			diff = df.getPatch(commit);

 			df.flush();

-			return diff;

 		} catch (Throwable t) {

 			LOGGER.error("failed to generate commit diff!", t);

 		}

-		return null;

+		return diff;

 	}

 

 	public static List<AnnotatedLine> blame(Repository r, String blobPath, String objectId) {

 		List<AnnotatedLine> lines = new ArrayList<AnnotatedLine>();

 		try {

+			if (StringUtils.isEmpty(objectId)) {

+				objectId = Constants.HEAD;

+			}

 			BlameCommand blameCommand = new BlameCommand(r);

 			blameCommand.setFilePath(blobPath);

 			blameCommand.setStartCommit(r.resolve(objectId));

diff --git a/src/com/gitblit/utils/StringUtils.java b/src/com/gitblit/utils/StringUtils.java
index a881b58c..fa84fe8f 100644
--- a/src/com/gitblit/utils/StringUtils.java
+++ b/src/com/gitblit/utils/StringUtils.java
@@ -18,7 +18,9 @@ package com.gitblit.utils;
 import java.io.UnsupportedEncodingException;

 import java.security.MessageDigest;

 import java.security.NoSuchAlgorithmException;

+import java.util.ArrayList;

 import java.util.List;

+import java.util.regex.PatternSyntaxException;

 

 public class StringUtils {

 

@@ -142,4 +144,24 @@ public class StringUtils {
 		}

 		return relativePath;

 	}

+	

+	public static List<String> getStringsFromValue(String value) {

+		return getStringsFromValue(value, " ");

+	}

+		

+	public static List<String> getStringsFromValue(String value, String separator) {

+		List<String> strings = new ArrayList<String>();

+		try {

+			String[] chunks = value.split(separator);

+			for (String chunk : chunks) {

+				chunk = chunk.trim();

+				if (chunk.length() > 0) {

+					strings.add(chunk);

+				}

+			}

+		} catch (PatternSyntaxException e) {

+			throw new RuntimeException(e);

+		}

+		return strings;

+	}

 }

diff --git a/src/com/gitblit/wicket/pages/CommitPage.java b/src/com/gitblit/wicket/pages/CommitPage.java
index 02956001..6da962ef 100644
--- a/src/com/gitblit/wicket/pages/CommitPage.java
+++ b/src/com/gitblit/wicket/pages/CommitPage.java
@@ -38,7 +38,6 @@ import com.gitblit.models.GitNote;
 import com.gitblit.models.PathModel.PathChangeModel;

 import com.gitblit.utils.JGitUtils;

 import com.gitblit.utils.JGitUtils.SearchType;

-import com.gitblit.utils.StringUtils;

 import com.gitblit.wicket.WicketUtils;

 import com.gitblit.wicket.panels.CommitHeaderPanel;

 import com.gitblit.wicket.panels.CommitLegendPanel;

@@ -129,7 +128,7 @@ public class CommitPage extends RepositoryPage {
 						SearchType.AUTHOR));

 				item.add(WicketUtils.createTimestampLabel("authorDate", entry.notesRef

 						.getAuthorIdent().getWhen(), getTimeZone()));

-				item.add(new Label("noteContent", StringUtils.breakLinesForHtml(entry.content))

+				item.add(new Label("noteContent", substituteText(entry.content))

 						.setEscapeModelStrings(false));

 			}

 		};

diff --git a/src/com/gitblit/wicket/pages/RepositoryPage.java b/src/com/gitblit/wicket/pages/RepositoryPage.java
index e2120f77..cff59f26 100644
--- a/src/com/gitblit/wicket/pages/RepositoryPage.java
+++ b/src/com/gitblit/wicket/pages/RepositoryPage.java
@@ -227,40 +227,48 @@ public abstract class RepositoryPage extends BasePage {
 	}

 

 	protected void addFullText(String wicketId, String text, boolean substituteRegex) {

-		String html = StringUtils.breakLinesForHtml(text);

+		String html;

 		if (substituteRegex) {

-			Map<String, String> map = new HashMap<String, String>();

-			// global regex keys

-			if (GitBlit.getBoolean(Keys.regex.global, false)) {

-				for (String key : GitBlit.getAllKeys(Keys.regex.global)) {

-					if (!key.equals(Keys.regex.global)) {

-						String subKey = key.substring(key.lastIndexOf('.') + 1);

-						map.put(subKey, GitBlit.getString(key, ""));

-					}

+			html = substituteText(text);

+		} else {

+			html = StringUtils.breakLinesForHtml(text);

+		}

+		add(new Label(wicketId, html).setEscapeModelStrings(false));

+	}

+

+	protected String substituteText(String text) {

+		String html = StringUtils.breakLinesForHtml(text);

+		Map<String, String> map = new HashMap<String, String>();

+		// global regex keys

+		if (GitBlit.getBoolean(Keys.regex.global, false)) {

+			for (String key : GitBlit.getAllKeys(Keys.regex.global)) {

+				if (!key.equals(Keys.regex.global)) {

+					String subKey = key.substring(key.lastIndexOf('.') + 1);

+					map.put(subKey, GitBlit.getString(key, ""));

 				}

 			}

+		}

 

-			// repository-specific regex keys

-			List<String> keys = GitBlit.getAllKeys(Keys.regex._ROOT + "."

-					+ repositoryName.toLowerCase());

-			for (String key : keys) {

-				String subKey = key.substring(key.lastIndexOf('.') + 1);

-				map.put(subKey, GitBlit.getString(key, ""));

-			}

+		// repository-specific regex keys

+		List<String> keys = GitBlit.getAllKeys(Keys.regex._ROOT + "."

+				+ repositoryName.toLowerCase());

+		for (String key : keys) {

+			String subKey = key.substring(key.lastIndexOf('.') + 1);

+			map.put(subKey, GitBlit.getString(key, ""));

+		}

 

-			for (Entry<String, String> entry : map.entrySet()) {

-				String definition = entry.getValue().trim();

-				String[] chunks = definition.split("!!!");

-				if (chunks.length == 2) {

-					html = html.replaceAll(chunks[0], chunks[1]);

-				} else {

-					logger.warn(entry.getKey()

-							+ " improperly formatted.  Use !!! to separate match from replacement: "

-							+ definition);

-				}

+		for (Entry<String, String> entry : map.entrySet()) {

+			String definition = entry.getValue().trim();

+			String[] chunks = definition.split("!!!");

+			if (chunks.length == 2) {

+				html = html.replaceAll(chunks[0], chunks[1]);

+			} else {

+				logger.warn(entry.getKey()

+						+ " improperly formatted.  Use !!! to separate match from replacement: "

+						+ definition);

 			}

 		}

-		add(new Label(wicketId, html).setEscapeModelStrings(false));

+		return html;

 	}

 

 	protected abstract String getPageName();

diff --git a/src/com/gitblit/wicket/resources/gitblit.css b/src/com/gitblit/wicket/resources/gitblit.css
index 43094588..7143f854 100644
--- a/src/com/gitblit/wicket/resources/gitblit.css
+++ b/src/com/gitblit/wicket/resources/gitblit.css
@@ -249,6 +249,10 @@ div.commit_message {
 	border-width: 1px 0px 0px;

 }

 

+div.commit_message a {

+	font-family: monospace;

+}

+

 div.bug_open, span.bug_open {

 	padding: 2px;

 	background-color: #803333;