From 7f7dd171df7c02d2e9b95c640909cc11acda2644 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 5 Nov 2013 16:57:06 +0100 Subject: [PATCH] Cache SimpleDateFormat in GitDateParser per locale Otherwise switching to another locale yields wrong results when parsing date strings in GitDateParser. Since the MockSystemReader explicitly uses english locale the tests need to specify the locale to be used when parsing date strings. Bug: 420772 Change-Id: I313ef6b1e9ef3bfb43d929ce34712ebd21f2cd9c Signed-off-by: Matthias Sohn --- .../jgit/api/GarbageCollectCommandTest.java | 11 ++- .../util/GitDateParserBadlyFormattedTest.java | 6 +- .../eclipse/jgit/util/GitDateParserTest.java | 39 +++++--- .../jgit/internal/storage/file/GC.java | 4 +- .../org/eclipse/jgit/util/GitDateParser.java | 92 ++++++++++++++++--- .../org/eclipse/jgit/util/SystemReader.java | 15 +++ 6 files changed, 133 insertions(+), 34 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GarbageCollectCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GarbageCollectCommandTest.java index 47ebb6b2a..098f2b3b5 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GarbageCollectCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GarbageCollectCommandTest.java @@ -49,6 +49,7 @@ import java.util.Properties; import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.util.GitDateParser; +import org.eclipse.jgit.util.SystemReader; import org.junit.Before; import org.junit.Test; @@ -68,7 +69,8 @@ public class GarbageCollectCommandTest extends RepositoryTestCase { @Test public void testGConeCommit() throws Exception { - Date expire = GitDateParser.parse("now", null); + Date expire = GitDateParser.parse("now", null, SystemReader + .getInstance().getLocale()); Properties res = git.gc().setExpire(expire).call(); assertTrue(res.size() == 7); } @@ -83,8 +85,11 @@ public class GarbageCollectCommandTest extends RepositoryTestCase { writeTrashFile("b.txt", "a couple of words for gc to pack more 2"); writeTrashFile("c.txt", "a couple of words for gc to pack more 3"); git.commit().setAll(true).setMessage("commit3").call(); - Properties res = git.gc().setExpire(GitDateParser.parse("now", null)) - .call(); + Properties res = git + .gc() + .setExpire( + GitDateParser.parse("now", null, SystemReader + .getInstance().getLocale())).call(); assertTrue(res.size() == 7); } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/GitDateParserBadlyFormattedTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/GitDateParserBadlyFormattedTest.java index e0e99a14c..a6af3a514 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/GitDateParserBadlyFormattedTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/GitDateParserBadlyFormattedTest.java @@ -92,7 +92,8 @@ public class GitDateParserBadlyFormattedTest { Calendar ref = new GregorianCalendar(SystemReader.getInstance() .getTimeZone(), SystemReader.getInstance().getLocale()); try { - GitDateParser.parse(dateStr, ref); + GitDateParser.parse(dateStr, ref, SystemReader.getInstance() + .getLocale()); fail("The expected ParseException while parsing '" + dateStr + "' did not occur."); } catch (ParseException e) { @@ -103,7 +104,8 @@ public class GitDateParserBadlyFormattedTest { @Theory public void badlyFormattedWithoutRef() { try { - GitDateParser.parse(dateStr, null); + GitDateParser.parse(dateStr, null, SystemReader.getInstance() + .getLocale()); fail("The expected ParseException while parsing '" + dateStr + "' did not occur."); } catch (ParseException e) { diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/GitDateParserTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/GitDateParserTest.java index 570f4999d..518ed53fd 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/GitDateParserTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/GitDateParserTest.java @@ -72,7 +72,8 @@ public class GitDateParserTest { GregorianCalendar cal = new GregorianCalendar(SystemReader .getInstance().getTimeZone(), SystemReader.getInstance() .getLocale()); - Date parse = GitDateParser.parse("yesterday", cal); + Date parse = GitDateParser.parse("yesterday", cal, SystemReader + .getInstance().getLocale()); cal.add(Calendar.DATE, -1); cal.set(Calendar.HOUR_OF_DAY, 0); cal.set(Calendar.MINUTE, 0); @@ -87,7 +88,8 @@ public class GitDateParserTest { GregorianCalendar cal = new GregorianCalendar(SystemReader .getInstance().getTimeZone(), SystemReader.getInstance() .getLocale()); - Date parse = GitDateParser.parse("never", cal); + Date parse = GitDateParser.parse("never", cal, SystemReader + .getInstance().getLocale()); Assert.assertEquals(GitDateParser.NEVER, parse); parse = GitDateParser.parse("never", null); Assert.assertEquals(GitDateParser.NEVER, parse); @@ -104,7 +106,8 @@ public class GitDateParserTest { .getLocale()); cal.setTime(refDate); - Date parse = GitDateParser.parse("now", cal); + Date parse = GitDateParser.parse("now", cal, SystemReader.getInstance() + .getLocale()); Assert.assertEquals(refDate, parse); long t1 = SystemReader.getInstance().getCurrentTime(); parse = GitDateParser.parse("now", null); @@ -123,7 +126,8 @@ public class GitDateParserTest { .getLocale()); cal.setTime(refDate); - Date parse = GitDateParser.parse("2 weeks ago", cal); + Date parse = GitDateParser.parse("2 weeks ago", cal, SystemReader + .getInstance().getLocale()); Assert.assertEquals(df.parse("2007-02-07 15:35:00 +0100"), parse); } @@ -138,7 +142,8 @@ public class GitDateParserTest { .getLocale()); cal.setTime(refDate); - Date parse = GitDateParser.parse("2 weeks ago", cal); + Date parse = GitDateParser.parse("2 weeks ago", cal, SystemReader.getInstance() + .getLocale()); Assert.assertEquals(df.parse("2007-02-07 15:35:00 +0100"), parse); parse = GitDateParser.parse("3 days 2 weeks ago", cal); Assert.assertEquals(df.parse("2007-02-04 15:35:00 +0100"), parse); @@ -151,7 +156,8 @@ public class GitDateParserTest { String dateStr = "2007-02-21 15:35:00 +0100"; Date exp = SystemReader.getInstance() .getSimpleDateFormat("yyyy-MM-dd HH:mm:ss Z").parse(dateStr); - Date parse = GitDateParser.parse(dateStr, null); + Date parse = GitDateParser.parse(dateStr, null, SystemReader + .getInstance().getLocale()); Assert.assertEquals(exp, parse); } @@ -161,7 +167,8 @@ public class GitDateParserTest { Date exp = SystemReader.getInstance() .getSimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss Z") .parse(dateStr); - Date parse = GitDateParser.parse(dateStr, null); + Date parse = GitDateParser.parse(dateStr, null, SystemReader + .getInstance().getLocale()); Assert.assertEquals(exp, parse); } @@ -170,7 +177,8 @@ public class GitDateParserTest { String dateStr = "2007-02-21"; Date exp = SystemReader.getInstance().getSimpleDateFormat("yyyy-MM-dd") .parse(dateStr); - Date parse = GitDateParser.parse(dateStr, null); + Date parse = GitDateParser.parse(dateStr, null, SystemReader + .getInstance().getLocale()); Assert.assertEquals(exp, parse); } @@ -179,7 +187,8 @@ public class GitDateParserTest { String dateStr = "2007.02.21"; Date exp = SystemReader.getInstance().getSimpleDateFormat("yyyy.MM.dd") .parse(dateStr); - Date parse = GitDateParser.parse(dateStr, null); + Date parse = GitDateParser.parse(dateStr, null, SystemReader + .getInstance().getLocale()); Assert.assertEquals(exp, parse); } @@ -188,7 +197,8 @@ public class GitDateParserTest { String dateStr = "02/21/2007"; Date exp = SystemReader.getInstance().getSimpleDateFormat("MM/dd/yyyy") .parse(dateStr); - Date parse = GitDateParser.parse(dateStr, null); + Date parse = GitDateParser.parse(dateStr, null, SystemReader + .getInstance().getLocale()); Assert.assertEquals(exp, parse); } @@ -197,7 +207,8 @@ public class GitDateParserTest { String dateStr = "21.02.2007"; Date exp = SystemReader.getInstance().getSimpleDateFormat("dd.MM.yyyy") .parse(dateStr); - Date parse = GitDateParser.parse(dateStr, null); + Date parse = GitDateParser.parse(dateStr, null, SystemReader + .getInstance().getLocale()); Assert.assertEquals(exp, parse); } @@ -207,7 +218,8 @@ public class GitDateParserTest { Date exp = SystemReader.getInstance() .getSimpleDateFormat("EEE MMM dd HH:mm:ss yyyy Z") .parse(dateStr); - Date parse = GitDateParser.parse(dateStr, null); + Date parse = GitDateParser.parse(dateStr, null, SystemReader + .getInstance().getLocale()); Assert.assertEquals(exp, parse); } @@ -216,7 +228,8 @@ public class GitDateParserTest { String dateStr = "Wed Feb 21 15:35:00 2007"; Date exp = SystemReader.getInstance() .getSimpleDateFormat("EEE MMM dd HH:mm:ss yyyy").parse(dateStr); - Date parse = GitDateParser.parse(dateStr, null); + Date parse = GitDateParser.parse(dateStr, null, SystemReader + .getInstance().getLocale()); Assert.assertEquals(exp, parse); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index 9eaeaa8c1..e06ff65ee 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -97,6 +97,7 @@ import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.treewalk.filter.TreeFilter; import org.eclipse.jgit.util.FileUtils; import org.eclipse.jgit.util.GitDateParser; +import org.eclipse.jgit.util.SystemReader; /** * A garbage collector for git {@link FileRepository}. Instances of this class @@ -307,7 +308,8 @@ public class GC { ConfigConstants.CONFIG_KEY_PRUNEEXPIRE); if (pruneExpireStr == null) pruneExpireStr = PRUNE_EXPIRE_DEFAULT; - expire = GitDateParser.parse(pruneExpireStr, null); + expire = GitDateParser.parse(pruneExpireStr, null, SystemReader + .getInstance().getLocale()); expireAgeMillis = -1; } if (expire != null) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/GitDateParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/GitDateParser.java index 8f4e491de..32859c9c5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/GitDateParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/GitDateParser.java @@ -49,6 +49,7 @@ import java.util.Calendar; import java.util.Date; import java.util.GregorianCalendar; import java.util.HashMap; +import java.util.Locale; import java.util.Map; import org.eclipse.jgit.internal.JGitText; @@ -71,23 +72,40 @@ public class GitDateParser { // Since SimpleDateFormat instances are expensive to instantiate they should // be cached. Since they are also not threadsafe they are cached using // ThreadLocal. - private static ThreadLocal> formatCache = new ThreadLocal>() { - protected Map initialValue() { - return new HashMap(); + private static ThreadLocal>> formatCache = + new ThreadLocal>>() { + + protected Map> initialValue() { + return new HashMap>(); } }; - // Gets an instance of a SimpleDateFormat. If there is not already an - // appropriate instance in the (ThreadLocal) cache the create one and put in - // into the cache - private static SimpleDateFormat getDateFormat(ParseableSimpleDateFormat f) { - Map map = formatCache + // Gets an instance of a SimpleDateFormat for the specified locale. If there + // is not already an appropriate instance in the (ThreadLocal) cache then + // create one and put it into the cache. + private static SimpleDateFormat getDateFormat(ParseableSimpleDateFormat f, + Locale locale) { + Map> cache = formatCache .get(); + Map map = cache + .get(locale); + if (map == null) { + map = new HashMap(); + cache.put(locale, map); + return getNewSimpleDateFormat(f, locale, map); + } SimpleDateFormat dateFormat = map.get(f); if (dateFormat != null) return dateFormat; + SimpleDateFormat df = getNewSimpleDateFormat(f, locale, map); + return df; + } + + private static SimpleDateFormat getNewSimpleDateFormat( + ParseableSimpleDateFormat f, Locale locale, + Map map) { SimpleDateFormat df = SystemReader.getInstance().getSimpleDateFormat( - f.formatStr); + f.formatStr, locale); map.put(f, df); return df; } @@ -115,9 +133,9 @@ public class GitDateParser { } /** - * Parses a string into a {@link Date}. Since this parser also supports - * relative formats (e.g. "yesterday") the caller can specify the reference - * date. These types of strings can be parsed: + * Parses a string into a {@link Date} using the default locale. Since this + * parser also supports relative formats (e.g. "yesterday") the caller can + * specify the reference date. These types of strings can be parsed: *
    *
  • "never"
    • *
  • "now"
    • @@ -151,6 +169,49 @@ public class GitDateParser { */ public static Date parse(String dateStr, Calendar now) throws ParseException { + return parse(dateStr, now, Locale.getDefault()); + } + + /** + * Parses a string into a {@link Date} using the given locale. Since this + * parser also supports relative formats (e.g. "yesterday") the caller can + * specify the reference date. These types of strings can be parsed: + *
      + *
    • "never"
      • + *
    • "now"
      • + *
    • "yesterday"
      • + *
    • "(x) years|months|weeks|days|hours|minutes|seconds ago"
      + * Multiple specs can be combined like in "2 weeks 3 days ago". Instead of + * ' ' one can use '.' to seperate the words
      • + *
    • "yyyy-MM-dd HH:mm:ss Z" (ISO)
      • + *
    • "EEE, dd MMM yyyy HH:mm:ss Z" (RFC)
      • + *
    • "yyyy-MM-dd"
      • + *
    • "yyyy.MM.dd"
      • + *
    • "MM/dd/yyyy",
      • + *
    • "dd.MM.yyyy"
      • + *
    • "EEE MMM dd HH:mm:ss yyyy Z" (DEFAULT)
      • + *
    • "EEE MMM dd HH:mm:ss yyyy" (LOCAL)
      • + *
    + * + * @param dateStr + * the string to be parsed + * @param now + * the base date which is used for the calculation of relative + * formats. E.g. if baseDate is "25.8.2012" then parsing of the + * string "1 week ago" would result in a date corresponding to + * "18.8.2012". This is used when a JGit command calls this + * parser often but wants a consistent starting point for calls.
    + * If set to null then the current time will be used + * instead. + * @param locale + * locale to be used to parse the date string + * @return the parsed {@link Date} + * @throws ParseException + * if the given dateStr was not recognized + * @since 3.2 + */ + public static Date parse(String dateStr, Calendar now, Locale locale) + throws ParseException { dateStr = dateStr.trim(); Date ret; @@ -161,7 +222,7 @@ public class GitDateParser { return ret; for (ParseableSimpleDateFormat f : ParseableSimpleDateFormat.values()) { try { - return parse_simple(dateStr, f); + return parse_simple(dateStr, f, locale); } catch (ParseException e) { // simply proceed with the next parser } @@ -177,9 +238,10 @@ public class GitDateParser { } // tries to parse a string with the formats supported by SimpleDateFormat - private static Date parse_simple(String dateStr, ParseableSimpleDateFormat f) + private static Date parse_simple(String dateStr, + ParseableSimpleDateFormat f, Locale locale) throws ParseException { - SimpleDateFormat dateFormat = getDateFormat(f); + SimpleDateFormat dateFormat = getDateFormat(f, locale); dateFormat.setLenient(false); return dateFormat.parse(dateStr); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java index b6028610b..e73f100f9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java @@ -229,6 +229,21 @@ public abstract class SystemReader { return new SimpleDateFormat(pattern); } + /** + * Returns a simple date format instance as specified by the given pattern. + * + * @param pattern + * the pattern as defined in + * {@link SimpleDateFormat#SimpleDateFormat(String)} + * @param locale + * locale to be used for the {@code SimpleDateFormat} + * @return the simple date format + * @since 3.2 + */ + public SimpleDateFormat getSimpleDateFormat(String pattern, Locale locale) { + return new SimpleDateFormat(pattern, locale); + } + /** * Returns a date/time format instance for the given styles. *