fix: polish error msg, tests, code

ropensci · Apr 19, 2023 · 4bd0c23 · 4bd0c23
1 parent 2448864
commit 4bd0c23
Show file tree

Hide file tree

Showing 14 changed files with 247 additions and 140 deletions.
diff --git a/R/01_gaz_search.R b/R/01_gaz_search.R
@@ -161,7 +161,7 @@ gaz_rest_record_by_mrgid <- function(mrgid, with_geometry = FALSE, rdf = FALSE){
   # Add more info to error message if 404 not found
   if(httr2::resp_status(resp) == 404){
     httr2::resp_check_status(resp, c(
-      "i" = glue::glue("The MRGID <{mrgid}> does not exists.")
+      "i" = glue::glue("The MRGID <{mrgid}> does not exist.")
     ))
   }
 
@@ -269,7 +269,7 @@ gaz_rest_records_by_name <- function(name, with_geometry = FALSE, typeid = NULL,
   # Check status: first offset should be 200
   if(httr2::resp_is_error(resp)){
 
-    # If first is 404, either the typeID is not correct or the name does not exists
+    # If first is 404, either the typeID is not correct or the name does not exist
     if(httr2::resp_status(resp) == 404){
 
       # base msg
@@ -283,7 +283,7 @@ gaz_rest_records_by_name <- function(name, with_geometry = FALSE, typeid = NULL,
         msg <- c(msg,
                  "i" = glue::glue("The term '{name}' may not be available for the selected type."),
                  "*" = "Try with `typeid = NULL`."
-                 )
+        )
         httr2::resp_check_status(resp, info = msg)
       }
 
@@ -311,52 +311,51 @@ gaz_rest_records_by_name <- function(name, with_geometry = FALSE, typeid = NULL,
     # In any other case of error, abort and return the HTTP error
     httr2::resp_check_status(resp)
 
-  }else{
-
-    # If all ok, continue with first offset
-    resp <- resp %>%
-      httr2::resp_body_json() %>%
-      dplyr::bind_rows()
+  }
 
-    # End if there are no more records
-    if(nrow(resp) < 100){
-      resp <- resp %>% dplyr::arrange(MRGID)
+  # If all ok, continue with first offset
+  resp <- resp %>%
+    httr2::resp_body_json() %>%
+    dplyr::bind_rows()
 
-      if(with_geometry){
-        resp <- resp %>% gaz_add_geometry()
-      }
+  # End if there are no more records
+  if(nrow(resp) < 100){
+    resp <- resp %>% dplyr::arrange(MRGID)
 
-      return(resp)
+    if(with_geometry){
+      resp <- resp %>% gaz_add_geometry()
     }
 
-    # Enter infinite loop
-    while(TRUE){
-      offset <- offset + 100
-      resp_n <- gaz_records_by_name_at(offset)
-      http_status <- httr2::resp_status(resp_n)
+    return(resp)
+  }
 
-      if(httr2::resp_is_error(resp_n) & http_status != 404){
-        # Sanity check
-        httr2::resp_check_status(resp_n)
-      }
+  # Enter infinite loop
+  while(TRUE){
+    offset <- offset + 100
+    resp_n <- gaz_records_by_name_at(offset)
+    http_status <- httr2::resp_status(resp_n)
 
-      if(http_status == 404){
-        # End of the loop
-        resp <- resp %>% dplyr::arrange(MRGID)
+    if(httr2::resp_is_error(resp_n) & http_status != 404){
+      # Sanity check
+      httr2::resp_check_status(resp_n)
+    }
 
-        if(with_geometry) resp <- resp %>% gaz_add_geometry()
+    if(http_status == 404){
+      # End of the loop
+      resp <- resp %>% dplyr::arrange(MRGID)
 
-        return(resp)
-      }
+      if(with_geometry) resp <- resp %>% gaz_add_geometry()
 
+      return(resp)
+    }
 
-      # If no errors and not 404, continue with pagination
-      resp <- resp_n %>%
-        httr2::resp_body_json() %>%
-        dplyr::bind_rows(resp)
 
-    }
+    # If no errors and not 404, continue with pagination
+    resp <- resp_n %>%
+      httr2::resp_body_json() %>%
+      dplyr::bind_rows(resp)
   }
+
 }
 
 
@@ -469,7 +468,7 @@ gaz_rest_records_by_lat_long <- function(latitude, longitude, with_geometry = FA
   # Check status: first offset should be 200
   if(httr2::resp_is_error(resp)){
 
-    # If first is 404, either the typeID is not correct or the name does not exists
+    # If first is 404, either the typeID is not correct or the name does not exist
     if(httr2::resp_status(resp) == 404){
 
       # base msg
@@ -493,15 +492,37 @@ gaz_rest_records_by_lat_long <- function(latitude, longitude, with_geometry = FA
     # In any other case of error, abort and return the HTTP error
     httr2::resp_check_status(resp)
 
-  }else{
+  }
+  # If all ok, continue with first offset
+  resp <- resp %>%
+    httr2::resp_body_json() %>%
+    dplyr::bind_rows()
+
+  # End if there are no more records
+  if(nrow(resp) < 100){
+    resp <- resp %>% dplyr::arrange(MRGID)
 
-    # If all ok, continue with first offset
-    resp <- resp %>%
-      httr2::resp_body_json() %>%
-      dplyr::bind_rows()
+    if(with_geometry){
+      resp <- resp %>% gaz_add_geometry()
+    }
+
+    return(resp)
+  }
+
+  # If 100 rows or more, enter infinite loop
+  while(TRUE){
+
+    offset <- offset + 100
+    resp_n <- get_records_by_lat_long_at(offset)
+    http_status <- httr2::resp_status(resp_n)
+
+    if(httr2::resp_is_error(resp_n) & http_status != 404){
+      # Sanity check
+      httr2::resp_check_status(resp_n)
+    }
 
-    # End if there are no more records
-    if(nrow(resp) < 100){
+    if(http_status == 404){
+      # End of the loop
       resp <- resp %>% dplyr::arrange(MRGID)
 
       if(with_geometry){
@@ -511,36 +532,16 @@ gaz_rest_records_by_lat_long <- function(latitude, longitude, with_geometry = FA
       return(resp)
     }
 
-    # If 100 rows or more, enter infinite loop
-    while(TRUE){
-
-      offset <- offset + 100
-      resp_n <- get_records_by_lat_long_at(offset)
-      http_status <- httr2::resp_status(resp_n)
-
-      if(httr2::resp_is_error(resp_n) & http_status != 404){
-        # Sanity check
-        httr2::resp_check_status(resp_n)
-      }
+    # If no errors and not 404, continue with pagination
+    resp <- resp_n %>%
+      httr2::resp_body_json() %>%
+      dplyr::bind_rows(resp)
 
-      if(http_status == 404){
-        # End of the loop
-        resp <- resp %>% dplyr::arrange(MRGID)
+  }
 
-        if(with_geometry){
-          resp <- resp %>% gaz_add_geometry()
-        }
+}
 
-        return(resp)
-      }
 
-      # If no errors and not 404, continue with pagination
-      resp <- resp_n %>%
-        httr2::resp_body_json() %>%
-        dplyr::bind_rows(resp)
 
-    }
-  }
 
-}
 
diff --git a/R/02_gaz_relations.R b/R/02_gaz_relations.R
@@ -26,7 +26,6 @@
 #' # Or using its mrgid
 #' gaz_relations(3293)
 #' }
-
 gaz_relations <- function(x, ...){
   UseMethod("gaz_relations")
 }
@@ -83,23 +82,6 @@ gaz_rest_relations_by_mrgid <- function(mrgid, with_geometry = FALSE, direction
   mrgid <- checkmate::assert_integerish(mrgid, lower = 1, any.missing = FALSE,
                                        null.ok = TRUE, coerce = TRUE, len = 1)
 
-  # Extra info for status 404 not found
-  .is_error <- function(resp){
-    if(httr2::resp_status(resp) == 404){
-      httr2::resp_check_status(resp, info = c(
-        "i" = glue::glue("MRGID: <{mrgid}>"),
-        "i" = glue::glue("type: `{type}`"),
-        "i" = glue::glue("direction: `{direction}`")
-      ))
-    }
-
-    if(httr2::resp_is_error(resp)){
-      TRUE
-    }else{
-      FALSE
-    }
-  }
-
   # Perform
   resp <- marineregions.org() %>%
     httr2::request() %>%
@@ -109,15 +91,29 @@ gaz_rest_relations_by_mrgid <- function(mrgid, with_geometry = FALSE, direction
     httr2::req_url_query(direction = direction, type = type) %>%
     httr2::req_user_agent(mr_user_agent) %>%
     httr2::req_headers(accept = "application/json") %>%
-    httr2::req_error(is_error = .is_error) %>%
-    httr2::req_perform() %>%
-    httr2::resp_body_json() %>%
+    httr2::req_error(is_error = function(resp) FALSE) %>%
+    httr2::req_perform()
+
+  # Sanity check
+  if(httr2::resp_is_error(resp)){
+    if(httr2::resp_status(resp) == 404){
+      assert_mrgid_exists(mrgid)
+
+      httr2::resp_check_status(resp, info = c("x" = glue::glue(
+        'No relations found for MRGID <{mrgid}> with `direction` = "{direction}" and `type` = "{type}".'
+      )))
+    }
+    httr2::resp_check_status(resp)
+  }
+
+  # Continue if ok
+  resp <- httr2::resp_body_json(resp) %>%
     dplyr::bind_rows()
 
   if(with_geometry){
     resp <- resp %>% gaz_add_geometry()
   }
 
   resp
-
 }
+
diff --git a/R/03_gaz_sources.R b/R/03_gaz_sources.R
@@ -78,7 +78,7 @@ gaz_rest_records_by_source <- function(source, with_geometry = FALSE){
     httr2::req_error(is_error = function(resp) FALSE) %>%
     httr2::req_perform()
 
-  # If error 404, either a wrong source was provided or there are no records for that source.
+  # If error 404, a wrong source was provided
   if(httr2::resp_is_error(resp)){
 
     if(httr2::resp_status(resp) == 404){
@@ -100,11 +100,10 @@ gaz_rest_records_by_source <- function(source, with_geometry = FALSE){
     httr2::resp_body_json() %>%
     dplyr::bind_rows()
 
+  # Sanity check
+  # This service returns 200 and "[]" when there are no records
   if(nrow(resp) == 0){
-    cli::cli_abort(c(
-      "!" = "There are no MRGIDs for this source.",
-      "i" = "Source: {.val {source}}"
-    ))
+    cli::cli_abort(c("!" = 'No records found for {.arg source} = {.val {source}}'))
   }
 
   if(with_geometry){
@@ -113,9 +112,6 @@ gaz_rest_records_by_source <- function(source, with_geometry = FALSE){
 
   resp
 }
-# src <- "Van Eck, B.T.M. (Ed.) (1999). De Scheldeatlas: een beeld van een estuarium. Rijksinstituut voor Kust en Zee/Schelde InformatieCentrum: Middelburg. ISBN 90-369-3434-6. 120 pp."
-# gaz_rest_records_by_source(src)
-# gaz_rest_records_by_source("this is not a source")
 
 
 #' Get all the Marine Regions sources
@@ -126,6 +122,9 @@ gaz_rest_records_by_source <- function(source, with_geometry = FALSE){
 #' - `sourceURL`: if available, the URL of the source.
 #' @export
 #'
+#' @details
+#' gaz_search() is a memoised function from gaz_rest_search(). See [memoise::memoise()].
+#'
 #' @seealso [gaz_rest], [gaz_search_by_source()], [gaz_rest_records_by_source()], [gaz_rest_source_by_sourceid()]
 #'
 #' @examples \dontrun{
@@ -134,6 +133,9 @@ gaz_rest_records_by_source <- function(source, with_geometry = FALSE){
 #'
 #' # is the same as
 #' gaz_sources()
+#'
+#' memoise::is.memoised(gaz_sources)
+#' #> [1] TRUE
 #' }
 gaz_rest_sources <- function(){
   sourceID <- NULL
@@ -158,36 +160,36 @@ gaz_rest_sources <- function(){
   if(httr2::resp_is_error(resp)){
     httr2::resp_check_status(resp)
 
-  }else{
+  }
 
-    resp <- resp %>%
-      httr2::resp_body_json() %>%
-      dplyr::bind_rows()
+  resp <- resp %>%
+    httr2::resp_body_json() %>%
+    dplyr::bind_rows()
 
-    # Enter infinite loop
-    while(TRUE){
-      offset <- offset + 100
-      resp_n <- get_source_at(offset)
-      http_status <- httr2::resp_status(resp_n)
+  # Enter infinite loop
+  while(TRUE){
+    offset <- offset + 100
+    resp_n <- get_source_at(offset)
+    http_status <- httr2::resp_status(resp_n)
 
-      if(httr2::resp_is_error(resp_n) & http_status != 404){
-        # Sanity check
-        httr2::resp_check_status(resp_n)
-      }
+    if(httr2::resp_is_error(resp_n) & http_status != 404){
+      # Sanity check
+      httr2::resp_check_status(resp_n)
+    }
 
-      if(http_status == 404){
-        # End of the loop
-        resp <- resp %>% dplyr::arrange(sourceID)
-        return(resp)
-      }
+    if(http_status == 404){
+      # End of the loop
+      resp <- resp %>% dplyr::arrange(sourceID)
+      return(resp)
+    }
 
 
-      # If no errors and not 404, continue with pagination
-      resp <- resp_n %>%
-        httr2::resp_body_json() %>%
-        dplyr::bind_rows(resp)
-    }
+    # If no errors and not 404, continue with pagination
+    resp <- resp_n %>%
+      httr2::resp_body_json() %>%
+      dplyr::bind_rows(resp)
   }
+
 }
 
 #' @name gaz_sources
@@ -226,4 +228,3 @@ gaz_rest_source_by_sourceid <- function(sourceid){
 
 }
 
-