From e3e98bcbfe27f8694172c7049bdd6809836ef0c9 Mon Sep 17 00:00:00 2001
From: Tim Cadman <41470917+timcadman@users.noreply.github.com>
Date: Fri, 9 Jan 2026 09:42:18 +0100
Subject: [PATCH 1/6] refactor colnames

---
 R/colnamesDS.R | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 R/colnamesDS.R

diff --git a/R/colnamesDS.R b/R/colnamesDS.R
new file mode 100644
index 00000000..6dc2e99e
--- /dev/null
+++ b/R/colnamesDS.R
@@ -0,0 +1,17 @@
+#'
+#' @title Returns the column names of a data frame or matrix
+#' @description This function is similar to R function \code{colnames}.
+#' @details The function returns the column names of the input dataframe or matrix
+#' @param x a string character, the name of a dataframe or matrix
+#' @return the column names of the input object
+#' @author Demetris Avraam, for DataSHIELD Development Team
+#' @export
+#'
+colnamesDS <- function(x){
+  x.val <- .loadServersideObject(x)
+  .checkClass(obj = x.val, obj_name = x, permitted_classes = c("data.frame", "matrix"))
+  out <- colnames(x.val)
+  return(out)
+}
+#AGGREGATE FUNCTION
+# colnamesDS

From c130e19f8e466a50365dbf94baf0bd4044997ed7 Mon Sep 17 00:00:00 2001
From: Tim Cadman <41470917+timcadman@users.noreply.github.com>
Date: Fri, 9 Jan 2026 09:44:19 +0100
Subject: [PATCH 2/6] added reusable functions

---
 R/utils.R | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 R/utils.R

diff --git a/R/utils.R b/R/utils.R
new file mode 100644
index 00000000..6fd53936
--- /dev/null
+++ b/R/utils.R
@@ -0,0 +1,42 @@
+#' Load a Server-Side Object by Name
+#'
+#' Evaluates a character string referring to an object name and returns the corresponding
+#' object from the parent environment. If the object does not exist, an error is raised.
+#'
+#' @param x A character string naming the object to be retrieved.
+#' @return The evaluated R object referred to by `x`.
+#' @noRd
+.loadServersideObject <- function(x) {
+  tryCatch(
+    eval(parse(text = x), envir = parent.frame(2)), 
+    error = function(e) {
+      stop("The server-side object", " '", x, "' ", "does not exist")
+    }
+  )
+}
+
+#' Check Class of a Server-Side Object
+#'
+#' Verifies that a given object is of an allowed class. If not, raises an informative error
+#' message listing the permitted classes and the actual class of the object.
+#'
+#' @param obj The object whose class should be checked.
+#' @param obj_name A character string with the name of the object (used in error messages).
+#' @param permitted_classes A character vector of allowed class names.
+#' @importFrom glue glue glue_collapse
+#' @return Invisibly returns `TRUE` if the class check passes; otherwise throws an error.
+#' @noRd
+.checkClass <- function(obj, obj_name, permitted_classes) {
+  typ <- class(obj)
+  
+  if (!any(permitted_classes %in% typ)) {
+    msg <- glue(
+      "The server-side object must be of type {glue_collapse(permitted_classes, sep = ' or ')}. ",
+      "'{obj_name}' is type {typ}."
+    )
+    
+    stop(msg, call. = FALSE)
+  }
+  
+  invisible(TRUE)
+}

From 490c1f89df29dafadf8ba1c32ced590fd72b53b6 Mon Sep 17 00:00:00 2001
From: Tim Cadman <41470917+timcadman@users.noreply.github.com>
Date: Fri, 9 Jan 2026 09:45:16 +0100
Subject: [PATCH 3/6] added tests

---
 tests/testthat/test-smk-colnamesDS.R | 67 ++++++++++++++++++++++++++++
 tests/testthat/test-smk-utils.R      | 53 ++++++++++++++++++++++
 2 files changed, 120 insertions(+)
 create mode 100644 tests/testthat/test-smk-colnamesDS.R
 create mode 100644 tests/testthat/test-smk-utils.R

