Skip to content

Commit

Permalink
[Fix JENKINS-30073] changeset time shouldn't be -1
Browse files Browse the repository at this point in the history 
The changeset time parsing implemented in an earlier commit did not
account for the most common format of the "nearly ISO" date format
generated by the "+%ci" format argument.

The fully ISO 8601 compliant date format argument ( +%cI ) is not
available in all the git versions supported by the plugin, so the plugin
continues to use the nearly ISO 8601 compliant format ( +%ci ), then
transforms it into an ISO 8601 format when the timestamp is requested.

For example, git 2.1.4 as shipped with Debian 8 does not recognize +%cI
for date formatting.  Git 2.6.0 version does recognizes +%cI.
  • Loading branch information
@MarkEWaite
MarkEWaite committed Oct 17, 2015
1 parent 4979ace commit b84c7c9
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 4 deletions.
77 changes: 75 additions & 2 deletions src/main/java/hudson/plugins/git/GitChangeSet.java
View file
Expand Up @@ -9,7 +9,6 @@
import hudson.scm.ChangeLogSet.AffectedFile;
import hudson.scm.EditType;
import org.apache.commons.lang.math.NumberUtils;
import org.apache.commons.lang.time.FastDateFormat;
import org.kohsuke.stapler.export.Exported;
import org.kohsuke.stapler.export.ExportedBean;

Expand All @@ -23,11 +22,19 @@
import java.util.HashSet;
import java.util.List;
import java.util.TimeZone;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static hudson.Util.fixEmpty;

import org.joda.time.DateTime;
import org.joda.time.DateTimeFieldType;
import org.joda.time.format.DateTimeFormatter;
import org.joda.time.format.DateTimeFormatterBuilder;
import org.joda.time.format.ISODateTimeFormat;

