Merge pull request #403 from r-world-devs/400-allow-for-defining-conn…

…ections-for-different-modes-orsrepo-in-the-same-gitstats-object Make work setting repos and orgs in different hosts and handle 500 error
r-world-devs · Apr 25, 2024 · 596c811 · 596c811
2 parents 46ab26a + 46a1104
commit 596c811
Show file tree

Hide file tree

Showing 13 changed files with 75 additions and 90 deletions.
diff --git a/R/EngineGraphQL.R b/R/EngineGraphQL.R
@@ -52,7 +52,7 @@ EngineGraphQL <- R6::R6Class("EngineGraphQL",
          httr2::req_headers("Authorization" = paste0("Bearer ", private$token)) %>%
          httr2::req_body_json(list(query = gql_query, variables = vars)) %>%
          httr2::req_retry(
-           is_transient = ~ httr2::resp_status(.x) == "400|500|502",
+           is_transient = ~ httr2::resp_status(.x) %in% c(400, 500, 502),
            max_seconds = 60
          ) %>%
          httr2::req_perform()

diff --git a/R/EngineRest.R b/R/EngineRest.R
@@ -87,18 +87,18 @@ EngineRest <- R6::R6Class("EngineRest",
       resp <- NULL
       resp <- httr2::request(endpoint) %>%
         httr2::req_headers("Authorization" = paste0("Bearer ", token)) %>%
-        httr2::req_error(is_error = function(resp) httr2::resp_status(resp) %in% c(404, 500)) %>%
+        httr2::req_error(is_error = function(resp) httr2::resp_status(resp) == 404) %>%
         httr2::req_perform()
       if (!private$scan_all) {
         if (resp$status == 401) {
           message("HTTP 401 Unauthorized.")
         }
       }
-      if (resp$status %in% c(400, 403)) {
+      if (resp$status %in% c(400, 500, 403)) {
         resp <- httr2::request(endpoint) %>%
           httr2::req_headers("Authorization" = paste0("Bearer ", token)) %>%
           httr2::req_retry(
-            is_transient = ~ httr2::resp_status(.x) %in% c(400, 403),
+            is_transient = ~ httr2::resp_status(.x) %in% c(400, 500, 403),
             max_seconds = 60
           ) %>%
           httr2::req_perform()

diff --git a/R/GitHost.R b/R/GitHost.R
@@ -19,9 +19,9 @@ GitHost <- R6::R6Class("GitHost",
       private$check_if_public(host)
       private$set_token(token)
       private$set_graphql_url()
-      private$check_orgs_and_repos(orgs, repos)
+      private$set_searching_scope(orgs, repos)
       private$setup_engines()
-      private$set_scanning_scope(orgs, repos)
+      private$set_orgs_and_repos(orgs, repos)
     },
 
     # Pull repositories method
@@ -160,6 +160,9 @@ GitHost <- R6::R6Class("GitHost",
     # A GraphQL API url.
     graphql_api_url = NULL,
 
+    # Either repos, orgs or whole platform
+    searching_scope = NULL,
+
     # An endpoint for basic checks.
     test_endpoint = NULL,
 
@@ -182,6 +185,9 @@ GitHost <- R6::R6Class("GitHost",
     # repos A character vector of repositories.
     repos = NULL,
 
+    # repos_fullnames A character vector of repositories with full names.
+    repos_fullnames = NULL,
+
     # orgs_repos A named list of organizations with repositories.
     orgs_repos = NULL,
 
@@ -248,8 +254,45 @@ GitHost <- R6::R6Class("GitHost",
       }
     },
 
+    # Check if both repos and orgs are defined or not.
+    set_searching_scope = function(orgs, repos) {
+      if (is.null(repos) && is.null(orgs)) {
+        if (private$is_public) {
+          cli::cli_abort(c(
+            "You need to specify `orgs` for public Git Host.",
+            "x" = "Host will not be added.",
+            "i" = "Add organizations to your `orgs` parameter."
+          ),
+          call = NULL)
+        } else {
+          cli::cli_alert_warning(cli::col_yellow(
+            "No `orgs` specified. I will pull all organizations from the Git Host."
+          ))
+          cli::cli_alert_info(cli::col_grey("Searching scope set to [all]."))
+          private$searching_scope <- "all"
+          private$scan_all <- TRUE
+        }
+      }
+      if (!is.null(repos) && is.null(orgs)) {
+        cli::cli_alert_info(cli::col_grey("Searching scope set to [repo]."))
+        private$searching_scope <- "repo"
+      }
+      if (is.null(repos) && !is.null(orgs)) {
+        cli::cli_alert_info(cli::col_grey("Searching scope set to [org]."))
+        private$searching_scope <- "org"
+      }
+      if (!is.null(repos) && !is.null(orgs)) {
+        cli::cli_abort(c(
+          "Do not specify `orgs` while specifing `repos`.",
+          "x" = "Host will not be added.",
+          "i" = "Specify `orgs` or `repos`."
+        ),
+        call = NULL)
+      }
+    },
+
     # Set organization or repositories
-    set_scanning_scope = function(orgs, repos) {
+    set_orgs_and_repos = function(orgs, repos) {
       if (private$scan_all) {
         cli::cli_alert_info("[{private$host_name}][Engine:{cli::col_yellow('GraphQL')}] Pulling all organizations...")
         private$orgs <- private$engines$graphql$pull_orgs()
@@ -259,6 +302,7 @@ GitHost <- R6::R6Class("GitHost",
         }
         if (!is.null(repos)) {
           repos <- private$check_repositories(repos)
+          private$repos_fullnames <- repos
           orgs_repos <- private$extract_repos_and_orgs(repos)
           private$orgs <- names(orgs_repos)
           private$repos <- unname(unlist(orgs_repos))
@@ -271,6 +315,7 @@ GitHost <- R6::R6Class("GitHost",
     #' @param repos A character vector of repositories
     #' @return repos or NULL.
     check_repositories = function(repos) {
+      cli::cli_alert_info(cli::col_grey("Checking passed repositories..."))
       repos <- purrr::map(repos, function(repo) {
         repo_endpoint = glue::glue("{private$endpoints$repositories}/{repo}")
         check <- private$check_endpoint(
@@ -314,8 +359,7 @@ GitHost <- R6::R6Class("GitHost",
       orgs
     },
 
-    # @description Check whether the endpoint exists.
-    # @param type Type of repository.
+    # Check whether the endpoint exists.
     check_endpoint = function(endpoint, type) {
       check <- TRUE
       tryCatch(
@@ -340,40 +384,6 @@ GitHost <- R6::R6Class("GitHost",
       private$graphql_api_url <- glue::glue("{clean_api_url}/graphql")
     },
 
-    # Check if both repos and orgs are defined or not.
-    check_orgs_and_repos = function(orgs, repos) {
-      if (is.null(repos) && is.null(orgs)) {
-        if (private$is_public) {
-          cli::cli_abort(c(
-            "You need to specify `orgs` for public Git Host.",
-            "x" = "Host will not be added.",
-            "i" = "Add organizations to your `orgs` parameter."
-          ),
-          call = NULL)
-        } else {
-          cli::cli_alert_warning(cli::col_yellow(
-            "No `orgs` specified. I will pull all organizations from the Git Host."
-          ))
-          cli::cli_alert_info(cli::col_grey("Searching scope set to [all]."))
-          private$scan_all <- TRUE
-        }
-      }
-      if (!is.null(repos) && is.null(orgs)) {
-        cli::cli_alert_info(cli::col_grey("Searching scope set to [repo]."))
-      }
-      if (is.null(repos) && !is.null(orgs)) {
-        cli::cli_alert_info(cli::col_grey("Searching scope set to [org]."))
-      }
-      if (!is.null(repos) && !is.null(orgs)) {
-        cli::cli_abort(c(
-          "Do not specify `orgs` while specifing `repos`.",
-          "x" = "Host will not be added.",
-          "i" = "Specify `orgs` or `repos`."
-        ),
-        call = NULL)
-      }
-    },
-
     # Set default token if none exists.
     set_default_token = function() {
       primary_token_name <- private$token_name
@@ -430,7 +440,7 @@ GitHost <- R6::R6Class("GitHost",
 
     # Set repositories
     set_repos = function(settings, org) {
-      if (settings$searching_scope == "repo") {
+      if (private$searching_scope == "repo") {
         repos <- private$orgs_repos[[org]]
       } else {
         repos <- NULL

diff --git a/R/GitHostGitHub.R b/R/GitHostGitHub.R
@@ -193,7 +193,7 @@ GitHostGitHub <- R6::R6Class("GitHostGitHub",
 
     # Use repositories either from parameter or, if not set, pull them from API
     set_repositories = function(org, settings) {
-      if (settings$searching_scope == "repo") {
+      if (private$searching_scope == "repo") {
         repos_names <- private$orgs_repos[[org]]
       } else {
         repos_table <- private$pull_all_repos(

diff --git a/R/GitHostGitLab.R b/R/GitHostGitLab.R
@@ -225,7 +225,7 @@ GitHostGitLab <- R6::R6Class("GitHostGitLab",
 
     # Use repositories either from parameter or, if not set, pull them from API
     set_repositories = function(org, settings) {
-      if (settings$searching_scope == "repo") {
+      if (private$searching_scope == "repo") {
         repos <- private$orgs_repos[[org]]
         repos_names <- paste0(org, "%2f", repos)
       } else {

diff --git a/R/GitStats.R b/R/GitStats.R
@@ -25,7 +25,6 @@ GitStats <- R6::R6Class("GitStats",
         token = token,
         host = host
       )
-      private$set_searching_scope(orgs, repos)
       private$add_new_host(new_host)
     },
 
@@ -51,7 +50,6 @@ GitStats <- R6::R6Class("GitStats",
         token = token,
         host = host
       )
-      private$set_searching_scope(orgs, repos)
       private$add_new_host(new_host)
     },
 
@@ -318,15 +316,13 @@ GitStats <- R6::R6Class("GitStats",
 
     # @field settings List of search preferences.
     settings = list(
-      searching_scope = NULL,
       files = NULL,
       verbose = TRUE,
       cache = TRUE
     ),
 
     # temporary settings used when calling some methods for custom purposes
     temp_settings = list(
-      searching_scope = "org",
       files = NULL,
       verbose = FALSE,
       cache = TRUE
@@ -351,19 +347,6 @@ GitStats <- R6::R6Class("GitStats",
       }
     },
 
-    # Set searching scope
-    set_searching_scope = function(orgs, repos) {
-      if (!is.null(repos)) {
-        private$settings$searching_scope <- "repo"
-      }
-      if (!is.null(orgs)) {
-        private$settings$searching_scope <- "org"
-      }
-      if (is.null(orgs) && is.null(repos)) {
-        private$settings$searching_scope <- "all"
-      }
-    },
-
     # @description Helper to check if there are any hosts.
     check_for_host = function() {
       if (length(private$hosts) == 0) {
@@ -752,11 +735,15 @@ GitStats <- R6::R6Class("GitStats",
     print_orgs_and_repos = function() {
       orgs <- purrr::map(private$hosts, function(host) {
         host_priv <- environment(host$initialize)$private
-        orgs <- host_priv$orgs
+        if (host_priv$searching_scope == "org") {
+          orgs <- host_priv$orgs
+        }
       })
       repos <- purrr::map(private$hosts, function(host) {
         host_priv <- environment(host$initialize)$private
-        repos <- host_priv$repos
+        if (host_priv$searching_scope == "repo") {
+          repos <- host_priv$repos_fullnames
+        }
       })
       private$print_item(" Organizations", orgs)
       private$print_item(" Repositories", repos)

diff --git a/tests/testthat/_snaps/04-GitHost.md b/tests/testthat/_snaps/04-GitHost.md
@@ -1,23 +1,17 @@
-# `check_orgs_and_repos` does not throw error when `orgs` or `repos` are defined
+# `set_searching_scope` does not throw error when `orgs` or `repos` are defined
 
     Code
-      test_host$check_orgs_and_repos(orgs = "mbtests", repos = NULL)
+      test_host$set_searching_scope(orgs = "mbtests", repos = NULL)
     Message
       i Searching scope set to [org].
 
 ---
 
     Code
-      test_host$check_orgs_and_repos(orgs = NULL, repos = "mbtests/GitStatsTesting")
+      test_host$set_searching_scope(orgs = NULL, repos = "mbtests/GitStatsTesting")
     Message
       i Searching scope set to [repo].
 
-# `check_orgs_and_repos` throws error when host is public one
-
-    Do not specify `orgs` while specifing `repos`.
-    x Host will not be added.
-    i Specify `orgs` or `repos`.
-
 # `prepare_repos_table()` prepares repos table
 
     Code

diff --git a/tests/testthat/_snaps/05-GitHostGitLab.md b/tests/testthat/_snaps/05-GitHostGitLab.md
@@ -7,7 +7,7 @@
     Message
       i Using PAT from GITLAB_PAT envar.
 
-# `check_orgs_and_repos` throws error when both `orgs` and `repos` are defined
+# `set_searching_scope` throws error when both `orgs` and `repos` are defined
 
     Do not specify `orgs` while specifing `repos`.
     x Host will not be added.

diff --git a/tests/testthat/_snaps/07-GitStats.md b/tests/testthat/_snaps/07-GitStats.md
@@ -30,8 +30,8 @@
       A GitStats object for 2 hosts: 
       Hosts: https://api.github.com, https://gitlab.com/api/v4
       Scanning scope: 
-       Organizations: [3] r-world-devs, openpharma, mbtests
-       Repositories: [4] GitStats, GithubMetrics, gitstatstesting, gitstats-testing-2
+       Organizations: [0] 
+       Repositories: [4] r-world-devs/GitStats, openpharma/GithubMetrics, mbtests/gitstatstesting, mbtests/gitstats-testing-2
       Storage: <no tables in storage>
 
 # check_for_host returns error when no hosts are passed

diff --git a/tests/testthat/_snaps/set_host.md b/tests/testthat/_snaps/set_host.md
@@ -49,6 +49,7 @@
         "openpharma/GithubMetrics", "openpharma/DataFakeR"))
     Message
       i Searching scope set to [repo].
+      i Checking passed repositories...
       v Set connection to GitHub.
 
 # Set GitLab host with particular repos vector instead of orgs
@@ -58,6 +59,7 @@
       repos = c("mbtests/gitstatstesting", "mbtests/gitstats-testing-2"))
     Message
       i Searching scope set to [repo].
+      i Checking passed repositories...
       v Set connection to GitLab.
 
 # Set host prints error when repos and orgs are defined and host is not passed to GitStats

diff --git a/tests/testthat/setup.R b/tests/testthat/setup.R
@@ -13,14 +13,12 @@ test_team <- list(
 )
 
 test_settings <- list(
-  searching_scope = "org",
   files = NULL,
   verbose = TRUE,
   storage = TRUE
 )
 
 test_settings_silence <- list(
-  searching_scope = "org",
   files = NULL,
   verbose = FALSE,
   storage = TRUE

diff --git a/tests/testthat/test-04-GitHost.R b/tests/testthat/test-04-GitHost.R
@@ -1,17 +1,11 @@
 test_host <- create_github_testhost(orgs = "r-world-devs", mode = "private")
 
-test_that("`check_orgs_and_repos` does not throw error when `orgs` or `repos` are defined", {
+test_that("`set_searching_scope` does not throw error when `orgs` or `repos` are defined", {
   expect_snapshot(
-    test_host$check_orgs_and_repos(orgs = "mbtests", repos = NULL)
+    test_host$set_searching_scope(orgs = "mbtests", repos = NULL)
   )
   expect_snapshot(
-    test_host$check_orgs_and_repos(orgs = NULL, repos = "mbtests/GitStatsTesting")
-  )
-})
-
-test_that("`check_orgs_and_repos` throws error when host is public one", {
-  expect_snapshot_error(
-    test_host$check_orgs_and_repos(orgs = "mbtests", repos = "mbtests/GitStatsTesting")
+    test_host$set_searching_scope(orgs = NULL, repos = "mbtests/GitStatsTesting")
   )
 })
 

diff --git a/tests/testthat/test-05-GitHostGitLab.R b/tests/testthat/test-05-GitHostGitLab.R
@@ -20,9 +20,9 @@ test_that("`set_default_token` sets default token for GitLab", {
   )
 })
 
-test_that("`check_orgs_and_repos` throws error when both `orgs` and `repos` are defined", {
+test_that("`set_searching_scope` throws error when both `orgs` and `repos` are defined", {
   expect_snapshot_error(
-    test_host$check_orgs_and_repos(
+    test_host$set_searching_scope(
       orgs = "mbtests",
       repos = "mbtests/GitStatsTesting"
     )