diff --git a/tests/testthat/test-smk-colnamesDS.R b/tests/testthat/test-smk-colnamesDS.R
new file mode 100644
index 00000000..36c4ceef
--- /dev/null
+++ b/tests/testthat/test-smk-colnamesDS.R
@@ -0,0 +1,67 @@
+#-------------------------------------------------------------------------------
+# Copyright (c) 2019-2022 University of Newcastle upon Tyne. All rights reserved.
+# Copyright (c) 2022-2025 Arjuna Technologies, Newcastle upon Tyne. All rights reserved.
+#
+# This program and the accompanying materials
+# are made available under the terms of the GNU Public License v3.0.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#-------------------------------------------------------------------------------
+
+#
+# Set up
+#
+
+# context("colnamesDS::smk::setup")
+
+#
+# Tests
+#
+
+# context("colnamesDS::smk::data.frame")
+test_that("simple colnamesDS, data.frame", {
+    input <- data.frame(v1 = c(0.0, 1.0, 2.0, 3.0, 4.0), v2 = c(4.0, 3.0, 2.0, 1.0, 0.0))
+
+    res <- colnamesDS("input")
+
+    expect_equal(class(res), "character")
+    expect_length(res, 2)
+    expect_true("v1" %in% res)
+    expect_true("v2" %in% res)
+})
+
+# context("colnamesDS::smk::data.matrix")
+test_that("simple colnamesDS, data.matrix", {
+    input <- data.matrix(data.frame(v1 = c(0.0, 1.0, 2.0, 3.0, 4.0), v2 = c(4.0, 3.0, 2.0, 1.0, 0.0)))
+
+    res <- colnamesDS("input")
+
+    expect_equal(class(res), "character")
+    expect_length(res, 2)
+    expect_true("v1" %in% res)
+    expect_true("v2" %in% res)
+})
+
+test_that("colnamesDS throws error when object does not exist", {
+  expect_error(
+    colnamesDS("nonexistent_object"),
+    regexp = "does not exist"
+  )
+})
+
+test_that("colnamesDS throws error when object is not data.frame or matrix", {
+  bad_input <- list(a = 1:3, b = 4:6)
+  expect_error(
+    colnamesDS("bad_input"),
+    regexp = "must be of type data.frame or matrix"
+  )
+})
+
+#
+# Done
+#
+
+# context("colnamesDS::smk::shutdown")
+
+# context("colnamesDS::smk::done")
\ No newline at end of file
diff --git a/tests/testthat/test-smk-utils.R b/tests/testthat/test-smk-utils.R
new file mode 100644
index 00000000..2c733cfb
--- /dev/null
+++ b/tests/testthat/test-smk-utils.R
@@ -0,0 +1,53 @@
+
+#-------------------------------------------------------------------------------
+# Copyright (c) 2019-2022 University of Newcastle upon Tyne. All rights reserved.
+#
+# This program and the accompanying materials
+# are made available under the terms of the GNU Public License v3.0.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#-------------------------------------------------------------------------------
+
+#
+# Set up
+#
+
+## When .loadServersideObject is called, the actual data exists two levels below the function,
+## i.e. data in global env --> ds function --> .loadServersideObject. We recreate this in
+## the test environment with a wrapper function.
+.dsFunctionWrapper <- function(x) {
+  .loadServersideObject(x)
+}
+
+# context("utils::smk::setup")
+test_that(".loadServersideObject() returns existing object", {
+  test_df <- data.frame(a = 1:3)
+  result <- .dsFunctionWrapper("test_df")
+  expect_identical(result, test_df)
+})
+
+test_that(".loadServersideObject() throws error for missing object", {
+  expect_error(
+    .dsFunctionWrapper("test_df"),
+    regexp = "does not exist"
+  )
+})
+
+test_that(".checkClass() passes for correct class", {
+  df <- data.frame(a = 1)
+  expect_invisible(
+    .checkClass(df, "df", c("data.frame", "matrix"))
+  )
+})
+
+test_that(".checkClass() throws informative error for wrong class", {
+  x <- list(a = 1)
+  expect_error(
+    .checkClass(x, "x", c("data.frame", "matrix")),
+    regexp = "must be of type data.frame or matrix"
+  )
+})
+
+# context("utils::smk::shutdown")
+# context("utils::smk::done")