/**
* Represents a change set.
* @author Nigel Magnay
Expand All @@ -49,6 +56,10 @@ public class GitChangeSet extends ChangeLogSet.Entry {
private static final String ISO_8601 = "yyyy-MM-dd'T'HH:mm:ss";
private static final String ISO_8601_WITH_TZ = "yyyy-MM-dd'T'HH:mm:ssX";

private final DateTimeFormatter [] dateFormatters;

public static final Logger LOGGER = Logger.getLogger(GitChangeSet.class.getName());

/**
* This is broken as a part of the 1.5 refactoring.
*
Expand Down Expand Up @@ -92,6 +103,51 @@ public GitChangeSet(List<String> lines, boolean authorOrCommitter) {
if (lines.size() > 0) {
parseCommit(lines);
}

// Nearly ISO dates generated by git whatchanged --format=+ci
// Look like '2015-09-30 08:21:24 -0600'
// ISO is '2015-09-30T08:21:24-06:00'
// Uses Builder rather than format pattern for more reliable parsing
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
builder.appendFixedDecimal(DateTimeFieldType.year(), 4);
builder.appendLiteral('-');
builder.appendFixedDecimal(DateTimeFieldType.monthOfYear(), 2);
builder.appendLiteral('-');
builder.appendFixedDecimal(DateTimeFieldType.dayOfMonth(), 2);
builder.appendLiteral(' ');
builder.appendFixedDecimal(DateTimeFieldType.hourOfDay(), 2);
builder.appendLiteral(':');
builder.appendFixedDecimal(DateTimeFieldType.minuteOfHour(), 2);
builder.appendLiteral(':');
builder.appendFixedDecimal(DateTimeFieldType.secondOfMinute(), 2);
builder.appendLiteral(' ');
builder.appendTimeZoneOffset(null, false, 2, 2);
DateTimeFormatter gitDateFormatter = builder.toFormatter();

// DateTimeFormat.forPattern("yyyy-MM-DDTHH:mm:ssZ");
// 2013-03-21T15:16:44+0100
// Uses Builder rather than format pattern for more reliable parsing
builder = new DateTimeFormatterBuilder();
builder.appendFixedDecimal(DateTimeFieldType.year(), 4);
builder.appendLiteral('-');
builder.appendFixedDecimal(DateTimeFieldType.monthOfYear(), 2);
builder.appendLiteral('-');
builder.appendFixedDecimal(DateTimeFieldType.dayOfMonth(), 2);
builder.appendLiteral('T');
builder.appendFixedDecimal(DateTimeFieldType.hourOfDay(), 2);
builder.appendLiteral(':');
builder.appendFixedDecimal(DateTimeFieldType.minuteOfHour(), 2);
builder.appendLiteral(':');
builder.appendFixedDecimal(DateTimeFieldType.secondOfMinute(), 2);
builder.appendTimeZoneOffset(null, false, 2, 2);
DateTimeFormatter nearlyISOFormatter = builder.toFormatter();

DateTimeFormatter isoDateFormat = ISODateTimeFormat.basicDateTimeNoMillis();

dateFormatters = new DateTimeFormatter[3];
dateFormatters[0] = gitDateFormatter; // First priority +%cI format
dateFormatters[1] = nearlyISOFormatter; // Second priority seen in git-plugin
dateFormatters[2] = isoDateFormat; // Third priority, ISO 8601 format
}

private void parseCommit(List<String> lines) {
Expand Down Expand Up @@ -210,8 +266,25 @@ public String getDate() {

@Override
public long getTimestamp() {
String date = getDate();
if (date == null) {
LOGGER.log(Level.WARNING, "Failed to parse null date");
return -1;
}
if (date.isEmpty()) {
LOGGER.log(Level.WARNING, "Failed to parse empty date");
return -1;
}

for (DateTimeFormatter dateFormatter : dateFormatters) {
try {
DateTime dateTime = DateTime.parse(date, dateFormatter);
return dateTime.getMillis();
} catch (IllegalArgumentException ia) {
}
}
try {
return new SimpleDateFormat(ISO_8601_WITH_TZ).parse(getDate()).getTime();
return new SimpleDateFormat(ISO_8601_WITH_TZ).parse(date).getTime();
} catch (ParseException e) {
return -1;
} catch (IllegalArgumentException ia) {
Expand Down
28 changes: 28 additions & 0 deletions src/test/java/hudson/plugins/git/GitChangeSetBasicTest.java
View file
Expand Up @@ -42,6 +42,24 @@ public void testAuthor() {
assertEquals(GitChangeSetUtil.AUTHOR_NAME, genChangeSet(true, false).getAuthorName());
}

@Test
public void testGetDate() {
assertEquals("1970-01-15T06:56:08-0600", genChangeSet(true, false).getDate());
}

@Test
public void testGetTimestamp() {
assertEquals(1256168000L, genChangeSet(true, false).getTimestamp());
}

@Test
public void testInvalidDate() {
final String badDateString = "2015-03-03x09:22:42 -0700";
GitChangeSet c = new GitChangeSet(Arrays.asList("author John Doe <john.doe@jenkins-ci.org> " + badDateString), true);
assertEquals(badDateString, c.getDate());
assertEquals(-1L, c.getTimestamp());
}

@Test
public void testIsoDate() {

Expand Down Expand Up @@ -100,4 +118,14 @@ public void testSwedishCommitterName() {
public void testSwedishAuthorName() {
assertEquals("misterÅ", genChangeSetForSwedCase(true).getAuthorName());
}

@Test
public void testSwedishDate() {
assertEquals("2013-03-21T15:16:44+0100", genChangeSetForSwedCase(true).getDate());
}

@Test
public void testSwedishTimestamp() {
assertEquals(1363875404000L, genChangeSetForSwedCase(true).getTimestamp());
}
}
5 changes: 5 additions & 0 deletions src/test/java/hudson/plugins/git/GitChangeSetEmptyTest.java
View file
Expand Up @@ -22,6 +22,11 @@ public void testGetDate() {
assertNull(changeSet.getDate());
}

@Test
public void testGetTimestamp() {
assertEquals(-1L, changeSet.getTimestamp());
}

@Test
public void testGetCommitId() {
assertNull(changeSet.getCommitId());
Expand Down
10 changes: 10 additions & 0 deletions src/test/java/hudson/plugins/git/GitChangeSetEuroTest.java
View file
Expand Up @@ -124,6 +124,16 @@ public void testGetBranch() {
assertNull(changeSet.getBranch());
}

@Test
public void testGetDate() {
assertEquals(useAuthorName ? "2013-03-21T15:16:44+0100" : "2013-03-25T08:18:59-0400", changeSet.getDate());
}

@Test
public void testGetTimestamp() {
assertEquals(useAuthorName ? 1363875404000L : 1364213939000L, changeSet.getTimestamp());
}

@Test
public void testHashCode() {
assertTrue(changeSet.hashCode() != 0);
Expand Down
5 changes: 5 additions & 0 deletions src/test/java/hudson/plugins/git/GitChangeSetSimpleTest.java
View file
Expand Up @@ -119,6 +119,11 @@ public void testGetAuthorName() {
assertEquals(useAuthorName ? GitChangeSetUtil.AUTHOR_NAME : GitChangeSetUtil.COMMITTER_NAME, changeSet.getAuthorName());
}

@Test
public void testGetDate() {
assertEquals(useAuthorName ? GitChangeSetUtil.AUTHOR_DATE_FORMATTED : GitChangeSetUtil.COMMITTER_DATE_FORMATTED, changeSet.getDate());
}

@Test
public void testGetMsg() {
assertEquals(GitChangeSetUtil.COMMIT_TITLE, changeSet.getMsg());
Expand Down
50 changes: 50 additions & 0 deletions src/test/java/hudson/plugins/git/GitChangeSetTimestampTest.java
View file
@@ -0,0 +1,50 @@
package hudson.plugins.git;

import java.util.ArrayList;
import static org.junit.Assert.*;
import org.junit.Before;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;

/**
* JENKINS-30073 reports that the timestamp returns -1 for the typical timestamp
* reported by the +%ci format to git log and git whatchanged. This test
* duplicates the bug.
*
* @author Mark Waite
*/
public class GitChangeSetTimestampTest {

private GitChangeSet changeSet = null;

@Before
public void createChangeSet() {
changeSet = genChangeSetForJenkins30073(true);
}

@Test
public void testChangeSetDate() {
assertEquals("2015-10-06 19:29:47 +0300", changeSet.getDate());
}

@Test
@Issue("JENKINS-30073")
public void testChangeSetTimeStamp() {
assertEquals(1444148987000L, changeSet.getTimestamp());
}

private GitChangeSet genChangeSetForJenkins30073(boolean authorOrCommitter) {
ArrayList<String> lines = new ArrayList<String>();
lines.add("commit 302548f75c3eb6fa1db83634e4061d0ded416e5a");
lines.add("tree e1bd430d3f45b7aae54a3061b7895ee1858ec1f8");
lines.add("parent c74f084d8f9bc9e52f0b3fe9175ad27c39947a73");
lines.add("author Viacheslav Kopchenin <vkopchenin@odin.com> 2015-10-06 19:29:47 +0300");
lines.add("committer Viacheslav Kopchenin <vkopchenin@odin.com> 2015-10-06 19:29:47 +0300");
lines.add("");
lines.add(" pom.xml");
lines.add(" ");
lines.add(" :100644 100644 bb32d78c69a7bf79849217bc02b1ba2c870a5a66 343a844ad90466d8e829896c1827ca7511d0d1ef M modules/platform/pom.xml");
lines.add("");
return new GitChangeSet(lines, authorOrCommitter);
}
}
8 changes: 6 additions & 2 deletions src/test/java/hudson/plugins/git/GitChangeSetUtil.java
View file
Expand Up @@ -12,7 +12,11 @@ public class GitChangeSetUtil {
static final String ID = "123abc456def";
static final String PARENT = "345mno678pqr";
static final String AUTHOR_NAME = "John Author";
static final String AUTHOR_DATE = "1234568 -0600";
static final String AUTHOR_DATE_FORMATTED = "1970-01-15T06:56:08-0600";
static final String COMMITTER_NAME = "John Committer";
static final String COMMITTER_DATE = "1234566 -0600";
static final String COMMITTER_DATE_FORMATTED = "1970-01-15T06:56:06-0600";
static final String COMMIT_TITLE = "Commit title.";
static final String COMMENT = COMMIT_TITLE + "\n";

Expand All @@ -31,8 +35,8 @@ static GitChangeSet genChangeSet(boolean authorOrCommitter, boolean useLegacyFor
} else {
lines.add("parent ");
}
lines.add("author " + AUTHOR_NAME + " <jauthor@nospam.com> 1234568 -0600");
lines.add("committer " + COMMITTER_NAME + " <jcommitter@nospam.com> 1234566 -0600");
lines.add("author " + AUTHOR_NAME + " <jauthor@nospam.com> " + AUTHOR_DATE);
lines.add("committer " + COMMITTER_NAME + " <jcommitter@nospam.com> " + COMMITTER_DATE);
lines.add("");
lines.add(" " + COMMIT_TITLE);
lines.add(" Commit extended description.");
Expand Down

0 comments on commit b84c7c9

Please sign in to comment.