Fix for references with simple test case added

OHDSI · Sep 13, 2023 · 903bb76 · 903bb76
1 parent 6935ccf
commit 903bb76
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 3 deletions.
diff --git a/R/SubsetDefinitions.R b/R/SubsetDefinitions.R
@@ -75,7 +75,7 @@ CohortSubsetDefinition <- R6::R6Class(
     #' @param overwrite if a subset operator of the same ID is present, replace it with a new definition
     addSubsetOperator = function(subsetOperator) {
       checkmate::assertR6(subsetOperator, "SubsetOperator")
-      private$.subsetOperators <- c(private$.subsetOperators, subsetOperator)
+      private$.subsetOperators <- c(private$.subsetOperators, subsetOperator$clone(deep = TRUE))
       self
     },
 
@@ -189,7 +189,8 @@ CohortSubsetDefinition <- R6::R6Class(
     #' @field subsetOperators list of subset operations
     subsetOperators = function(subsetOperators) {
       if (missing(subsetOperators)) {
-        return(private$.subsetOperators)
+        # We don't want to return references to the operators in case users modify them after this
+        return(lapply(private$.subsetOperators, function(x) { x$clone(deep = TRUE) }))
       }
 
       checkmate::assertList(subsetOperators, types = "SubsetOperator")

diff --git a/tests/testthat/test-Subsets.R b/tests/testthat/test-Subsets.R
@@ -44,7 +44,11 @@ test_that("Subset definition", {
   checkmate::expect_list(listDef)
   expect_equal(length(subsetDef$subsetOperators), length(listDef$subsetOperators))
   checkmate::expect_character(subsetDef$toJSON())
-
+  # check reference isn't passed
+  operators <- subsetDef$subsetOperators
+  # Operators should not be modfiable after being added to the subset definition
+  operators[[1]]$cohortIds <- 22
+  expect_equal(subsetDef$subsetOperators[[1]]$cohortIds, 11)
 
   # Check serialized version is identical to code defined version
   subsetDef2 <- CohortSubsetDefinition$new(subsetDef$toJSON())