From 8f820b027cb7fb5da70f8f66e8f6e88abd1d4f8b Mon Sep 17 00:00:00 2001
From: Abhinav Pandey <abhinavpandey1230@gmail.com>
Date: Wed, 9 Oct 2024 11:56:54 +0530
Subject: [PATCH] Apply suggestions from code review

Co-authored-by: Chris Black <chris@ckblack.org>
---
 base/db/R/check.missing.files.R                   | 1 -
 base/db/R/convert_input.R                         | 6 +++++-
 base/db/R/get.machine.info.R                      | 4 ++--
 base/db/tests/testthat/test.check.missing.files.R | 6 ++----
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/base/db/R/check.missing.files.R b/base/db/R/check.missing.files.R
index 29ce044f68..f3a496cf5d 100644
--- a/base/db/R/check.missing.files.R
+++ b/base/db/R/check.missing.files.R
@@ -1,7 +1,6 @@
 #' Function to check if result has empty or missing files
 #'
 #' @param result A list of dataframes with file paths
-#' @param outname Name of the output file
 #' @param existing.input Existing input records
 #' @param existing.dbfile Existing dbfile records
 #' @return A list of dataframes with file paths, a list of strings with the output file name, a list of existing input records, and a list of existing dbfile records
diff --git a/base/db/R/convert_input.R b/base/db/R/convert_input.R
index a074a68938..042c9da08d 100644
--- a/base/db/R/convert_input.R
+++ b/base/db/R/convert_input.R
@@ -380,7 +380,7 @@ convert_input <-
     if (!is.null(ensemble) && ensemble) {
       return.all <-TRUE
       
-    } else{
+    } else {
       return.all <- FALSE
     }
       existing.dbfile <- dbfile.input.check(siteid = site.id,
@@ -518,6 +518,10 @@ convert_input <-
   # Get machine information
   machine.info <- get.machine.info(host, input.args = input.args, input.id = input.id)
 
+  if (any(sapply(machine.info, is.null))) {
+    PEcAn.logger::logger.error("failed lookup of inputs or dbfiles")
+    return(NULL)  
+  }
   machine <- machine.info$machine
   input <- machine.info$input
   dbfile <- machine.info$dbfile
diff --git a/base/db/R/get.machine.info.R b/base/db/R/get.machine.info.R
index 31f489dadd..14123a586e 100644
--- a/base/db/R/get.machine.info.R
+++ b/base/db/R/get.machine.info.R
@@ -18,7 +18,7 @@ get_machine_info <- function(host, input.args, input.id = NULL, con = NULL) {
     return(NULL)
   }
 
-  if (missing(input.id) || is.na(input.id) || is.null(input.id)) {
+  if (is.na(input.id) || is.null(input.id)) {
     input <- dbfile <- NULL
   } else {
     input <- db.query(paste("SELECT * from inputs where id =", input.id), con)
@@ -71,7 +71,7 @@ get_machine_info <- function(host, input.args, input.id = NULL, con = NULL) {
 #' @param con database connection
 #' @return list of machine host and machine information
 #' @author Abhinav Pandey
-get.machine.host <- function(host, con = NULL) {
+get_machine_host <- function(host, con) {
   #Grab machine info of host machine
   machine.host <- ifelse(host$name == "localhost", PEcAn.remote::fqdn(), host$name)
   machine <- db.query(paste0(
diff --git a/base/db/tests/testthat/test.check.missing.files.R b/base/db/tests/testthat/test.check.missing.files.R
index bc61bb1ad4..75a531283d 100644
--- a/base/db/tests/testthat/test.check.missing.files.R
+++ b/base/db/tests/testthat/test.check.missing.files.R
@@ -1,7 +1,7 @@
 test_that("`check_missing_files()` able to return correct missing files", {
     # Mock `purrr::map_dfr`
-    mocked_res <- mockery::mock(data.frame(file = c("A", "B"), file_size = c(100, 200), missing = c(FALSE, FALSE), empty = c(FALSE, FALSE)))
-    mockery::stub(check_missing_files, "purrr::map_dfr", mocked_res)
+    mocked_size <- mockery::mock(100,200)
+    mockery::stub(check_missing_files, "file.size", mocked_res)
 
     res <- check_missing_files(
         result = list(data.frame(file = c("A", "B"))),
@@ -9,8 +9,6 @@ test_that("`check_missing_files()` able to return correct missing files", {
         existing.dbfile = data.frame()
     )
 
-    # Print the structure of `res` for debugging
-    str(res)
 
     expect_equal(length(res), 2)
     expect_true(is.list(res[[1]]))