[#1774] Fix unintended behavior related to --since d1

docs/ug/cli.md

@@ -172,6 +172,7 @@ This flag overrides the `Ignore file size limit` field in the CSV config file.
 <box type="info" seamless>
 
 * If both start date and end date are not specified, the date of generating the report will be taken as the end date.
+* May analyse 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.
 </box>
 <!-- ------------------------------------------------------------------------------------------------------ -->
@@ -216,7 +217,8 @@ Cannot be used with `--last-modified-date`. This may result in an incorrect last
 <box type="info" seamless>
 
 * 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 analysed may be incorrect.
 </box>
 <!-- ------------------------------------------------------------------------------------------------------ -->
 

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<CommitResult> 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<CommitResult> commitInfos, ZoneId zoneId) {
         return (commitInfos.isEmpty())
-                ? ARBITRARY_FIRST_COMMIT_DATE_UTC.withZoneSameInstant(zoneId).toLocalDateTime()
+                ? TimeUtil.getArbitraryFirstCommitDateConverted(zoneId)
                 : commitInfos.get(0).getTime();
     }
 }

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);
@@ -290,6 +292,13 @@ 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)) {
+                    if (isPeriodProvided) {
+                        logger.warning(MESSAGE_SINCE_D1_WITH_PERIOD);
+                    }
+                    sinceDate = TimeUtil.getArbitraryFirstCommitDateConverted(zoneId);
+                }
             } else {
                 if (isUntilDateProvided) {
                     sinceDate = isPeriodProvided

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 = ZonedDateTime.ofInstant(
+            Instant.ofEpochMilli(0), ZoneId.of("Z"));
+    private static final LocalDateTime ARBITRARY_FIRST_COMMIT_DATE_UTC_LOCAL = ARBITRARY_FIRST_COMMIT_DATE
+            .toLocalDateTime();
 
     /**
-     * Returns an arbitrary year {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE} if user specifies
+     * Returns an arbitrary year {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE_UTC_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<LocalDateTime> 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_UTC_LOCAL);
         }
         String sinceDate = TimeUtil.extractDate(value);
         return super.convert(parser, arg, sinceDate + " 00:00:00");
     }
+
+    /**
+     * Returns the {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE_UTC_LOCAL}, which is the
+     * {@link LocalDateTime} of {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE}.
+     */
+    public static LocalDateTime getArbitraryFirstCommitDateUtcLocal() {
+        return ARBITRARY_FIRST_COMMIT_DATE_UTC_LOCAL;
+    }
+
+    /**
+     * Returns the {@link SinceDateArgumentType#ARBITRARY_FIRST_COMMIT_DATE} 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.withZoneSameInstant(toZoneId).toLocalDateTime();
+    }
 }

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<Path> generateReposReport(List<RepoConfiguration> configs, St
         List<Path> 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<Path> summaryPath = FileUtil.writeJsonFile(

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 getArbitraryFirstCommitDateUtcLocal() {
+        return SinceDateArgumentType.getArbitraryFirstCommitDateUtcLocal();
+    }
+
+    /**
+     * 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(getArbitraryFirstCommitDateUtcLocal());
+    }
+
+    /**
+     * 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}.
      *

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)