Merge "Don't allow null name or e-mail in PersonIdent"

3 files changed, 47 insertions, 38 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java
index 34a557bd92..1b7276bf77 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java
@@ -73,4 +73,14 @@ public class T0001_PersonIdentTest {
 		assertEquals("A U Thor <author@example.com> 1142878501 +0230",
 				p.toExternalString());
 	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void nullForNameShouldThrowIllegalArgumentException() {
+		new PersonIdent(null, "author@example.com");
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void nullForEmailShouldThrowIllegalArgumentException() {
+		new PersonIdent("A U Thor", null);
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java
index a166790a40..ee92083175 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java
@@ -548,9 +548,8 @@ public class CommitCommand extends GitCommand<RevCommit> {
 
 	/**
 	 * Sets the committer for this {@code commit}. If no committer is explicitly
-	 * specified because this method is never called or called with {@code null}
-	 * value then the committer will be deduced from config info in repository,
-	 * with current time.
+	 * specified because this method is never called then the committer will be
+	 * deduced from config info in repository, with current time.
 	 *
 	 * @param name
 	 *            the name of the committer used for the {@code commit}
@@ -591,9 +590,8 @@ public class CommitCommand extends GitCommand<RevCommit> {
 
 	/**
 	 * Sets the author for this {@code commit}. If no author is explicitly
-	 * specified because this method is never called or called with {@code null}
-	 * value then the author will be set to the committer or to the original
-	 * author when amending.
+	 * specified because this method is never called then the author will be set
+	 * to the committer or to the original author when amending.
 	 *
 	 * @param name
 	 *            the name of the author used for the {@code commit}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java
index 1ec8f4c9ea..f81d9c5a84 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java
@@ -78,11 +78,7 @@ public class PersonIdent implements Serializable {
 	 * @param repo
 	 */
 	public PersonIdent(final Repository repo) {
-		final UserConfig config = repo.getConfig().get(UserConfig.KEY);
-		name = config.getCommitterName();
-		emailAddress = config.getCommitterEmail();
-		when = SystemReader.getInstance().getCurrentTime();
-		tzOffset = SystemReader.getInstance().getTimezone(when);
+		this(repo.getConfig().get(UserConfig.KEY));
 	}
 
 	/**
@@ -102,10 +98,7 @@ public class PersonIdent implements Serializable {
 	 * @param aEmailAddress
 	 */
 	public PersonIdent(final String aName, final String aEmailAddress) {
-		name = aName;
-		emailAddress = aEmailAddress;
-		when = SystemReader.getInstance().getCurrentTime();
-		tzOffset = SystemReader.getInstance().getTimezone(when);
+		this(aName, aEmailAddress, SystemReader.getInstance().getCurrentTime());
 	}
 
 	/**
@@ -131,10 +124,7 @@ public class PersonIdent implements Serializable {
 	 *            local time
 	 */
 	public PersonIdent(final PersonIdent pi, final Date aWhen) {
-		name = pi.getName();
-		emailAddress = pi.getEmailAddress();
-		when = aWhen.getTime();
-		tzOffset = pi.tzOffset;
+		this(pi.getName(), pi.getEmailAddress(), aWhen.getTime(), pi.tzOffset);
 	}
 
 	/**
@@ -149,43 +139,54 @@ public class PersonIdent implements Serializable {
 	 */
 	public PersonIdent(final String aName, final String aEmailAddress,
 			final Date aWhen, final TimeZone aTZ) {
-		name = aName;
-		emailAddress = aEmailAddress;
-		when = aWhen.getTime();
-		tzOffset = aTZ.getOffset(when) / (60 * 1000);
+		this(aName, aEmailAddress, aWhen.getTime(), aTZ.getOffset(aWhen
+				.getTime()) / (60 * 1000));
 	}
 
 	/**
-	 * Construct a {@link PersonIdent}
+	 * Copy a PersonIdent, but alter the clone's time stamp
 	 *
-	 * @param aName
-	 * @param aEmailAddress
+	 * @param pi
+	 *            original {@link PersonIdent}
 	 * @param aWhen
 	 *            local time stamp
 	 * @param aTZ
 	 *            time zone
 	 */
-	public PersonIdent(final String aName, final String aEmailAddress,
-			final long aWhen, final int aTZ) {
-		name = aName;
-		emailAddress = aEmailAddress;
-		when = aWhen;
-		tzOffset = aTZ;
+	public PersonIdent(final PersonIdent pi, final long aWhen, final int aTZ) {
+		this(pi.getName(), pi.getEmailAddress(), aWhen, aTZ);
+	}
+
+	private PersonIdent(final String aName, final String aEmailAddress,
+			long when) {
+		this(aName, aEmailAddress, when, SystemReader.getInstance()
+				.getTimezone(when));
+	}
+
+	private PersonIdent(final UserConfig config) {
+		this(config.getCommitterName(), config.getCommitterEmail());
 	}
 
 	/**
-	 * Copy a PersonIdent, but alter the clone's time stamp
+	 * Construct a {@link PersonIdent}
 	 *
-	 * @param pi
-	 *            original {@link PersonIdent}
+	 * @param aName
+	 * @param aEmailAddress
 	 * @param aWhen
 	 *            local time stamp
 	 * @param aTZ
 	 *            time zone
 	 */
-	public PersonIdent(final PersonIdent pi, final long aWhen, final int aTZ) {
-		name = pi.getName();
-		emailAddress = pi.getEmailAddress();
+	public PersonIdent(final String aName, final String aEmailAddress,
+			final long aWhen, final int aTZ) {
+		if (aName == null)
+			throw new IllegalArgumentException(
+					"Name of PersonIdent must not be null.");
+		if (aEmailAddress == null)
+			throw new IllegalArgumentException(
+					"E-mail address of PersonIdent must not be null.");
+		name = aName;
+		emailAddress = aEmailAddress;
 		when = aWhen;
 		tzOffset = aTZ;
 	}