From 4305357be60acb412aed9b03e938c555bae76765 Mon Sep 17 00:00:00 2001
From: Tim Cadman <41470917+timcadman@users.noreply.github.com>
Date: Fri, 9 Jan 2026 09:52:21 +0100
Subject: [PATCH 4/6] redocumented package

---
 NAMESPACE         |  3 +++
 man/colnamesDS.Rd | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 man/colnamesDS.Rd

diff --git a/NAMESPACE b/NAMESPACE
index c8a715fb..db4a5378 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -25,6 +25,7 @@ export(changeRefGroupDS)
 export(checkNegValueDS)
 export(checkPermissivePrivacyControlLevel)
 export(classDS)
+export(colnamesDS)
 export(completeCasesDS)
 export(corDS)
 export(corTestDS)
@@ -139,3 +140,5 @@ import(gamlss.dist)
 import(mice)
 importFrom(gamlss.dist,pST3)
 importFrom(gamlss.dist,qST3)
+importFrom(glue,glue)
+importFrom(glue,glue_collapse)
diff --git a/man/colnamesDS.Rd b/man/colnamesDS.Rd
new file mode 100644
index 00000000..e13abde4
--- /dev/null
+++ b/man/colnamesDS.Rd
@@ -0,0 +1,23 @@
+% Generated by roxygen2: do not edit by hand
+% Please edit documentation in R/colnamesDS.R
+\name{colnamesDS}
+\alias{colnamesDS}
+\title{Returns the column names of a data frame or matrix}
+\usage{
+colnamesDS(x)
+}
+\arguments{
+\item{x}{a string character, the name of a dataframe or matrix}
+}
+\value{
+the column names of the input object
+}
+\description{
+This function is similar to R function \code{colnames}.
+}
+\details{
+The function returns the column names of the input dataframe or matrix
+}
+\author{
+Demetris Avraam, for DataSHIELD Development Team
+}

From 57de79ff97fb95e4240f9d55a65a7d481454d29a Mon Sep 17 00:00:00 2001
From: Tim Cadman <41470917+timcadman@users.noreply.github.com>
Date: Fri, 9 Jan 2026 10:20:10 +0100
Subject: [PATCH 5/6] added PR template

---
 PULL_REQUEST_TEMPLATE.md | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 PULL_REQUEST_TEMPLATE.md

diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md
new file mode 100644
index 00000000..1cbc8c4a
--- /dev/null
+++ b/PULL_REQUEST_TEMPLATE.md
@@ -0,0 +1,20 @@
+## Instructions & checklist for PR author
+
+### Description of changes
+[Add descriptions of changes made]
+
+### Refactor instructions
+- [ ] Replaced  `x <- eval(parse(text = x.name), envir = parent.frame())` with `x <- .loadServersideObject(x)`
+- [ ] If necessary, check the class of the object using `.checkClass()`
+
+### Testing instructions
+- [ ] Writen server-side unit tests for unhappy flow
+- [ ] Run `devtools::test(filter = "smk-|disc|arg")` and check it passes
+- [ ] Run `devtools::check(args = '--no-tests')` and check it passes (we run tests separately to skip performance checks)
+- [ ] Run `devtools::build()` and check it builds without errors
+
+## Instructions & checklist for PR reviewers
+- [ ] Run `devtools::test(filter = "smk-|disc|arg")` and check it passes
+- [ ] Run `devtools::check(args = '--no-tests')` and check it passes (we run tests separately to skip performance checks)
+- [ ] Run `devtools::build()` and check it builds without errors
+

From 40eb96a8bbec964a2a3af83fb5825dd561b2c3fc Mon Sep 17 00:00:00 2001
From: Tim Cadman <41470917+timcadman@users.noreply.github.com>
Date: Fri, 9 Jan 2026 11:30:05 +0100
Subject: [PATCH 6/6] use get instead of eval as more secure

---
 R/utils.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/utils.R b/R/utils.R
index 6fd53936..56e24928 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -8,7 +8,7 @@
 #' @noRd
 .loadServersideObject <- function(x) {
   tryCatch(
-    eval(parse(text = x), envir = parent.frame(2)), 
+    get(x, envir = parent.frame(2)), 
     error = function(e) {
       stop("The server-side object", " '", x, "' ", "does not exist")
     }