From ba2c8a1043dc5e2de1d6c2310a9f72ea9a2d401a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maximilian=20L=C3=B6ffler?= <s8maloef@stud.uni-saarland.de>
Date: Mon, 20 Nov 2023 18:26:05 +0100
Subject: [PATCH] Improve input validation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Check if the 'bins' parameter of the 'split.data.by.bins' actually
contains 'bins' component. Use 'get.date.from.string' instead of
accessing 'lubridate::ymd_hms' directly, to encapsulate date conversion.
Allow 'vector' component of 'bins' to be of any subclass of
'numeric' instead of explicitly 'numeric'.

Disallow lists that contain elements that are not representing a date
in 'split.data.time.based' as they do not comply with the expected
format of bins for 'split.data.by.time.or.bins'.

Signed-off-by: Maximilian Löffler <s8maloef@stud.uni-saarland.de>
---
 util-split.R | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/util-split.R b/util-split.R
index 35144e35..f192ab2b 100644
--- a/util-split.R
+++ b/util-split.R
@@ -65,11 +65,12 @@ split.data.time.based = function(project.data, time.period = "3 months", bins =
                                  number.windows = NULL, split.basis = c("commits", "mails", "issues"),
                                  sliding.window = FALSE, project.conf.new = NULL) {
 
-    # validate type of the 'bins' parameter
+    # validate existence and type of the 'bins' parameter
     if (!is.null(bins) && !lubridate::is.POSIXct(bins)) {
-        dates = parallel::mclapply(bins, function(bin) lubridate::ymd_hms(bin, truncated = 3))
+        dates = parallel::mclapply(unlist(bins), get.date.from.string)
         if (any(is.na(dates))) {
-            logging::logerror("The bins parameter, if present, needs to be a character representing a date")
+            logging::logerror(paste("The bins parameter, if present, needs to be vector",
+                                    "whose elements represent dates"))
             stop("Stopped due to incorrect parameter types")
         }
     }
@@ -104,21 +105,27 @@ split.data.by.bins = function(project.data, activity.amount, bins, split.basis =
         stop("Stopped due to incorrect parameter types")
     }
 
-    # validate type of the 'bins' component of the 'bins' parameter
-    dates = parallel::mclapply(bins[["bins"]], function(bin) lubridate::ymd_hms(bin, truncated = 3))
+    # validate existence and type of the 'bins' component of the 'bins' parameter
+    if (!("bins" %in% names(bins))) {
+        logging::logerror("The 'bins' parameter needs to include a component 'bins'")
+        stop("Stopped due to incorrect parameter types")
+    }
+
+    dates = parallel::mclapply(bins[["bins"]], get.date.from.string)
     if (any(is.na(dates))) {
-        logging::logerror("The 'bins' component of the bins parameter needs to be a character representing a date")
+        logging::logerror(paste("The 'bins' component of the 'bins' parameter, needs to be vector",
+                                "whose elements represent dates"))
         stop("Stopped due to incorrect parameter types")
     }
 
-    # validate type of the 'vector' component of the 'bins' parameter
-    if (class(bins[["vector"]]) != "numeric") {
+    # validate existence and type of the 'vector' component of the 'bins' parameter
+    if (!inherits(bins[["vector"]], "numeric")) {
         logging::logerror("The 'vector' component of the bins parameter needs to be a numeric vector")
         stop("Stopped due to incorrect parameter types")
     }
 
     split = split.data.by.time.or.bins(project.data, activity.amount, bins, split.by.time = FALSE,
-                                        sliding.window = sliding.window, split.basis = split.basis)
+                                       sliding.window = sliding.window, split.basis = split.basis)
     return(split)
 }