Skip to content
Permalink
Browse files

[Fix JENKINS-30073] changeset time shouldn't be -1

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 committed Oct 12, 2015
1 parent 4979ace commit b84c7c9dcea3a30a87eadb7d20c8be773cab46e3
@@ -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;

@@ -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
@@ -49,6 +56,10 @@
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.
*
@@ -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) {
@@ -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) {
@@ -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() {

@@ -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());
}
}
@@ -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());
@@ -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);
@@ -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());
@@ -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);
}
}
@@ -12,7 +12,11 @@
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";

@@ -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.");

0 comments on commit b84c7c9

Please sign in to comment.
You can’t perform that action at this time.