From f50b26ffd0989c3ae952ad6a30fd1139d4544931 Mon Sep 17 00:00:00 2001
From: MEO265 <99362508+MEO265@users.noreply.github.com>
Date: Fri, 3 May 2024 15:00:00 +0200
Subject: [PATCH] Version 2.2.2 fixes (#62)

* mnt: Inc version

* fix: Don't add empty fields or un-sanitize in `rotate_logs`

* mnt: Reset options on exit

* mnt: Shorten title

* doc: Add return values
---
 DESCRIPTION                             |  4 ++--
 NEWS.md                                 |  2 +-
 R/configurations.R                      | 12 +++++++++-
 R/handlers.R                            |  6 ++++-
 R/json.R                                |  9 +++++--
 R/loggit.R                              |  2 ++
 R/utils.R                               |  4 +++-
 README.Rmd                              | 10 +++++---
 README.md                               | 32 ++++++++++++-------------
 man/get_logfile.Rd                      |  3 +++
 man/get_timestamp_format.Rd             |  3 +++
 man/loggit.Rd                           |  3 +++
 man/message.Rd                          |  3 +++
 man/rotate_logs.Rd                      |  3 +++
 man/set_logfile.Rd                      |  3 +++
 man/set_timestamp_format.Rd             |  3 +++
 man/stop.Rd                             |  3 +++
 tests/testthat/_snaps/utils/test.loggit |  3 +++
 tests/testthat/test-json.R              |  2 +-
 tests/testthat/test-utils.R             | 22 +++++++++++++----
 tests/testthat/testdata/test.loggit     |  6 +++++
 vignettes/getting-started.Rmd           |  5 +++-
 22 files changed, 110 insertions(+), 33 deletions(-)
 create mode 100644 tests/testthat/_snaps/utils/test.loggit
 create mode 100644 tests/testthat/testdata/test.loggit

diff --git a/DESCRIPTION b/DESCRIPTION
index b69fd62..3cd0fdf 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -1,5 +1,5 @@
 Package: loggit2
-Title: Easy-to-Use, Dependencyless Logger for R
+Title: Easy-to-Use, Dependencyless Logger
 Description:
     An easy-to-use 'ndjson' (newline-delimited 'JSON') logger.
     It provides a set of wrappings for base R's message(), warning(), and
@@ -7,7 +7,7 @@ Description:
     the handler message to an 'ndjson' log file.
     No change in existing code is necessary to use this package, and should
     only require additions to fully leverage the power of the logging system.
-Version: 2.2.1
+Version: 2.2.2
 Authors@R: c(
     person("Matthias", "Ollech", role = c("cre", "aut"), email = "ollech@gmx.com"),
     person("Ryan", "Price", role = c("fnd", "aut"))
diff --git a/NEWS.md b/NEWS.md
index de171d9..df91194 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,4 +1,4 @@
-# loggit2 2.2.1
+# loggit2 2.2.2
 
 ## Breaking changes
 * Custom sanitizers and unsanitizers must be able to process character vectors. 
diff --git a/R/configurations.R b/R/configurations.R
index 8cfc566..e27d348 100644
--- a/R/configurations.R
+++ b/R/configurations.R
@@ -15,6 +15,8 @@
 #' @details No logs outside of a temporary directory will be written until this is set explicitly, as per CRAN policy.
 #' Therefore, the default behavior is to create a file named `loggit.log` in your system's temporary directory.
 #'
+#' @return Invisible `NULL`.
+#'
 #' @examples set_logfile(file.path(tempdir(), "loggit.log"))
 #'
 #' @export
@@ -22,9 +24,10 @@ set_logfile <- function(logfile = NULL, confirm = TRUE, create = TRUE) {
   if (is.null(logfile)) {
     logfile <- file.path(tempdir(), "loggit.log")
   }
-  if(create && !file.exists(logfile)) file.create(logfile)
+  if (create && !file.exists(logfile)) file.create(logfile)
   .config$logfile <- normalizePath(logfile, winslash = "/", mustWork = FALSE)
   if (confirm) base::message("Log file set to ", .config$logfile)
+  invisible(NULL)
 }
 
 
@@ -32,6 +35,8 @@ set_logfile <- function(logfile = NULL, confirm = TRUE, create = TRUE) {
 #'
 #' Return the log file that `loggit()` will write to.
 #'
+#' @return The log file path.
+#'
 #' @examples get_logfile()
 #'
 #' @export
@@ -56,6 +61,8 @@ get_logfile <- function() {
 #' @param confirm Print confirmation message of timestamp format? Defaults to
 #'   `TRUE`.
 #'
+#' @return Invisible `NULL`.
+#'
 #' @examples set_timestamp_format("%Y-%m-%d %H:%M:%S")
 #'
 #' @export
@@ -67,6 +74,7 @@ set_timestamp_format <- function(ts_format = "%Y-%m-%dT%H:%M:%S%z", confirm = TR
       "Current time in this format: ", format(Sys.time(), format = ts_format)
     )
   }
+  invisible(NULL)
 }
 
 
@@ -74,6 +82,8 @@ set_timestamp_format <- function(ts_format = "%Y-%m-%dT%H:%M:%S%z", confirm = TR
 #'
 #' Get timestamp format for use in output logs.
 #'
+#' @return The timestamp format.
+#'
 #' @examples get_timestamp_format()
 #'
 #' @export
diff --git a/R/handlers.R b/R/handlers.R
index 1b950a8..0242cf4 100644
--- a/R/handlers.R
+++ b/R/handlers.R
@@ -3,11 +3,13 @@
 #' This function is identical to base R's [`message`][base::message],
 #' but it includes logging of the exception message via `loggit()`.
 #'
-#' @inherit base::message params return
+#' @inherit base::message params
 #'
 #' @param .loggit Should `loggit()` execute? Defaults to `TRUE`.
 #' @param echo Should `loggit()`'s log entry be echoed to the console, as well? Defaults to `TRUE`.
 #'
+#' @return Invisible `NULL`.
+#'
 #' @family handlers
 #'
 #' @examples
@@ -92,6 +94,8 @@ warning <- function(..., call. = TRUE, immediate. = FALSE, noBreaks. = FALSE,
 #' @inherit base::stop params
 #' @inheritParams message
 #'
+#' @return No return value.
+#'
 #' @family handlers
 #'
 #' @examples
diff --git a/R/json.R b/R/json.R
index 7129c06..5dcb936 100644
--- a/R/json.R
+++ b/R/json.R
@@ -45,6 +45,9 @@ default_ndjson_sanitizer <- function(string) {
     string <- gsub(pattern = k, replacement =  sanitizer_map[[k]], string, fixed = TRUE)
   }
 
+  # Explicit NAs must be marked so that no new ones are inserted when rotating the log
+  string[is.na(string)] <- "__NA__"
+
   string
 }
 
@@ -54,6 +57,8 @@ default_ndjson_unsanitizer <- function(string) {
     string <- gsub(pattern = sanitizer_map[[k]], replacement = k, string, fixed = TRUE)
   }
 
+  string[string == "__NA__"] <- NA_character_
+
   string
 }
 
@@ -136,9 +141,9 @@ read_ndjson <- function(logfile, unsanitizer = default_ndjson_unsanitizer) {
       # is the corresponding key.
       if (logfieldnum %% 2L == 0L) {
         colname <- rowdata[logfieldnum - 1L]
-        # If the field doesn't exist, create it with the right length
+        # If the field doesn't exist, create it (filled with NAs) with the right length
         if (!(colname %in% names(log_df))) {
-          log_df[[colname]] <- character(length = rowcount)
+          log_df[[colname]] <- rep(NA_character_, length = rowcount)
         }
         # Unsanitize text, and store to df
         rowdata[logfieldnum] <- unsanitizer(rowdata[logfieldnum])
diff --git a/R/loggit.R b/R/loggit.R
index 05a3208..60026a7 100644
--- a/R/loggit.R
+++ b/R/loggit.R
@@ -13,6 +13,8 @@
 #' @param sanitizer [Sanitizer function][sanitizers] to run over elements in log data.
 #'   Defaults to [default_ndjson_sanitizer()].
 #'
+#' @return Invisible `NULL`.
+#'
 #' @examples
 #'   loggit("INFO", "This is a message", but_maybe = "you want more fields?",
 #'   sure = "why not?", like = 2, or = 10, what = "ever")
diff --git a/R/utils.R b/R/utils.R
index 067d345..013e728 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -29,6 +29,8 @@ read_logs <- function(logfile = get_logfile(), unsanitizer = default_ndjson_unsa
 #' @param rotate_lines The number of log entries to keep in the logfile.
 #' @param logfile Log file to truncate.
 #'
+#' @return Invisible `NULL`.
+#'
 #' @examples
 #'   # Truncate "default" log file to 100 lines
 #'   set_logfile()
@@ -49,7 +51,7 @@ rotate_logs <- function(rotate_lines = 100000L, logfile = get_logfile()) {
     cat(NULL, file = logfile)
     return(invisible(NULL))
   }
-  log_df <- read_logs(logfile)
+  log_df <- read_logs(logfile, unsanitizer = identity)
   if (nrow(log_df) <= rotate_lines) {
     return(invisible(NULL))
   }
diff --git a/README.Rmd b/README.Rmd
index 004170e..a7fff04 100644
--- a/README.Rmd
+++ b/README.Rmd
@@ -14,7 +14,7 @@ knitr::opts_chunk$set(
   collapse = TRUE,
   comment = "#>"
 )
-options(width = 150)
+old <- options(width = 150)
 ```
 
 Easy-to-use, dependencyless Logger for R
@@ -105,6 +105,10 @@ or, get the latest development version from GitHub via
 
 Acknowledgments
 -----------
-This package is based on the [eponymous package by Ryan Price](https://github.com/ryapric/loggit2), specifically version 2.1.1.
+This package is based on the [eponymous package by Ryan Price](https://github.com/ryapric/loggit), specifically version 2.1.1.
 
-Due to technical reasons, this repository is not a GitHub fork of [Ryan's repository](https://github.com/ryapric/loggit2).
+Due to technical reasons, this repository is not a GitHub fork of [Ryan's repository](https://github.com/ryapric/loggit).
+
+```{r, include = FALSE}
+options(old)
+```
\ No newline at end of file
diff --git a/README.md b/README.md
index fa0db8b..c9e4fdd 100644
--- a/README.md
+++ b/README.md
@@ -3,7 +3,7 @@
 <!-- badges: start -->
 
 [![CRAN_Status_Badge](http://www.r-pkg.org/badges/version/loggit2)](https://cran.r-project.org/package=loggit2)
-[![Downloads](https://cranlogs.r-pkg.org/badges/last-week/loggit2)](https://cran.r-project.org/package=loggit2)
+[![Downloads](https://cranlogs.r-pkg.org/badges/grand-total/loggit2)](https://cran.r-project.org/package=loggit2)
 [![R-CMD-check](https://github.com/MEO265/loggit2/actions/workflows/R-CMD-check.yaml/badge.svg)](https://github.com/MEO265/loggit2/actions/workflows/R-CMD-check.yaml)
 [![codecov](https://codecov.io/gh/MEO265/loggit2/graph/badge.svg)](https://codecov.io/gh/MEO265/loggit2)
 <!-- badges: end -->
@@ -29,7 +29,7 @@ for the Getting Started guide.
 
 Additionally, the boilerplate to get going with `loggit2` is minimal at
 worst. No need to write custom formatters, handlers, levels, etc. –
-***just loggit!***
+***just loggit2!***
 
 ## Usage
 
@@ -44,13 +44,13 @@ at the desired locations.
 
 ``` r
 loggit2::message("This is a message")
-#> {"timestamp": "2024-04-30T18:19:08+0200", "log_lvl": "INFO", "log_msg": "This is a message__LF__"}
+#> {"timestamp": "2024-05-02T18:27:28+0200", "log_lvl": "INFO", "log_msg": "This is a message__LF__"}
 #> This is a message
 loggit2::warning("This is a warning")
-#> {"timestamp": "2024-04-30T18:19:08+0200", "log_lvl": "WARN", "log_msg": "This is a warning"}
+#> {"timestamp": "2024-05-02T18:27:28+0200", "log_lvl": "WARN", "log_msg": "This is a warning"}
 #> Warning: This is a warning
 loggit2::stop("This is an error")
-#> {"timestamp": "2024-04-30T18:19:08+0200", "log_lvl": "ERROR", "log_msg": "This is an error"}
+#> {"timestamp": "2024-05-02T18:27:28+0200", "log_lvl": "ERROR", "log_msg": "This is an error"}
 #> Error in eval(expr, envir, enclos): This is an error
 ```
 
@@ -83,18 +83,18 @@ it’ll become a structured log.
 
 ``` r
 loggit2::loggit("ERROR", "This will log an error", anything_else = "you want to include")
-#> {"timestamp": "2024-04-30T18:19:08+0200", "log_lvl": "ERROR", "log_msg": "This will log an error", "anything_else": "you want to include"}
+#> {"timestamp": "2024-05-02T18:27:28+0200", "log_lvl": "ERROR", "log_msg": "This will log an error", "anything_else": "you want to include"}
 
 # Read log file into data frame to implement logic based on entries
 loggit2::read_logs()
 #>                  timestamp log_lvl                   log_msg       anything_else
-#> 1 2024-04-30T18:19:08+0200    INFO       This is a message\n                    
-#> 2 2024-04-30T18:19:08+0200    WARN         This is a warning                    
-#> 3 2024-04-30T18:19:08+0200   ERROR          This is an error                    
-#> 4 2024-04-30T18:19:08+0200    INFO This is another message\n                    
-#> 5 2024-04-30T18:19:08+0200    WARN   This is another warning                    
-#> 6 2024-04-30T18:19:08+0200   ERROR     This is another error                    
-#> 7 2024-04-30T18:19:08+0200   ERROR    This will log an error you want to include
+#> 1 2024-05-02T18:27:28+0200    INFO       This is a message\n                    
+#> 2 2024-05-02T18:27:28+0200    WARN         This is a warning                    
+#> 3 2024-05-02T18:27:28+0200   ERROR          This is an error                    
+#> 4 2024-05-02T18:27:28+0200    INFO This is another message\n                    
+#> 5 2024-05-02T18:27:28+0200    WARN   This is another warning                    
+#> 6 2024-05-02T18:27:28+0200   ERROR     This is another error                    
+#> 7 2024-05-02T18:27:28+0200   ERROR    This will log an error you want to include
 ```
 
 Check out the
@@ -113,8 +113,8 @@ or, get the latest development version from GitHub via
 
 ## Acknowledgments
 
-This package is based on the ["loggit" package by Ryan
-Price](https://github.com/ryapric/loggit2), specifically version 2.1.1.
+This package is based on the [eponymous package by Ryan
+Price](https://github.com/ryapric/loggit), specifically version 2.1.1.
 
 Due to technical reasons, this repository is not a GitHub fork of
-[Ryan’s repository](https://github.com/ryapric/loggit2).
+[Ryan’s repository](https://github.com/ryapric/loggit).
diff --git a/man/get_logfile.Rd b/man/get_logfile.Rd
index 5553565..d9b8e4c 100644
--- a/man/get_logfile.Rd
+++ b/man/get_logfile.Rd
@@ -6,6 +6,9 @@
 \usage{
 get_logfile()
 }
+\value{
+The log file path.
+}
 \description{
 Return the log file that \code{loggit()} will write to.
 }
diff --git a/man/get_timestamp_format.Rd b/man/get_timestamp_format.Rd
index e1fc29a..73b3192 100644
--- a/man/get_timestamp_format.Rd
+++ b/man/get_timestamp_format.Rd
@@ -6,6 +6,9 @@
 \usage{
 get_timestamp_format()
 }
+\value{
+The timestamp format.
+}
 \description{
 Get timestamp format for use in output logs.
 }
diff --git a/man/loggit.Rd b/man/loggit.Rd
index 6281155..dc0c996 100644
--- a/man/loggit.Rd
+++ b/man/loggit.Rd
@@ -30,6 +30,9 @@ and "ERROR"? Defaults to \code{FALSE}.}
 \item{sanitizer}{\link[=sanitizers]{Sanitizer function} to run over elements in log data.
 Defaults to \code{\link[=default_ndjson_sanitizer]{default_ndjson_sanitizer()}}.}
 }
+\value{
+Invisible \code{NULL}.
+}
 \description{
 Log entries to a \href{https://github.com/ndjson}{ndjson} log file, defined by \code{\link[=set_logfile]{set_logfile()}}.
 }
diff --git a/man/message.Rd b/man/message.Rd
index f49c797..a90753e 100644
--- a/man/message.Rd
+++ b/man/message.Rd
@@ -21,6 +21,9 @@ message(..., domain = NULL, appendLF = TRUE, .loggit = TRUE, echo = TRUE)
 
 \item{echo}{Should \code{loggit()}'s log entry be echoed to the console, as well? Defaults to \code{TRUE}.}
 }
+\value{
+Invisible \code{NULL}.
+}
 \description{
 This function is identical to base R's \code{\link[base:message]{message}},
 but it includes logging of the exception message via \code{loggit()}.
diff --git a/man/rotate_logs.Rd b/man/rotate_logs.Rd
index 85bce2b..167a078 100644
--- a/man/rotate_logs.Rd
+++ b/man/rotate_logs.Rd
@@ -11,6 +11,9 @@ rotate_logs(rotate_lines = 100000L, logfile = get_logfile())
 
 \item{logfile}{Log file to truncate.}
 }
+\value{
+Invisible \code{NULL}.
+}
 \description{
 Truncates the log file to the line count provided as \code{rotate_lines}.
 }
diff --git a/man/set_logfile.Rd b/man/set_logfile.Rd
index 93ace87..325b367 100644
--- a/man/set_logfile.Rd
+++ b/man/set_logfile.Rd
@@ -15,6 +15,9 @@ If \code{NULL} will set to \verb{<tmpdir>/loggit.log}.}
 
 \item{create}{Create the log file if it does not exist? Defaults to \code{TRUE}.}
 }
+\value{
+Invisible \code{NULL}.
+}
 \description{
 Set the log file that loggit will write to.
 }
diff --git a/man/set_timestamp_format.Rd b/man/set_timestamp_format.Rd
index e5a395d..12a589b 100644
--- a/man/set_timestamp_format.Rd
+++ b/man/set_timestamp_format.Rd
@@ -13,6 +13,9 @@ set_timestamp_format(ts_format = "\%Y-\%m-\%dT\%H:\%M:\%S\%z", confirm = TRUE)
 \item{confirm}{Print confirmation message of timestamp format? Defaults to
 \code{TRUE}.}
 }
+\value{
+Invisible \code{NULL}.
+}
 \description{
 Set timestamp format for use in output logs.
 }
diff --git a/man/stop.Rd b/man/stop.Rd
index 97ec928..17d4266 100644
--- a/man/stop.Rd
+++ b/man/stop.Rd
@@ -21,6 +21,9 @@ stop(..., call. = TRUE, domain = NULL, .loggit = TRUE, echo = TRUE)
 
 \item{echo}{Should \code{loggit()}'s log entry be echoed to the console, as well? Defaults to \code{TRUE}.}
 }
+\value{
+No return value.
+}
 \description{
 This function is identical to base R's \code{\link[base:stop]{stop}},
 but it includes logging of the exception message via \code{loggit()}.
diff --git a/tests/testthat/_snaps/utils/test.loggit b/tests/testthat/_snaps/utils/test.loggit
new file mode 100644
index 0000000..f4cf55b
--- /dev/null
+++ b/tests/testthat/_snaps/utils/test.loggit
@@ -0,0 +1,3 @@
+{"timestamp": "2024-05-01T20:09:27+0200", "log_lvl": "INFO", "log_msg": "log_98"}
+{"timestamp": "2024-05-01T20:09:27+0200", "log_lvl": "INFO", "log_msg": "log_99"}
+{"timestamp": "2024-05-01T20:09:27+0200", "log_lvl": "INFO", "log_msg": "log_100 __LF__", "extra": "extra"}
diff --git a/tests/testthat/test-json.R b/tests/testthat/test-json.R
index 628f253..47b63c4 100644
--- a/tests/testthat/test-json.R
+++ b/tests/testthat/test-json.R
@@ -10,7 +10,7 @@ test_that("write_logs() and read_logs() work in tandem", {
   log_df_want <- data.frame(
     log_lvl = "INFO",
     log_msg = c("msg1", "msg2", "msg3", "msg4: should: be :displayed:"),
-    extra = c("", "", "", "fields"),
+    extra = c(NA_character_, NA_character_, NA_character_, "fields"),
     stringsAsFactors = FALSE
   )
 
diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R
index 275690b..0163aa6 100644
--- a/tests/testthat/test-utils.R
+++ b/tests/testthat/test-utils.R
@@ -10,7 +10,7 @@ test_that("rotate_logs works on default log file", {
 
   rotate_lines <- 50L
   rotate_logs(rotate_lines = rotate_lines)
-log_df <- read_logs()
+  log_df <- read_logs()
   expect_identical(nrow(log_df), rotate_lines)
 
   rotate_lines <- 0L
@@ -38,13 +38,13 @@ test_that("rotate_logs works on non-default log file", {
 
   rotate_lines <- 150L
   rotate_logs(rotate_lines = rotate_lines, other_logfile)
-  log_df <- read_logs( other_logfile)
-expect_identical(nrow(log_df), 100L)
+  log_df <- read_logs(other_logfile)
+  expect_identical(nrow(log_df), 100L)
 
   rotate_lines <- 50L
   rotate_logs(rotate_lines = rotate_lines, other_logfile)
   log_df <- read_logs(other_logfile)
-expect_identical(nrow(log_df), rotate_lines)
+  expect_identical(nrow(log_df), rotate_lines)
 
   rotate_lines <- 0L
   rotate_logs(rotate_lines = rotate_lines, other_logfile)
@@ -56,3 +56,17 @@ expect_identical(nrow(log_df), rotate_lines)
   expect_identical(nrow(log_df), 1L)
 })
 cleanup()
+
+test_that("rotate_logs preserves sanitization", {
+
+  tmp_log <- file.path(tempdir(), "test.loggit")
+  file.copy("testdata/test.loggit", tmp_log)
+  on.exit(file.remove(tmp_log))
+
+  bevor <- read_logs(tmp_log)
+
+  rotate_lines <- 3L
+  rotate_logs(rotate_lines = rotate_lines, logfile = tmp_log)
+  expect_snapshot_file(tmp_log)
+
+})
\ No newline at end of file
diff --git a/tests/testthat/testdata/test.loggit b/tests/testthat/testdata/test.loggit
new file mode 100644
index 0000000..f4aef44
--- /dev/null
+++ b/tests/testthat/testdata/test.loggit
@@ -0,0 +1,6 @@
+{"timestamp": "2024-05-01T20:09:27+0200", "log_lvl": "INFO", "log_msg": "log_1", "extra": "extra"}
+{"timestamp": "2024-05-01T20:09:27+0200", "log_lvl": "INFO", "log_msg": "log_96"}
+{"timestamp": "2024-05-01T20:09:27+0200", "log_lvl": "INFO", "log_msg": "log_97"}
+{"timestamp": "2024-05-01T20:09:27+0200", "log_lvl": "INFO", "log_msg": "log_98"}
+{"timestamp": "2024-05-01T20:09:27+0200", "log_lvl": "INFO", "log_msg": "log_99"}
+{"timestamp": "2024-05-01T20:09:27+0200", "log_lvl": "INFO", "log_msg": "log_100 __LF__", "extra": "extra"}
diff --git a/vignettes/getting-started.Rmd b/vignettes/getting-started.Rmd
index fdbb1b7..96c30f6 100644
--- a/vignettes/getting-started.Rmd
+++ b/vignettes/getting-started.Rmd
@@ -12,7 +12,7 @@ knitr::opts_chunk$set(
   collapse = TRUE,
   comment = "#>"
 )
-options(width = 200)
+old <- options(width = 200)
 ```
 
 ```{r setup, include = FALSE}
@@ -143,3 +143,6 @@ can wrap this in a call to `.onLoad()`, so that logging is set on package
 load. If not, then make the set call as soon as possible (e.g. at the top of
 your script(s), right after your calls to `library()`); otherwise, no logs
 will be written to persistent storage!
+```{r, include = FALSE}
+options(old)
+```
\ No newline at end of file