From a578be2dfc274c10a100c90e1febf1474d5289f7 Mon Sep 17 00:00:00 2001 From: Abhinav Pandey Date: Wed, 9 Oct 2024 11:45:13 +0530 Subject: [PATCH] Applied changes as suggested by @infotroph Signed-off-by: Abhinav Pandey --- base/db/R/add.database.entries.R | 161 ++++++++++++++++--------------- base/db/R/convert_input.R | 22 +++-- 2 files changed, 100 insertions(+), 83 deletions(-) diff --git a/base/db/R/add.database.entries.R b/base/db/R/add.database.entries.R index d3eb994a64..8b36e88439 100644 --- a/base/db/R/add.database.entries.R +++ b/base/db/R/add.database.entries.R @@ -23,99 +23,108 @@ add.database.entries <- function( result, con, start_date, - end_date, write, overwrite, + end_date, overwrite, insert.new.file, input.args, machine, mimetype, formatname, allow.conflicting.dates, ensemble, ensemble_name, existing.input, existing.dbfile, input) { - if (write) { - # Setup newinput. This list will contain two variables: a vector of input IDs and a vector of DB IDs for each entry in result. - # This list will be returned. - newinput <- list(input.id = NULL, dbfile.id = NULL) # Blank vectors are null. + # Setup newinput. This list will contain two variables: a vector of input IDs and a vector of DB IDs for each entry in result. + # This list will be returned. + newinput <- list(input.id = NULL, dbfile.id = NULL) # Blank vectors are null. - for (i in 1:length(result)) { # Master for loop - id_not_added <- TRUE + for (i in 1:length(result)) { # Master for loop + id_not_added <- TRUE - if (!is.null(existing.input) && nrow(existing.input[[i]]) > 0 && - (existing.input[[i]]$start_date != start_date || existing.input[[i]]$end_date != end_date)) { - # Updating record with new dates - db.query(paste0("UPDATE inputs SET start_date='", start_date, "', end_date='", end_date, "' WHERE id=", existing.input[[i]]$id), con) - id_not_added <- FALSE + if (!is.null(existing.input) && nrow(existing.input[[i]]) > 0 && + (existing.input[[i]]$start_date != start_date || existing.input[[i]]$end_date != end_date)) { + # Updating record with new dates + db.query( + paste0( + "UPDATE inputs SET start_date='", start_date, "', end_date='", end_date, + "' WHERE id=", existing.input[[i]]$id + ), + con + ) + id_not_added <- FALSE - # The overall structure of this loop has been set up so that exactly one input.id and one dbfile.id will be written to newinput every iteration. - newinput$input.id <- c(newinput$input.id, existing.input[[i]]$id) - newinput$dbfile.id <- c(newinput$dbfile.id, existing.dbfile[[i]]$id) - } - - if (overwrite) { - # A bit hacky, but need to make sure that all fields are updated to expected values (i.e., what they'd be if convert_input was creating a new record) - if (!is.null(existing.input) && nrow(existing.input[[i]]) > 0) { - db.query(paste0("UPDATE inputs SET name='", basename(dirname(result[[i]]$file[1])), "' WHERE id=", existing.input[[i]]$id), con) - } + # The overall structure of this loop has been set up so that exactly one input.id and one dbfile.id will be written to newinput every iteration. + newinput$input.id <- c(newinput$input.id, existing.input[[i]]$id) + newinput$dbfile.id <- c(newinput$dbfile.id, existing.dbfile[[i]]$id) + } - if (!is.null(existing.dbfile) && nrow(existing.dbfile[[i]]) > 0) { - db.query(paste0("UPDATE dbfiles SET file_path='", dirname(result[[i]]$file[1]), "', file_name='", result[[i]]$dbfile.name[1], "' WHERE id=", existing.dbfile[[i]]$id), con) - } + if (overwrite) { + # A bit hacky, but need to make sure that all fields are updated to expected values (i.e., what they'd be if convert_input was creating a new record) + if (!is.null(existing.input) && nrow(existing.input[[i]]) > 0) { + db.query( + paste0( + "UPDATE dbfiles SET file_path='", dirname(result[[i]]$file[1]), + "', file_name='", result[[i]]$dbfile.name[1], + "' WHERE id=", existing.dbfile[[i]]$id + ), + con + ) } - # If there is no ensemble then for each record there should be one parent - # But when you have ensembles, all of the members have one parent !! - parent.id <- if (is.numeric(ensemble)) { - ifelse(is.null(input[[i]]), NA, input[[1]]$id) - } else { - ifelse(is.null(input[[i]]), NA, input[[i]]$id) + if (!is.null(existing.dbfile) && nrow(existing.dbfile[[i]]) > 0) { + db.query(paste0( + "UPDATE dbfiles SET file_path='", dirname(result[[i]]$file[1]), + "', file_name='", result[[i]]$dbfile.name[1], + "' WHERE id=", existing.dbfile[[i]]$id + ), con) } + } + # If there is no ensemble then for each record there should be one parent + # But when you have ensembles, all of the members have one parent !! + parent.id <- if (is.numeric(ensemble)) { + ifelse(is.null(input[[i]]), NA, input[[1]]$id) + } else { + ifelse(is.null(input[[i]]), NA, input[[i]]$id) + } - if ("newsite" %in% names(input.args) && !is.null(input.args[["newsite"]])) { - site.id <- input.args$newsite - } - if (insert.new.file && id_not_added) { - dbfile.id <- dbfile.insert( - in.path = dirname(result[[i]]$file[1]), - in.prefix = result[[i]]$dbfile.name[1], - "Input", - existing.input[[i]]$id, - con, - reuse = TRUE, - hostname = machine$hostname - ) + if ("newsite" %in% names(input.args) && !is.null(input.args[["newsite"]])) { + site.id <- input.args$newsite + } - newinput$input.id <- c(newinput$input.id, existing.input[[i]]$id) - newinput$dbfile.id <- c(newinput$dbfile.id, dbfile.id) - } else if (id_not_added) { - # This is to tell input.insert if we are writing ensembles - # Why does it need it? Because it checks for inputs with the same time period, site, and machine - # and if it returns something it does not insert anymore, but for ensembles, it needs to bypass this condition - ens.flag <- if (!is.null(ensemble) | is.null(ensemble_name)) TRUE else FALSE + if (insert.new.file && id_not_added) { + dbfile.id <- dbfile.insert( + in.path = dirname(result[[i]]$file[1]), + in.prefix = result[[i]]$dbfile.name[1], + "Input", + existing.input[[i]]$id, + con, + reuse = TRUE, + hostname = machine$hostname + ) - new_entry <- dbfile.input.insert( - in.path = dirname(result[[i]]$file[1]), - in.prefix = result[[i]]$dbfile.name[1], - siteid = site.id, - startdate = start_date, - enddate = end_date, - mimetype = mimetype, - formatname = formatname, - parentid = parent.id, - con = con, - hostname = machine$hostname, - allow.conflicting.dates = allow.conflicting.dates, - ens = ens.flag - ) + newinput$input.id <- c(newinput$input.id, existing.input[[i]]$id) + newinput$dbfile.id <- c(newinput$dbfile.id, dbfile.id) + } else if (id_not_added) { + # This is to tell input.insert if we are writing ensembles + # Why does it need it? Because it checks for inputs with the same time period, site, and machine + # and if it returns something it does not insert anymore, but for ensembles, it needs to bypass this condition + ens.flag <- if (!is.null(ensemble) || is.null(ensemble_name)) TRUE else FALSE - newinput$input.id <- c(newinput$input.id, new_entry$input.id) - newinput$dbfile.id <- c(newinput$dbfile.id, new_entry$dbfile.id) - } - } # End for loop + new_entry <- dbfile.input.insert( + in.path = dirname(result[[i]]$file[1]), + in.prefix = result[[i]]$dbfile.name[1], + siteid = site.id, + startdate = start_date, + enddate = end_date, + mimetype = mimetype, + formatname = formatname, + parentid = parent.id, + con = con, + hostname = machine$hostname, + allow.conflicting.dates = allow.conflicting.dates, + ens = ens.flag + ) - successful <- TRUE - return(newinput) - } else { - PEcAn.logger::logger.warn("Input was not added to the database") - successful <- TRUE - return(NULL) - } + newinput$input.id <- c(newinput$input.id, new_entry$input.id) + newinput$dbfile.id <- c(newinput$dbfile.id, new_entry$dbfile.id) + } + } # End for loop + return(newinput) } diff --git a/base/db/R/convert_input.R b/base/db/R/convert_input.R index 8828d069d6..ed267440fb 100644 --- a/base/db/R/convert_input.R +++ b/base/db/R/convert_input.R @@ -591,13 +591,21 @@ convert_input <- #---------------------------------------------------------------# # New arrangement of database adding code to deal with ensembles. - return (add.database.entries(result, con, start_date, - end_date, write, overwrite, - insert.new.file, input.args, - machine, mimetype, formatname, - allow.conflicting.dates, ensemble, - ensemble_name, existing.input, - existing.dbfile, input)) + if(write) { + add_entries_result <- return (add.database.entries(result, con, start_date, + end_date, overwrite, + insert.new.file, input.args, + machine, mimetype, formatname, + allow.conflicting.dates, ensemble, + ensemble_name, existing.input, + existing.dbfile, input)) + } else { + PEcAn.logger::logger.warn("Input was not added to the database") + successful <- TRUE + return(NULL) + } + successful <- TRUE + return (add_entries_result) } # convert_input