Merge pull request #69 from Boehringer-Ingelheim/342810-expand_review_revision_limit

[review] Expand revision count limit from 1000 to 10000 entries

Author: ml-ebs-ext

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: dv.listings
 Type: Package
 Title: Data listings module
-Version: 4.3.4-9003
+Version: 4.3.4-9004
 Authors@R: 
     c(
       person("Boehringer-Ingelheim Pharma GmbH & Co.KG", role = c("cph", "fnd")),

NEWS.md

@@ -1,3 +1,7 @@
+# dv.listings 4.3.4-9004
+- Review functionality:
+  - Expand revision count limit from 1000 to 10000 entries
+
 # dv.listings 4.3.4-9003
 - Review functionality:
   - Guard against problematic characters in dataset names

R/review.R

@@ -259,10 +259,21 @@ REV_load_annotation_info <- function(folder_contents, review, dataset_lists) {
 
       base_timestamp <- NA_real_
       data_timestamps_st <- rep(NA_real_, row_count)
-      # <domain>_000.base
-      file_path <- file.path(dataset_lists_name, paste0(dataset_review_name, "_000.base"))
-      if (file_path %in% names(folder_contents)) {
-        contents <- folder_contents[[file_path]]        
+      
+      # <domain>_0000.base
+      # - Older versions of the review functionality devoted three digits to the `.base` and `.delta` sequence numbers.
+      #   The current version uses four digits. Here we detect the one that was used for this particular domain (if it
+      #   exists) and use it for the associated delta files.
+      base_file_path_pattern <- sprintf("^%s_0+.base$", file.path(dataset_lists_name, dataset_review_name))
+      base_file_path <- grep(base_file_path_pattern, names(folder_contents), value = TRUE)
+      if (length(base_file_path) > 1L) {
+        error <- c(error, paste0("[", dataset_review_name, "] ", "Multiple `.base` files found:\n",
+                                 paste(sprintf("`%s`", base_file_path), collapse = ", "), ".\n"))
+        base_file_path <- sort(base_file_path)[[1]]
+      }
+      
+      if (length(base_file_path) == 1) { # existing `.base` file
+        contents <- folder_contents[[base_file_path]]        
 
         sorted_delta_file_paths <- local({
           pattern <- sprintf("^%s_[0-9]*.delta", file.path(dataset_lists_name, dataset_review_name))
@@ -383,18 +394,22 @@ REV_load_annotation_info <- function(folder_contents, review, dataset_lists) {
               base_info <- RS_load(contents, deltas)
 
               delta_number <- length(sorted_delta_file_paths) + 1
-              file_path <- file.path(dataset_lists_name, sprintf("%s_%03d.delta", dataset_review_name, delta_number))
+              revision_digit_count <- nchar(sub(".*_(0+)\\.base$", "\\1", base_file_path))
+              file_path <- file.path(
+                dataset_lists_name, sprintf("%s_%.*d.delta", dataset_review_name, revision_digit_count, delta_number)
+              )
               append_IO_action(list(kind = "write", path = file_path, contents = new_delta_contents, offset = 0L))
             }
         }
-      } else {
+      } else { # new `.base` file
+        base_file_path <- file.path(dataset_lists_name, paste0(dataset_review_name, "_0000.base"))
         contents <- RS_compute_base_memory(dataset_review_name, dataset, id_vars, tracked_vars)
         if (inherits(contents, "simpleCondition")) {
           # IMPORTANT: Not being able to compute the base info is too severe an error to recover from, so we error out
           return(list(error = c(error, contents[["message"]])))
         } else {
           base_info <- RS_load(base = contents, deltas = list())
-          append_IO_action(list(kind = "write", path = file_path, contents = contents, offset = 0L))
+          append_IO_action(list(kind = "write", path = base_file_path, contents = contents, offset = 0L))
         }
       }
 

tests/testthat/test-review.R

@@ -44,7 +44,7 @@ local({
     expect_length(info[["IO_plan"]], 1)
     folder_op <- info[["IO_plan"]][[1]]
     expect_equal(folder_op[["kind"]], "write")
-    expect_equal(folder_op[["path"]], "dataset_list/ae_001.delta")
+    expect_equal(folder_op[["path"]], "dataset_list/ae_0001.delta")
     expect_equal(folder_op[["offset"]], 0L)
 
     delta <- RS_parse_delta(info[["IO_plan"]][[1]][["contents"]], length(tracked_vars))

vignettes/data_review.Rmd

@@ -157,7 +157,7 @@ Lastly, using plain file storage allows for easy local testing and for universal
 
 If we take a hypothetical "xyz" domain, `dv.listings` will store the following files for review purposes (expand to see the layout of their contents):
 
-<details><summary>`xyz_000.base` (information related to the first version of the "xyz" domain dataset seen by the module)</summary>
+<details><summary>`xyz_0000.base` (information related to the first version of the "xyz" domain dataset seen by the module)</summary>
 - 1 file magic code ("LISTBASE")
 - 1 format version number (0)
 - 1 generation marker (0)
@@ -173,7 +173,7 @@ If we take a hypothetical "xyz" domain, `dv.listings` will store the following f
 - p (1 per "xyz" row, 2\*m bytes long) `hash_tracked(xyz[tracked_vars])`
 </details>
 
-<details><summary>`xyz_001.delta` (one per domain dataset update)</summary>
+<details><summary>`xyz_0001.delta` (one per domain dataset update)</summary>
 - 1 file magic code ("LISTDELT")
 - 1 format version number (0)
 - 1 generation marker (1). Wraps around to 0 after 255 (e.g. for `xyz_256.delta`)