From 538365a95ee967b79b2f81cd79cef1c44057972c Mon Sep 17 00:00:00 2001 From: Tay Yi Hsuen <69678785+yhtMinceraft1010X@users.noreply.github.com> Date: Sun, 5 Jun 2022 23:29:26 +0800 Subject: [PATCH 1/7] [#1774] Fix unintended behavior related to --since d1 (#1776) The use of Instant.ofEpochMilli(Long.MIN_VALUE) as the arbitrary first commit date now fails shallow cloning for testSinceBeginningDateRangeWithShallowCloning system test. In addition, using this arbitrary date in convertToGitDateRangeArgs when converting timezone results in an overflow causing sinceDate to be greater than untilDate. Thus, no commit results are retrieved, leading to failing system tests. Also, the arbitrary first commit date is in UTC. However, in the convertToGitDateRangeArgs method, the zoneId used for the conversion is from the config, which may be different. This leads to an erroneous timezone conversion. Finally, using --since d1 with --period results in an unintended date range because the --period and the arbitrary commit date from --since d1 are used to calculate the until date as the actual earliest commit date is not immediately available. Let's change Instant.ofEpochMilli() to use 0 so that the arbitrary date is 1970-01-01 and the overflow of the sinceDate can be avoided. At the same time, let's convert the arbitrary date to the correct timezone within the ArgsParser class and add a component test for parsing --since d1 to check that the correct timezone is used for each RepoConfiguration before cloning any repos. Finally, let's raise a warning when --since d1 is being used with --period. --- docs/ug/cli.md | 4 ++- .../commits/CommitResultAggregator.java | 14 ++++---- .../java/reposense/parser/ArgsParser.java | 12 +++++++ .../parser/SinceDateArgumentType.java | 28 ++++++++++++--- .../reposense/report/ReportGenerator.java | 4 +-- src/main/java/reposense/util/TimeUtil.java | 36 +++++++++++++++---- .../java/reposense/parser/ArgsParserTest.java | 23 ++++++++++++ 7 files changed, 100 insertions(+), 21 deletions(-) diff --git a/docs/ug/cli.md b/docs/ug/cli.md index 512f4c4b69..96a0bf7a84 100644 --- a/docs/ug/cli.md +++ b/docs/ug/cli.md @@ -172,6 +172,7 @@ This flag overrides the `Ignore file size limit` field in the CSV config file. * If both start date and end date are not specified, the date of generating the report will be taken as the end date. +* May analyze the incorrect date range if used with `--since d1`. The program will throw a warning. * Cannot be used with both `--since` and `--until`. The program will throw an exception. @@ -216,7 +217,8 @@ Cannot be used with `--last-modified-date`. This may result in an incorrect last * If the start date is not specified, only commits made one month before the end date (if specified) or the date of generating the report, will be captured and analyzed. -* If `d1` is specified as the start date (`--since d1` or `-s d1`), then the earliest commit date of all repositories will be taken as the start date. +* If `d1` is specified as the start date (`--since d1` or `-s d1`), then the program will search for the earliest commit date of all repositories and use that as the start date. +* If `d1` is specified together with `--period`, then the program will warn that the date range being analyzed may be incorrect. diff --git a/src/main/java/reposense/commits/CommitResultAggregator.java b/src/main/java/reposense/commits/CommitResultAggregator.java index e722e7b644..66014125c6 100644 --- a/src/main/java/reposense/commits/CommitResultAggregator.java +++ b/src/main/java/reposense/commits/CommitResultAggregator.java @@ -14,16 +14,14 @@ import reposense.commits.model.CommitResult; import reposense.model.Author; import reposense.model.RepoConfiguration; -import reposense.parser.SinceDateArgumentType; import reposense.report.ReportGenerator; +import reposense.util.TimeUtil; /** * Uses the commit analysis results to generate the summary information of a repository. */ public class CommitResultAggregator { private static final int DAYS_IN_MS = 24 * 60 * 60 * 1000; - private static final ZonedDateTime ARBITRARY_FIRST_COMMIT_DATE_UTC = - ZonedDateTime.of(SinceDateArgumentType.ARBITRARY_FIRST_COMMIT_DATE, ZoneId.of("Z")); /** * Returns the {@link CommitContributionSummary} generated from aggregating the {@code commitResults}. @@ -33,7 +31,7 @@ public static CommitContributionSummary aggregateCommitResults( RepoConfiguration config, List commitResults) { LocalDateTime startDate; ZoneId zoneId = ZoneId.of(config.getZoneId()); - startDate = (config.getSinceDate().equals(SinceDateArgumentType.ARBITRARY_FIRST_COMMIT_DATE)) + startDate = (TimeUtil.isEqualToArbitraryFirstDateConverted(config.getSinceDate(), zoneId)) ? getStartOfDate(getStartDate(commitResults, zoneId), zoneId) : config.getSinceDate(); ReportGenerator.setEarliestSinceDate(startDate); @@ -157,11 +155,11 @@ private static void addDailyContributionForNewDate( /** * Gets the starting point of the {@code current} date. * - * @return the {@code current} date if it is equal to the {@code ARBITRARY_FIRST_COMMIT_DATE_UTC} adjusted to the + * @return the {@code current} date if it is equal to the {@code ARBITRARY_FIRST_COMMIT_DATE} adjusted to the * timezone given by {@code zoneId}. Otherwise, return a {@link LocalDateTime} adjusted to have a time of 00:00:00. */ private static LocalDateTime getStartOfDate(LocalDateTime current, ZoneId zoneId) { - if (current.equals(ARBITRARY_FIRST_COMMIT_DATE_UTC.withZoneSameInstant(zoneId).toLocalDateTime())) { + if (TimeUtil.isEqualToArbitraryFirstDateConverted(current, zoneId)) { return current; } @@ -172,11 +170,11 @@ private static LocalDateTime getStartOfDate(LocalDateTime current, ZoneId zoneId * Gets the earliest commit date from {@code commitInfos}. * * @return First commit date if there is at least one {@link CommitResult}. Otherwise, return - * the {@code ARBITRARY_FIRST_COMMIT_DATE_UTC} converted to the timezone given by {@code zoneId}. + * the {@code ARBITRARY_FIRST_COMMIT_DATE} converted to the timezone given by {@code zoneId}. */ private static LocalDateTime getStartDate(List commitInfos, ZoneId zoneId) { return (commitInfos.isEmpty()) - ? ARBITRARY_FIRST_COMMIT_DATE_UTC.withZoneSameInstant(zoneId).toLocalDateTime() + ? TimeUtil.getArbitraryFirstCommitDateConverted(zoneId) : commitInfos.get(0).getTime(); } } diff --git a/src/main/java/reposense/parser/ArgsParser.java b/src/main/java/reposense/parser/ArgsParser.java index 197ce6f2e5..eca175ac3e 100644 --- a/src/main/java/reposense/parser/ArgsParser.java +++ b/src/main/java/reposense/parser/ArgsParser.java @@ -74,6 +74,8 @@ public class ArgsParser { "Config path not provided, using the config folder as default."; private static final String MESSAGE_INVALID_CONFIG_PATH = "%s is malformed."; private static final String MESSAGE_INVALID_CONFIG_JSON = "%s Ignoring the report config provided."; + private static final String MESSAGE_SINCE_D1_WITH_PERIOD = "You may be using --since d1 with the --period flag. " + + "This may result in an incorrect date range being analysed."; private static final Path EMPTY_PATH = Paths.get(""); private static final Path DEFAULT_CONFIG_PATH = Paths.get(System.getProperty("user.dir") + File.separator + "config" + File.separator); @@ -280,6 +282,7 @@ public static CliArguments parse(String[] args) throws HelpScreenException, Pars boolean isSinceDateProvided = cliSinceDate.isPresent(); boolean isUntilDateProvided = cliUntilDate.isPresent(); boolean isPeriodProvided = cliPeriod.isPresent(); + boolean isUsingArbitraryDate = false; if (isSinceDateProvided && isUntilDateProvided && isPeriodProvided) { throw new ParseException(MESSAGE_HAVE_SINCE_DATE_UNTIL_DATE_AND_PERIOD); } @@ -290,6 +293,11 @@ public static CliArguments parse(String[] args) throws HelpScreenException, Pars if (isSinceDateProvided) { sinceDate = TimeUtil.getSinceDate(cliSinceDate.get()); + // For --since d1, need to adjust the arbitrary date based on timezone + if (TimeUtil.isEqualToArbitraryFirstDateUtc(sinceDate)) { + isUsingArbitraryDate = true; + sinceDate = TimeUtil.getArbitraryFirstCommitDateConverted(zoneId); + } } else { if (isUntilDateProvided) { sinceDate = isPeriodProvided @@ -303,6 +311,10 @@ public static CliArguments parse(String[] args) throws HelpScreenException, Pars } + if (isPeriodProvided && isUsingArbitraryDate) { + logger.warning(MESSAGE_SINCE_D1_WITH_PERIOD); + } + if (isUntilDateProvided) { untilDate = TimeUtil.getUntilDate(cliUntilDate.get()); } else { diff --git a/src/main/java/reposense/parser/SinceDateArgumentType.java b/src/main/java/reposense/parser/SinceDateArgumentType.java index 26d788d60e..bd5732a4bf 100644 --- a/src/main/java/reposense/parser/SinceDateArgumentType.java +++ b/src/main/java/reposense/parser/SinceDateArgumentType.java @@ -3,6 +3,7 @@ import java.time.Instant; import java.time.LocalDateTime; import java.time.ZoneId; +import java.time.ZonedDateTime; import java.util.Optional; import net.sourceforge.argparse4j.inf.Argument; @@ -16,14 +17,17 @@ public class SinceDateArgumentType extends DateArgumentType { /* * When user specifies "d1", arbitrary first commit date will be returned. + * This date is equivalent to 1970-01-01 00:00:00 in UTC time. * Then, ReportGenerator will replace the arbitrary since date with the earliest commit date. */ - public static final LocalDateTime ARBITRARY_FIRST_COMMIT_DATE = LocalDateTime.ofInstant( - Instant.ofEpochMilli(Long.MIN_VALUE), ZoneId.of("Z")); public static final String FIRST_COMMIT_DATE_SHORTHAND = "d1"; + private static final ZonedDateTime ARBITRARY_FIRST_COMMIT_DATE_UTC = ZonedDateTime.ofInstant( + Instant.ofEpochMilli(0), ZoneId.of("Z")); + private static final LocalDateTime ARBITRARY_FIRST_COMMIT_DATE_LOCAL = ARBITRARY_FIRST_COMMIT_DATE_UTC + .toLocalDateTime(); /** - * Returns an arbitrary year {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE} if user specifies + * Returns an arbitrary year {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE_LOCAL} if user specifies * {@link SinceDateArgumentType#FIRST_COMMIT_DATE_SHORTHAND} in {@code value}, or attempts to return the * desired date otherwise. * @@ -33,9 +37,25 @@ public class SinceDateArgumentType extends DateArgumentType { public Optional convert(ArgumentParser parser, Argument arg, String value) throws ArgumentParserException { if (FIRST_COMMIT_DATE_SHORTHAND.equals(value)) { - return Optional.of(ARBITRARY_FIRST_COMMIT_DATE); + return Optional.of(ARBITRARY_FIRST_COMMIT_DATE_LOCAL); } String sinceDate = TimeUtil.extractDate(value); return super.convert(parser, arg, sinceDate + " 00:00:00"); } + + /** + * Returns the {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE_LOCAL}, which is the + * {@link LocalDateTime} of {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE_UTC}. + */ + public static LocalDateTime getArbitraryFirstCommitDateLocal() { + return ARBITRARY_FIRST_COMMIT_DATE_LOCAL; + } + + /** + * Returns the {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE_UTC} adjusted for the time zone based on + * {@code toZoneId} and converted to a {@link LocalDateTime} object. + */ + public static LocalDateTime getArbitraryFirstCommitDateConverted(ZoneId toZoneId) { + return ARBITRARY_FIRST_COMMIT_DATE_UTC.withZoneSameInstant(toZoneId).toLocalDateTime(); + } } diff --git a/src/main/java/reposense/report/ReportGenerator.java b/src/main/java/reposense/report/ReportGenerator.java index 5744497fb3..bf56eb4fbd 100644 --- a/src/main/java/reposense/report/ReportGenerator.java +++ b/src/main/java/reposense/report/ReportGenerator.java @@ -46,12 +46,12 @@ import reposense.model.RepoLocation; import reposense.model.ReportConfiguration; import reposense.model.StandaloneConfig; -import reposense.parser.SinceDateArgumentType; import reposense.parser.StandaloneConfigJsonParser; import reposense.report.exception.NoAuthorsWithCommitsFoundException; import reposense.system.LogsManager; import reposense.util.FileUtil; import reposense.util.ProgressTracker; +import reposense.util.TimeUtil; /** * Contains report generation related functionalities. @@ -160,7 +160,7 @@ public static List generateReposReport(List configs, St List reportFoldersAndFiles = cloneAndAnalyzeRepos(configs, outputPath, numCloningThreads, numAnalysisThreads, shouldFreshClone); - LocalDateTime reportSinceDate = (cliSinceDate.equals(SinceDateArgumentType.ARBITRARY_FIRST_COMMIT_DATE)) + LocalDateTime reportSinceDate = (TimeUtil.isEqualToArbitraryFirstDateConverted(cliSinceDate, zoneId)) ? earliestSinceDate : cliSinceDate; Optional summaryPath = FileUtil.writeJsonFile( diff --git a/src/main/java/reposense/util/TimeUtil.java b/src/main/java/reposense/util/TimeUtil.java index 0bff26a5da..9e9e974e83 100644 --- a/src/main/java/reposense/util/TimeUtil.java +++ b/src/main/java/reposense/util/TimeUtil.java @@ -66,14 +66,8 @@ public static String getElapsedTimeMessage() { /** * Returns a {@link LocalDateTime} that is set to midnight for the given {@code sinceDate}. - * If {@code sinceDate} is {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE}, it is simply returned - * as such. */ public static LocalDateTime getSinceDate(LocalDateTime sinceDate) { - if (sinceDate.equals(SinceDateArgumentType.ARBITRARY_FIRST_COMMIT_DATE)) { - return sinceDate; - } - return sinceDate.withHour(0).withMinute(0).withSecond(0); } @@ -114,6 +108,36 @@ public static LocalDateTime getCurrentDate(ZoneId zoneId) { return LocalDateTime.now(zoneId).withHour(23).withMinute(59).withSecond(59).withNano(0); } + /** + * Returns the {@link LocalDateTime} of {@code ARBITRARY_FIRST_COMMIT_DATE} in the UTC time zone. + */ + public static LocalDateTime getArbitraryFirstCommitDateLocal() { + return SinceDateArgumentType.getArbitraryFirstCommitDateLocal(); + } + + /** + * Returns the {@link LocalDateTime} of {@code ARBITRARY_FIRST_COMMIT_DATE} adjusted for the time zone based on + * {@code toZoneId}. + */ + public static LocalDateTime getArbitraryFirstCommitDateConverted(ZoneId toZoneId) { + return SinceDateArgumentType.getArbitraryFirstCommitDateConverted(toZoneId); + } + + /** + * Checks whether the given {@code dateTime} is the {@code ARBITRARY_FIRST_COMMIT_DATE} in UTC time. + */ + public static boolean isEqualToArbitraryFirstDateUtc(LocalDateTime dateTime) { + return dateTime.equals(getArbitraryFirstCommitDateLocal()); + } + + /** + * Checks whether the given {@code dateTime} is the {@code ARBITRARY_FIRST_COMMIT_DATE} in the time zone given by + * {@code zoneId}. + */ + public static boolean isEqualToArbitraryFirstDateConverted(LocalDateTime dateTime, ZoneId zoneId) { + return dateTime.equals(getArbitraryFirstCommitDateConverted(zoneId)); + } + /** * Verifies that {@code sinceDate} is earlier than {@code untilDate}. * diff --git a/src/test/java/reposense/parser/ArgsParserTest.java b/src/test/java/reposense/parser/ArgsParserTest.java index fab23907db..1867524e18 100644 --- a/src/test/java/reposense/parser/ArgsParserTest.java +++ b/src/test/java/reposense/parser/ArgsParserTest.java @@ -31,6 +31,7 @@ import reposense.util.FileUtil; import reposense.util.InputBuilder; import reposense.util.TestUtil; +import reposense.util.TimeUtil; public class ArgsParserTest { @@ -71,6 +72,28 @@ public void after() { } } + @Test + public void parse_d1CorrectTimeZone_success() throws Exception { + String input = new InputBuilder().addConfig(CONFIG_FOLDER_ABSOLUTE) + .addSinceDate(SinceDateArgumentType.FIRST_COMMIT_DATE_SHORTHAND) + .addUntilDate("30/11/2017") + .addTimezone(DEFAULT_TIME_ZONE_STRING) + .build(); + CliArguments cliArguments = ArgsParser.parse(translateCommandline(input)); + Assertions.assertTrue(cliArguments instanceof ConfigCliArguments); + Assertions.assertTrue(Files.isSameFile( + REPO_CONFIG_CSV_FILE, ((ConfigCliArguments) cliArguments).getRepoConfigFilePath())); + Assertions.assertTrue(Files.isSameFile( + AUTHOR_CONFIG_CSV_FILE, ((ConfigCliArguments) cliArguments).getAuthorConfigFilePath())); + + LocalDateTime expectedSinceDate = TimeUtil.getArbitraryFirstCommitDateConverted(DEFAULT_TIME_ZONE_ID); + LocalDateTime expectedUntilDate = TestUtil.getUntilDate(2017, Month.NOVEMBER.getValue(), 30); + Assertions.assertEquals(expectedSinceDate, cliArguments.getSinceDate()); + Assertions.assertEquals(expectedUntilDate, cliArguments.getUntilDate()); + + Assertions.assertEquals(DEFAULT_TIME_ZONE_ID, cliArguments.getZoneId()); + } + @Test public void parse_allCorrectInputs_success() throws Exception { String input = new InputBuilder().addConfig(CONFIG_FOLDER_ABSOLUTE) From 2a28238bbf6a554cb82593f26cf29f680acc21c7 Mon Sep 17 00:00:00 2001 From: Ger Hean Date: Mon, 6 Jun 2022 23:37:47 +0800 Subject: [PATCH 2/7] hotfix --- frontend/src/app.vue | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frontend/src/app.vue b/frontend/src/app.vue index 956417dbd3..25dc507ab6 100644 --- a/frontend/src/app.vue +++ b/frontend/src/app.vue @@ -1,10 +1,8 @@